diff --git a/builder-api/src/main/java/org/acme/controller/EligibilityCheckResource.java b/builder-api/src/main/java/org/acme/controller/EligibilityCheckResource.java index 4cec9cfd..636baa23 100644 --- a/builder-api/src/main/java/org/acme/controller/EligibilityCheckResource.java +++ b/builder-api/src/main/java/org/acme/controller/EligibilityCheckResource.java @@ -48,22 +48,38 @@ public class EligibilityCheckResource { // By default, returns the most recent versions of all published checks owned by the calling user // If the query parameter 'working' is set to true, - // then all the working check objects owned by the user are returned + // then the active (non-archived) working check objects owned by the user are returned + // Adding 'includeArchived=true' to a working request returns the archived ones alongside them, + // so a caller that renders both lists can do it with a single read. + // Published checks are never archived, so 'includeArchived=true' without 'working=true' + // is rejected rather than silently answered with an unfiltered published list @GET public Response getCustomChecks( @Context SecurityIdentity identity, - @QueryParam("working") Boolean working + @QueryParam("working") Boolean working, + @QueryParam("includeArchived") Boolean includeArchived ) { String userId = AuthUtils.getUserId(identity); if (userId == null) { return Response.status(Response.Status.UNAUTHORIZED).build(); } + if (Boolean.TRUE.equals(includeArchived) && !Boolean.TRUE.equals(working)) { + return Response.status(Response.Status.BAD_REQUEST) + .entity(Map.of("error", "includeArchived requires working=true")) + .build(); + } + List checks; - if (working != null && working){ - Log.info("Fetching all working custom checks. User: " + userId); - checks = eligibilityCheckRepository.getWorkingCustomChecks(userId); + if (Boolean.TRUE.equals(working)){ + if (Boolean.TRUE.equals(includeArchived)) { + Log.info("Fetching active and archived custom checks. User: " + userId); + checks = eligibilityCheckRepository.getAllWorkingCustomChecks(userId); + } else { + Log.info("Fetching active working custom checks. User: " + userId); + checks = eligibilityCheckRepository.getWorkingCustomChecks(userId); + } } else { Log.info("Fetching all published custom checks. User: " + userId); checks = eligibilityCheckRepository.getLatestVersionPublishedCustomChecks(userId); @@ -435,6 +451,43 @@ public Response archiveCustomCheck(@Context SecurityIdentity identity, @PathPara } } + @POST + @Path("/{checkId}/restore") + public Response restoreCustomCheck(@Context SecurityIdentity identity, @PathParam("checkId") String checkId) { + String userId = AuthUtils.getUserId(identity); + if (userId == null) { + return Response.status(Response.Status.UNAUTHORIZED).build(); + } + + Optional checkOpt = eligibilityCheckRepository + .getWorkingCustomCheck(userId, checkId, true); + if (checkOpt.isEmpty()) { + return Response.status(Response.Status.NOT_FOUND).build(); + } + + EligibilityCheck check = checkOpt.get(); + if (!check.getOwnerId().equals(userId)) { + return Response.status(Response.Status.UNAUTHORIZED).build(); + } + + if (!check.getIsArchived()) { + return Response.status(Response.Status.BAD_REQUEST) + .entity(Map.of("error", "Check is not archived")) + .build(); + } + + check.setIsArchived(false); + try { + eligibilityCheckRepository.updateWorkingCustomCheck(check); + return Response.ok(check, MediaType.APPLICATION_JSON).build(); + } catch (Exception e) { + Log.error("Could not restore check " + checkId, e); + return Response.status(Response.Status.INTERNAL_SERVER_ERROR) + .entity(Map.of("error", "Could not restore check")) + .build(); + } + } + // ========== Sub-Resource Endpoints: Related Resources ========== /* Endpoint for returning all Published Check Versions related to a given Working Eligibility Check */ diff --git a/builder-api/src/main/java/org/acme/persistence/EligibilityCheckRepository.java b/builder-api/src/main/java/org/acme/persistence/EligibilityCheckRepository.java index 6ab2786e..c19473f2 100644 --- a/builder-api/src/main/java/org/acme/persistence/EligibilityCheckRepository.java +++ b/builder-api/src/main/java/org/acme/persistence/EligibilityCheckRepository.java @@ -10,6 +10,8 @@ public interface EligibilityCheckRepository { List getWorkingCustomChecks(String userId); + List getAllWorkingCustomChecks(String userId); + List getPublishedCheckVersions(EligibilityCheck workingCustomCheck) throws Exception; List getLatestVersionPublishedCustomChecks(String userId); diff --git a/builder-api/src/main/java/org/acme/persistence/impl/EligibilityCheckRepositoryImpl.java b/builder-api/src/main/java/org/acme/persistence/impl/EligibilityCheckRepositoryImpl.java index cc31b7a1..64e972b9 100644 --- a/builder-api/src/main/java/org/acme/persistence/impl/EligibilityCheckRepositoryImpl.java +++ b/builder-api/src/main/java/org/acme/persistence/impl/EligibilityCheckRepositoryImpl.java @@ -28,11 +28,16 @@ public class EligibilityCheckRepositoryImpl implements EligibilityCheckRepositor private StorageService storageService; public List getWorkingCustomChecks(String userId){ + return getAllWorkingCustomChecks(userId).stream() + .filter(check -> !check.getIsArchived()) + .toList(); + } + + public List getAllWorkingCustomChecks(String userId){ List> checkMaps = FirestoreUtils.getFirestoreDocsByField(CollectionNames.WORKING_CUSTOM_CHECK_COLLECTION, FieldNames.OWNER_ID, userId); ObjectMapper mapper = new ObjectMapper(); return checkMaps.stream() .map(checkMap -> mapper.convertValue(checkMap, EligibilityCheck.class)) - .filter(check -> !check.getIsArchived()) .toList(); } diff --git a/builder-api/src/test/java/org/acme/controller/EligibilityCheckResourceTest.java b/builder-api/src/test/java/org/acme/controller/EligibilityCheckResourceTest.java index 2b636503..8adc1d6a 100644 --- a/builder-api/src/test/java/org/acme/controller/EligibilityCheckResourceTest.java +++ b/builder-api/src/test/java/org/acme/controller/EligibilityCheckResourceTest.java @@ -19,6 +19,7 @@ import java.util.Optional; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertSame; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; @@ -226,6 +227,79 @@ private CreateCheckRequest createCheckRequest() { List.of()); } + @Test + void getCustomChecksCanIncludeArchivedChecksInOneRead() { + EligibilityCheck archivedCheck = new EligibilityCheck( + "old-check", "income", "an old check", List.of(), USER_ID); + archivedCheck.setIsArchived(true); + when(repository.getAllWorkingCustomChecks(USER_ID)) + .thenReturn(List.of(workingCheck, archivedCheck)); + + Response response = resource.getCustomChecks(identity, true, true); + + assertEquals(Response.Status.OK.getStatusCode(), response.getStatus()); + assertEquals(List.of(workingCheck, archivedCheck), response.getEntity()); + verify(repository).getAllWorkingCustomChecks(USER_ID); + verify(repository, never()).getWorkingCustomChecks(USER_ID); + } + + @Test + void getCustomChecksOmitsArchivedChecksByDefault() { + when(repository.getWorkingCustomChecks(USER_ID)).thenReturn(List.of(workingCheck)); + + Response response = resource.getCustomChecks(identity, true, null); + + assertEquals(Response.Status.OK.getStatusCode(), response.getStatus()); + assertEquals(List.of(workingCheck), response.getEntity()); + verify(repository).getWorkingCustomChecks(USER_ID); + verify(repository, never()).getAllWorkingCustomChecks(USER_ID); + } + + @Test + void getCustomChecksRejectsIncludeArchivedWithoutWorking() { + Response response = resource.getCustomChecks(identity, null, true); + + assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), response.getStatus()); + assertEquals("includeArchived requires working=true", + ((java.util.Map) response.getEntity()).get("error")); + verify(repository, never()).getLatestVersionPublishedCustomChecks(USER_ID); + } + + @Test + void restoreCustomCheckMakesAnArchivedCheckActive() throws Exception { + workingCheck.setIsArchived(true); + when(repository.getWorkingCustomCheck(USER_ID, CHECK_ID, true)) + .thenReturn(Optional.of(workingCheck)); + + Response response = resource.restoreCustomCheck(identity, CHECK_ID); + + assertEquals(Response.Status.OK.getStatusCode(), response.getStatus()); + assertFalse(workingCheck.getIsArchived()); + verify(repository).updateWorkingCustomCheck(workingCheck); + } + + @Test + void restoreCustomCheckRejectsAnActiveCheck() throws Exception { + when(repository.getWorkingCustomCheck(USER_ID, CHECK_ID, true)) + .thenReturn(Optional.of(workingCheck)); + + Response response = resource.restoreCustomCheck(identity, CHECK_ID); + + assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), response.getStatus()); + assertEquals("Check is not archived", ((java.util.Map) response.getEntity()).get("error")); + verify(repository, never()).updateWorkingCustomCheck(any()); + } + + @Test + void restoreCustomCheckReturnsNotFoundForAnUnknownCheck() { + when(repository.getWorkingCustomCheck(USER_ID, CHECK_ID, true)) + .thenReturn(Optional.empty()); + + Response response = resource.restoreCustomCheck(identity, CHECK_ID); + + assertEquals(Response.Status.NOT_FOUND.getStatusCode(), response.getStatus()); + } + @Test void firstPublishKeepsInitialVersion() throws Exception { when(repository.getPublishedCheckVersions(workingCheck)).thenReturn(List.of()); diff --git a/builder-frontend/src/api/check.ts b/builder-frontend/src/api/check.ts index 20a64216..06b06495 100644 --- a/builder-frontend/src/api/check.ts +++ b/builder-frontend/src/api/check.ts @@ -151,11 +151,20 @@ export const validateCheckDmn = async ( } }; -export const fetchUserDefinedChecks = async ( - working: boolean, -): Promise => { +export const fetchUserDefinedChecks = async ({ + working, + includeArchived = false, +}: { + working: boolean; + includeArchived?: boolean; +}): Promise => { const workingQueryParam = working ? "true" : "false"; - let url: string = apiUrl + `/custom-checks?working=${workingQueryParam}`; + const includeArchivedQueryParam = includeArchived + ? "&includeArchived=true" + : ""; + const url = + apiUrl + + `/custom-checks?working=${workingQueryParam}${includeArchivedQueryParam}`; try { const response = await authGet(url); @@ -243,3 +252,17 @@ export const archiveCheck = async (checkId: string): Promise => { throw error; } }; + +export const restoreCheck = async (checkId: string): Promise => { + const url = apiUrl + `/custom-checks/${checkId}/restore`; + try { + const response = await authPost(url); + + if (!response.ok) { + throw new Error(`Restore failed with status: ${response.status}`); + } + } catch (error) { + console.error("Error restoring check:", error); + throw error; + } +}; diff --git a/builder-frontend/src/components/homeScreen/eligibilityCheckList/EligibilityChecksList.tsx b/builder-frontend/src/components/homeScreen/eligibilityCheckList/EligibilityChecksList.tsx index 18831027..e1e55c46 100644 --- a/builder-frontend/src/components/homeScreen/eligibilityCheckList/EligibilityChecksList.tsx +++ b/builder-frontend/src/components/homeScreen/eligibilityCheckList/EligibilityChecksList.tsx @@ -1,4 +1,4 @@ -import { createSignal, For, Setter, Show } from "solid-js"; +import { Accessor, createSignal, For, Setter, Show } from "solid-js"; import { useNavigate } from "@solidjs/router"; import Loading from "@/components/Loading"; @@ -13,11 +13,18 @@ import { ArchiveCheck } from "@/components/homeScreen/eligibilityCheckList/modal import { Button } from "@/components/shared/Button"; const EligibilityChecksList = () => { - const { checks, actions, actionInProgress, initialLoadStatus } = - eligibilityCheckResource(); + const { + checks, + archivedChecks, + actions, + actionInProgress, + initialLoadStatus, + } = eligibilityCheckResource(); const navigate = useNavigate(); const [addingNewCheck, setAddingNewCheck] = createSignal(false); + const [showArchivedChecks, setShowArchivedChecks] = + createSignal(false); const [checkIdToRemove, setCheckIdToRemove] = createSignal( null, @@ -77,6 +84,41 @@ const EligibilityChecksList = () => { )} + 0}> +
+ + +
+

Archived checks

+

+ Restore a check to edit it or use its name again. +

+
+ + {(check) => ( + actions.restoreCheck(check.id)} + actionInProgress={actionInProgress} + /> + )} + +
+
+ +
+ setCheckIdToRemove(null)} @@ -98,10 +140,16 @@ const CheckCard = ({ eligibilityCheck, navigateToCheck, setCheckIdToRemove, + archived = false, + onRestore, + actionInProgress, }: { eligibilityCheck: EligibilityCheck; navigateToCheck: (check: EligibilityCheck) => void; setCheckIdToRemove: Setter; + archived?: boolean; + onRestore?: () => Promise; + actionInProgress?: Accessor; }) => { return (
@@ -124,22 +172,36 @@ const CheckCard = ({ id={"benefit-card-actions-" + eligibilityCheck.id} class="p-4 flex justify-end space-x-2" > - - + + + } > - Archive - + +
diff --git a/builder-frontend/src/components/homeScreen/eligibilityCheckList/eligibilityCheckResource.test.ts b/builder-frontend/src/components/homeScreen/eligibilityCheckList/eligibilityCheckResource.test.ts index bce82f52..1bb90312 100644 --- a/builder-frontend/src/components/homeScreen/eligibilityCheckList/eligibilityCheckResource.test.ts +++ b/builder-frontend/src/components/homeScreen/eligibilityCheckList/eligibilityCheckResource.test.ts @@ -4,11 +4,17 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; vi.mock("@/api/check", () => ({ addCheck: vi.fn(), archiveCheck: vi.fn(), + restoreCheck: vi.fn(), fetchUserDefinedChecks: vi.fn().mockResolvedValue([]), })); -import { addCheck } from "@/api/check"; +vi.mock("solid-toast", () => ({ + default: { success: vi.fn(), error: vi.fn() }, +})); + +import { addCheck, fetchUserDefinedChecks, restoreCheck } from "@/api/check"; import eligibilityCheckResource from "./eligibilityCheckResource"; +import type { EligibilityCheck } from "@/types"; describe("eligibilityCheckResource", () => { beforeEach(() => vi.clearAllMocks()); @@ -42,4 +48,81 @@ describe("eligibilityCheckResource", () => { }); }); }); + + it("splits one response into the active and archived lists", async () => { + const active = { id: "active-id", isArchived: false }; + const archived = { id: "archived-id", isArchived: true }; + vi.mocked(fetchUserDefinedChecks).mockResolvedValue([ + active, + archived, + ] as unknown as EligibilityCheck[]); + + await new Promise((resolve, reject) => { + createRoot((dispose) => { + const resource = eligibilityCheckResource(); + queueMicrotask(() => { + try { + expect(fetchUserDefinedChecks).toHaveBeenCalledTimes(1); + expect(resource.checks()).toEqual([active]); + expect(resource.archivedChecks()).toEqual([archived]); + resolve(); + } catch (assertionError) { + reject(assertionError); + } finally { + dispose(); + } + }); + }); + }); + }); + + it("survives a failed fetch instead of throwing out of the effect", async () => { + vi.mocked(fetchUserDefinedChecks).mockRejectedValue( + new Error("Fetch failed with status: 500"), + ); + + await new Promise((resolve, reject) => { + createRoot((dispose) => { + const resource = eligibilityCheckResource(); + queueMicrotask(() => { + try { + expect(resource.checks()).toEqual([]); + expect(resource.archivedChecks()).toEqual([]); + expect(resource.initialLoadStatus.error()).toBeInstanceOf(Error); + resolve(); + } catch (assertionError) { + reject(assertionError); + } finally { + dispose(); + } + }); + }); + }); + }); + + it("restores a check and refreshes the check list", async () => { + await new Promise((resolve, reject) => { + createRoot((dispose) => { + const resource = eligibilityCheckResource(); + resource.actions + .restoreCheck("archived-check-id") + .then(() => { + try { + expect(restoreCheck).toHaveBeenCalledWith("archived-check-id"); + expect(fetchUserDefinedChecks).toHaveBeenCalledWith({ + working: true, + includeArchived: true, + }); + expect(resource.actionInProgress()).toBe(false); + resolve(); + } catch (assertionError) { + reject(assertionError); + } finally { + dispose(); + } + }) + .catch(reject); + }); + }); + }); }); diff --git a/builder-frontend/src/components/homeScreen/eligibilityCheckList/eligibilityCheckResource.ts b/builder-frontend/src/components/homeScreen/eligibilityCheckList/eligibilityCheckResource.ts index cafa69b2..f0af54e7 100644 --- a/builder-frontend/src/components/homeScreen/eligibilityCheckList/eligibilityCheckResource.ts +++ b/builder-frontend/src/components/homeScreen/eligibilityCheckList/eligibilityCheckResource.ts @@ -1,14 +1,22 @@ import { createResource, createEffect, Accessor, createSignal } from "solid-js"; import { createStore } from "solid-js/store"; +import toast from "solid-toast"; import type { EligibilityCheck, CreateCheckRequest } from "@/types"; -import { addCheck, archiveCheck, fetchUserDefinedChecks } from "@/api/check"; +import { + addCheck, + archiveCheck, + fetchUserDefinedChecks, + restoreCheck, +} from "@/api/check"; export interface EligibilityCheckResource { checks: () => EligibilityCheck[]; + archivedChecks: () => EligibilityCheck[]; actions: { addNewCheck: (check: CreateCheckRequest) => Promise; removeCheck: (checkIdToRemove: string) => Promise; + restoreCheck: (checkIdToRestore: string) => Promise; }; actionInProgress: Accessor; initialLoadStatus: { @@ -18,16 +26,28 @@ export interface EligibilityCheckResource { } const eligibilityCheckResource = (): EligibilityCheckResource => { - const [checksResource, { refetch }] = createResource(fetchUserDefinedChecks); + // Both lists come from one request: the two are the same Firestore + // collection, so asking for them separately doubles the reads. + const [checksResource, { refetch: refetchChecks }] = createResource(() => + fetchUserDefinedChecks({ working: true, includeArchived: true }), + ); const [actionInProgress, setActionInProgress] = createSignal(false); // Local fine-grained store const [checks, setChecks] = createStore([]); + const [archivedChecks, setArchivedChecks] = createStore( + [], + ); - // When resource resolves, sync it into the store + // When the resource resolves, split it into the two stores. Reading an + // errored resource rethrows, so check for failure before touching the + // accessor. createEffect(() => { - if (checksResource()) { - setChecks(checksResource()!); + if (checksResource.error) return; + const loadedChecks = checksResource(); + if (loadedChecks) { + setChecks(loadedChecks.filter((check) => !check.isArchived)); + setArchivedChecks(loadedChecks.filter((check) => check.isArchived)); } }); @@ -36,7 +56,7 @@ const eligibilityCheckResource = (): EligibilityCheckResource => { setActionInProgress(true); try { await addCheck(check); - await refetch(); + await refetchChecks(); } catch (e) { console.error("Failed to add new check", e); throw e; @@ -49,16 +69,38 @@ const eligibilityCheckResource = (): EligibilityCheckResource => { setActionInProgress(true); try { await archiveCheck(checkIdToRemove); - await refetch(); + await refetchChecks(); + toast.success("Check archived."); } catch (e) { console.error("Failed to archive check", e); + toast.error("Could not archive check. Please try again."); + } finally { + setActionInProgress(false); + } + }; + + const restoreArchivedCheck = async (checkIdToRestore: string) => { + setActionInProgress(true); + try { + await restoreCheck(checkIdToRestore); + await refetchChecks(); + toast.success("Check restored."); + } catch (e) { + console.error("Failed to restore check", e); + toast.error("Could not restore check. Please try again."); + } finally { + setActionInProgress(false); } - setActionInProgress(false); }; return { checks: () => checks, - actions: { addNewCheck, removeCheck }, + archivedChecks: () => archivedChecks, + actions: { + addNewCheck, + removeCheck, + restoreCheck: restoreArchivedCheck, + }, actionInProgress, initialLoadStatus: { loading: () => checksResource.loading, diff --git a/builder-frontend/src/components/homeScreen/eligibilityCheckList/modals/ArchiveCheck.tsx b/builder-frontend/src/components/homeScreen/eligibilityCheckList/modals/ArchiveCheck.tsx index a3b533f8..4e843051 100644 --- a/builder-frontend/src/components/homeScreen/eligibilityCheckList/modals/ArchiveCheck.tsx +++ b/builder-frontend/src/components/homeScreen/eligibilityCheckList/modals/ArchiveCheck.tsx @@ -10,8 +10,8 @@ export const ArchiveCheck = (props: Props) => {
Archive Check
- Are you sure you want to archive this Eligibility Check? This action - cannot be undone. + Are you sure you want to archive this Eligibility Check? You can restore + it later from the archived checks section.