From 14c80a386ae8dbfc2caddc5ed5a41e298488ad62 Mon Sep 17 00:00:00 2001 From: Preston Cabe Date: Mon, 7 Sep 2026 20:44:12 -0400 Subject: [PATCH 1/5] Add restore flow for archived checks --- .../controller/EligibilityCheckResource.java | 51 ++++++++++- .../EligibilityCheckRepository.java | 2 + .../impl/EligibilityCheckRepositoryImpl.java | 10 ++- .../EligibilityCheckResourceTest.java | 51 +++++++++++ builder-frontend/src/api/check.ts | 19 +++- .../EligibilityChecksList.tsx | 88 +++++++++++++++---- .../eligibilityCheckResource.test.ts | 31 ++++++- .../eligibilityCheckResource.ts | 59 +++++++++++-- .../modals/ArchiveCheck.tsx | 4 +- builder-frontend/src/types.ts | 1 + 10 files changed, 282 insertions(+), 34 deletions(-) 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..b6d5262e 100644 --- a/builder-api/src/main/java/org/acme/controller/EligibilityCheckResource.java +++ b/builder-api/src/main/java/org/acme/controller/EligibilityCheckResource.java @@ -52,7 +52,8 @@ public class EligibilityCheckResource { @GET public Response getCustomChecks( @Context SecurityIdentity identity, - @QueryParam("working") Boolean working + @QueryParam("working") Boolean working, + @QueryParam("archived") Boolean archived ) { String userId = AuthUtils.getUserId(identity); if (userId == null) { @@ -61,9 +62,14 @@ public Response getCustomChecks( 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(archived)) { + Log.info("Fetching archived custom checks. User: " + userId); + checks = eligibilityCheckRepository.getArchivedCustomChecks(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 +441,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..4284b253 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 getArchivedCustomChecks(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..585a86c1 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,19 @@ public class EligibilityCheckRepositoryImpl implements EligibilityCheckRepositor private StorageService storageService; public List getWorkingCustomChecks(String userId){ + return getWorkingCustomChecksByArchivedStatus(userId, false); + } + + public List getArchivedCustomChecks(String userId){ + return getWorkingCustomChecksByArchivedStatus(userId, true); + } + + private List getWorkingCustomChecksByArchivedStatus(String userId, boolean archived){ 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()) + .filter(check -> check.getIsArchived() == archived) .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..7f5061da 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,56 @@ private CreateCheckRequest createCheckRequest() { List.of()); } + @Test + void getCustomChecksCanListArchivedChecks() { + EligibilityCheck archivedCheck = new EligibilityCheck( + "old-check", "income", "an old check", List.of(), USER_ID); + archivedCheck.setIsArchived(true); + when(repository.getArchivedCustomChecks(USER_ID)).thenReturn(List.of(archivedCheck)); + + Response response = resource.getCustomChecks(identity, true, true); + + assertEquals(Response.Status.OK.getStatusCode(), response.getStatus()); + assertEquals(List.of(archivedCheck), response.getEntity()); + verify(repository).getArchivedCustomChecks(USER_ID); + verify(repository, never()).getWorkingCustomChecks(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..38c5e51a 100644 --- a/builder-frontend/src/api/check.ts +++ b/builder-frontend/src/api/check.ts @@ -153,9 +153,12 @@ export const validateCheckDmn = async ( export const fetchUserDefinedChecks = async ( working: boolean, + archived = false, ): Promise => { const workingQueryParam = working ? "true" : "false"; - let url: string = apiUrl + `/custom-checks?working=${workingQueryParam}`; + const archivedQueryParam = archived ? "&archived=true" : ""; + const url = + apiUrl + `/custom-checks?working=${workingQueryParam}${archivedQueryParam}`; try { const response = await authGet(url); @@ -243,3 +246,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..10e0c966 100644 --- a/builder-frontend/src/components/homeScreen/eligibilityCheckList/EligibilityChecksList.tsx +++ b/builder-frontend/src/components/homeScreen/eligibilityCheckList/EligibilityChecksList.tsx @@ -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,40 @@ const EligibilityChecksList = () => { )} + 0}> +
+ + +
+

Archived checks

+

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

+
+ + {(check) => ( + actions.restoreCheck(check.id)} + /> + )} + +
+
+ +
+ setCheckIdToRemove(null)} @@ -98,10 +139,14 @@ const CheckCard = ({ eligibilityCheck, navigateToCheck, setCheckIdToRemove, + archived = false, + onRestore, }: { eligibilityCheck: EligibilityCheck; navigateToCheck: (check: EligibilityCheck) => void; setCheckIdToRemove: Setter; + archived?: boolean; + onRestore?: () => Promise; }) => { return (
@@ -124,22 +169,31 @@ 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..7a7e3fef 100644 --- a/builder-frontend/src/components/homeScreen/eligibilityCheckList/eligibilityCheckResource.test.ts +++ b/builder-frontend/src/components/homeScreen/eligibilityCheckList/eligibilityCheckResource.test.ts @@ -4,10 +4,15 @@ 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"; describe("eligibilityCheckResource", () => { @@ -42,4 +47,28 @@ describe("eligibilityCheckResource", () => { }); }); }); + + it("restores a check and refreshes both active and archived lists", 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(true); + expect(fetchUserDefinedChecks).toHaveBeenCalledWith(true, 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..52baa027 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,11 +26,18 @@ export interface EligibilityCheckResource { } const eligibilityCheckResource = (): EligibilityCheckResource => { - const [checksResource, { refetch }] = createResource(fetchUserDefinedChecks); + const [checksResource, { refetch: refetchChecks }] = createResource(() => + fetchUserDefinedChecks(true), + ); + const [archivedChecksResource, { refetch: refetchArchivedChecks }] = + createResource(() => fetchUserDefinedChecks(true, 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 createEffect(() => { @@ -31,12 +46,18 @@ const eligibilityCheckResource = (): EligibilityCheckResource => { } }); + createEffect(() => { + if (archivedChecksResource()) { + setArchivedChecks(archivedChecksResource()!); + } + }); + // Actions const addNewCheck = async (check: CreateCheckRequest) => { setActionInProgress(true); try { await addCheck(check); - await refetch(); + await refetchChecks(); } catch (e) { console.error("Failed to add new check", e); throw e; @@ -49,20 +70,42 @@ const eligibilityCheckResource = (): EligibilityCheckResource => { setActionInProgress(true); try { await archiveCheck(checkIdToRemove); - await refetch(); + await Promise.all([refetchChecks(), refetchArchivedChecks()]); + 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 Promise.all([refetchChecks(), refetchArchivedChecks()]); + 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, - error: () => checksResource.error, + loading: () => checksResource.loading || archivedChecksResource.loading, + error: () => checksResource.error ?? archivedChecksResource.error, }, }; }; 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.
+
From c08b9ab8348c3e366cb1b89494e8bdeb77cd7cf4 Mon Sep 17 00:00:00 2001 From: Preston Cabe Date: Mon, 7 Sep 2026 21:04:02 -0400 Subject: [PATCH 3/5] Do not read an errored check resource inside the sync effects Reading a failed Solid resource rethrows, so a failed fetch threw out of createEffect. The home screen has no ErrorBoundary, so a failure of the archived list took down a page whose active list had loaded fine. Check .error before touching the accessor, and cover it with a test that fails (exit 1 on unhandled errors) without the guard. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_013kjK519eM76gGhLAB8dyZa --- .../eligibilityCheckResource.test.ts | 24 +++++++++++++++++++ .../eligibilityCheckResource.ts | 15 ++++++++---- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/builder-frontend/src/components/homeScreen/eligibilityCheckList/eligibilityCheckResource.test.ts b/builder-frontend/src/components/homeScreen/eligibilityCheckList/eligibilityCheckResource.test.ts index 7a7e3fef..87ad42ef 100644 --- a/builder-frontend/src/components/homeScreen/eligibilityCheckList/eligibilityCheckResource.test.ts +++ b/builder-frontend/src/components/homeScreen/eligibilityCheckList/eligibilityCheckResource.test.ts @@ -48,6 +48,30 @@ describe("eligibilityCheckResource", () => { }); }); + 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 both active and archived lists", async () => { await new Promise((resolve, reject) => { createRoot((dispose) => { diff --git a/builder-frontend/src/components/homeScreen/eligibilityCheckList/eligibilityCheckResource.ts b/builder-frontend/src/components/homeScreen/eligibilityCheckList/eligibilityCheckResource.ts index 52baa027..aa8df795 100644 --- a/builder-frontend/src/components/homeScreen/eligibilityCheckList/eligibilityCheckResource.ts +++ b/builder-frontend/src/components/homeScreen/eligibilityCheckList/eligibilityCheckResource.ts @@ -39,16 +39,21 @@ const eligibilityCheckResource = (): EligibilityCheckResource => { [], ); - // When resource resolves, sync it into the store + // When resource resolves, sync it into the store. 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); } }); createEffect(() => { - if (archivedChecksResource()) { - setArchivedChecks(archivedChecksResource()!); + if (archivedChecksResource.error) return; + const loadedArchivedChecks = archivedChecksResource(); + if (loadedArchivedChecks) { + setArchivedChecks(loadedArchivedChecks); } }); From 14fb175b8eb0c050909fa5b79222374b3109e4fc Mon Sep 17 00:00:00 2001 From: Preston Cabe Date: Mon, 7 Sep 2026 21:06:08 -0400 Subject: [PATCH 4/5] Serve the active and archived check lists from one read The home screen fetched active and archived checks separately, but both come from the same Firestore collection filtered on isArchived, so every page load ran the same query twice and deserialized the same documents twice. Replace the archived=true filter with includeArchived=true, which returns all working checks in one response, and split them in the frontend on the isArchived field that the entity already carries. fetchUserDefinedChecks now takes an options object. Its second positional parameter made it unsafe to hand to createResource, which calls fetchers as fetcher(source, info) -- the truthy info object would have landed in the archived slot and silently requested the wrong list. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_013kjK519eM76gGhLAB8dyZa --- .../controller/EligibilityCheckResource.java | 12 ++++--- .../EligibilityCheckRepository.java | 2 +- .../impl/EligibilityCheckRepositoryImpl.java | 11 +++--- .../EligibilityCheckResourceTest.java | 21 ++++++++--- builder-frontend/src/api/check.ts | 18 ++++++---- .../eligibilityCheckResource.test.ts | 36 +++++++++++++++++-- .../eligibilityCheckResource.ts | 30 +++++++--------- .../configureBenefit/ConfigureBenefit.tsx | 2 +- 8 files changed, 87 insertions(+), 45 deletions(-) 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 b6d5262e..8c8b7ce5 100644 --- a/builder-api/src/main/java/org/acme/controller/EligibilityCheckResource.java +++ b/builder-api/src/main/java/org/acme/controller/EligibilityCheckResource.java @@ -48,12 +48,14 @@ 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 @GET public Response getCustomChecks( @Context SecurityIdentity identity, @QueryParam("working") Boolean working, - @QueryParam("archived") Boolean archived + @QueryParam("includeArchived") Boolean includeArchived ) { String userId = AuthUtils.getUserId(identity); if (userId == null) { @@ -63,9 +65,9 @@ public Response getCustomChecks( List checks; if (Boolean.TRUE.equals(working)){ - if (Boolean.TRUE.equals(archived)) { - Log.info("Fetching archived custom checks. User: " + userId); - checks = eligibilityCheckRepository.getArchivedCustomChecks(userId); + 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); 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 4284b253..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,7 +10,7 @@ public interface EligibilityCheckRepository { List getWorkingCustomChecks(String userId); - List getArchivedCustomChecks(String userId); + List getAllWorkingCustomChecks(String userId); List getPublishedCheckVersions(EligibilityCheck workingCustomCheck) throws Exception; 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 585a86c1..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,19 +28,16 @@ public class EligibilityCheckRepositoryImpl implements EligibilityCheckRepositor private StorageService storageService; public List getWorkingCustomChecks(String userId){ - return getWorkingCustomChecksByArchivedStatus(userId, false); - } - - public List getArchivedCustomChecks(String userId){ - return getWorkingCustomChecksByArchivedStatus(userId, true); + return getAllWorkingCustomChecks(userId).stream() + .filter(check -> !check.getIsArchived()) + .toList(); } - private List getWorkingCustomChecksByArchivedStatus(String userId, boolean archived){ + 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() == archived) .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 7f5061da..59a4bf1b 100644 --- a/builder-api/src/test/java/org/acme/controller/EligibilityCheckResourceTest.java +++ b/builder-api/src/test/java/org/acme/controller/EligibilityCheckResourceTest.java @@ -228,20 +228,33 @@ private CreateCheckRequest createCheckRequest() { } @Test - void getCustomChecksCanListArchivedChecks() { + void getCustomChecksCanIncludeArchivedChecksInOneRead() { EligibilityCheck archivedCheck = new EligibilityCheck( "old-check", "income", "an old check", List.of(), USER_ID); archivedCheck.setIsArchived(true); - when(repository.getArchivedCustomChecks(USER_ID)).thenReturn(List.of(archivedCheck)); + 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(archivedCheck), response.getEntity()); - verify(repository).getArchivedCustomChecks(USER_ID); + 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 restoreCustomCheckMakesAnArchivedCheckActive() throws Exception { workingCheck.setIsArchived(true); diff --git a/builder-frontend/src/api/check.ts b/builder-frontend/src/api/check.ts index 38c5e51a..06b06495 100644 --- a/builder-frontend/src/api/check.ts +++ b/builder-frontend/src/api/check.ts @@ -151,14 +151,20 @@ export const validateCheckDmn = async ( } }; -export const fetchUserDefinedChecks = async ( - working: boolean, - archived = false, -): Promise => { +export const fetchUserDefinedChecks = async ({ + working, + includeArchived = false, +}: { + working: boolean; + includeArchived?: boolean; +}): Promise => { const workingQueryParam = working ? "true" : "false"; - const archivedQueryParam = archived ? "&archived=true" : ""; + const includeArchivedQueryParam = includeArchived + ? "&includeArchived=true" + : ""; const url = - apiUrl + `/custom-checks?working=${workingQueryParam}${archivedQueryParam}`; + apiUrl + + `/custom-checks?working=${workingQueryParam}${includeArchivedQueryParam}`; try { const response = await authGet(url); diff --git a/builder-frontend/src/components/homeScreen/eligibilityCheckList/eligibilityCheckResource.test.ts b/builder-frontend/src/components/homeScreen/eligibilityCheckList/eligibilityCheckResource.test.ts index 87ad42ef..1bb90312 100644 --- a/builder-frontend/src/components/homeScreen/eligibilityCheckList/eligibilityCheckResource.test.ts +++ b/builder-frontend/src/components/homeScreen/eligibilityCheckList/eligibilityCheckResource.test.ts @@ -14,6 +14,7 @@ vi.mock("solid-toast", () => ({ import { addCheck, fetchUserDefinedChecks, restoreCheck } from "@/api/check"; import eligibilityCheckResource from "./eligibilityCheckResource"; +import type { EligibilityCheck } from "@/types"; describe("eligibilityCheckResource", () => { beforeEach(() => vi.clearAllMocks()); @@ -48,6 +49,33 @@ 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"), @@ -72,7 +100,7 @@ describe("eligibilityCheckResource", () => { }); }); - it("restores a check and refreshes both active and archived lists", async () => { + it("restores a check and refreshes the check list", async () => { await new Promise((resolve, reject) => { createRoot((dispose) => { const resource = eligibilityCheckResource(); @@ -81,8 +109,10 @@ describe("eligibilityCheckResource", () => { .then(() => { try { expect(restoreCheck).toHaveBeenCalledWith("archived-check-id"); - expect(fetchUserDefinedChecks).toHaveBeenCalledWith(true); - expect(fetchUserDefinedChecks).toHaveBeenCalledWith(true, true); + expect(fetchUserDefinedChecks).toHaveBeenCalledWith({ + working: true, + includeArchived: true, + }); expect(resource.actionInProgress()).toBe(false); resolve(); } catch (assertionError) { diff --git a/builder-frontend/src/components/homeScreen/eligibilityCheckList/eligibilityCheckResource.ts b/builder-frontend/src/components/homeScreen/eligibilityCheckList/eligibilityCheckResource.ts index aa8df795..f0af54e7 100644 --- a/builder-frontend/src/components/homeScreen/eligibilityCheckList/eligibilityCheckResource.ts +++ b/builder-frontend/src/components/homeScreen/eligibilityCheckList/eligibilityCheckResource.ts @@ -26,11 +26,11 @@ export interface EligibilityCheckResource { } const eligibilityCheckResource = (): EligibilityCheckResource => { + // 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(true), + fetchUserDefinedChecks({ working: true, includeArchived: true }), ); - const [archivedChecksResource, { refetch: refetchArchivedChecks }] = - createResource(() => fetchUserDefinedChecks(true, true)); const [actionInProgress, setActionInProgress] = createSignal(false); // Local fine-grained store @@ -39,21 +39,15 @@ const eligibilityCheckResource = (): EligibilityCheckResource => { [], ); - // When resource resolves, sync it into the store. Reading an errored - // resource rethrows, so check for failure before touching the accessor. + // 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.error) return; const loadedChecks = checksResource(); if (loadedChecks) { - setChecks(loadedChecks); - } - }); - - createEffect(() => { - if (archivedChecksResource.error) return; - const loadedArchivedChecks = archivedChecksResource(); - if (loadedArchivedChecks) { - setArchivedChecks(loadedArchivedChecks); + setChecks(loadedChecks.filter((check) => !check.isArchived)); + setArchivedChecks(loadedChecks.filter((check) => check.isArchived)); } }); @@ -75,7 +69,7 @@ const eligibilityCheckResource = (): EligibilityCheckResource => { setActionInProgress(true); try { await archiveCheck(checkIdToRemove); - await Promise.all([refetchChecks(), refetchArchivedChecks()]); + await refetchChecks(); toast.success("Check archived."); } catch (e) { console.error("Failed to archive check", e); @@ -89,7 +83,7 @@ const eligibilityCheckResource = (): EligibilityCheckResource => { setActionInProgress(true); try { await restoreCheck(checkIdToRestore); - await Promise.all([refetchChecks(), refetchArchivedChecks()]); + await refetchChecks(); toast.success("Check restored."); } catch (e) { console.error("Failed to restore check", e); @@ -109,8 +103,8 @@ const eligibilityCheckResource = (): EligibilityCheckResource => { }, actionInProgress, initialLoadStatus: { - loading: () => checksResource.loading || archivedChecksResource.loading, - error: () => checksResource.error ?? archivedChecksResource.error, + loading: () => checksResource.loading, + error: () => checksResource.error, }, }; }; diff --git a/builder-frontend/src/components/project/manageBenefits/configureBenefit/ConfigureBenefit.tsx b/builder-frontend/src/components/project/manageBenefits/configureBenefit/ConfigureBenefit.tsx index 57130819..42f82233 100644 --- a/builder-frontend/src/components/project/manageBenefits/configureBenefit/ConfigureBenefit.tsx +++ b/builder-frontend/src/components/project/manageBenefits/configureBenefit/ConfigureBenefit.tsx @@ -27,7 +27,7 @@ const ConfigureBenefit = ({ createSignal("public"); const [publicChecks] = createResource(fetchPublicChecks); const [userDefinedChecks] = createResource(() => - fetchUserDefinedChecks(false), + fetchUserDefinedChecks({ working: false }), ); const onRemoveEligibilityCheck = (checkId: string) => { From f2ce81259ca5619d706bc894732c96560ef29b88 Mon Sep 17 00:00:00 2001 From: Preston Cabe Date: Mon, 7 Sep 2026 21:06:45 -0400 Subject: [PATCH 5/5] Reject includeArchived on a published check request includeArchived was only honored inside the working=true branch. A request for archived checks without it fell through to the published list, which by construction excludes archived checks -- the caller got the opposite of what it asked for, with a 200 and no signal. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_013kjK519eM76gGhLAB8dyZa --- .../org/acme/controller/EligibilityCheckResource.java | 10 +++++++++- .../acme/controller/EligibilityCheckResourceTest.java | 10 ++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) 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 8c8b7ce5..636baa23 100644 --- a/builder-api/src/main/java/org/acme/controller/EligibilityCheckResource.java +++ b/builder-api/src/main/java/org/acme/controller/EligibilityCheckResource.java @@ -50,7 +50,9 @@ public class EligibilityCheckResource { // If the query parameter 'working' is set to true, // 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 + // 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, @@ -62,6 +64,12 @@ public Response getCustomChecks( 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 (Boolean.TRUE.equals(working)){ 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 59a4bf1b..8adc1d6a 100644 --- a/builder-api/src/test/java/org/acme/controller/EligibilityCheckResourceTest.java +++ b/builder-api/src/test/java/org/acme/controller/EligibilityCheckResourceTest.java @@ -255,6 +255,16 @@ void getCustomChecksOmitsArchivedChecksByDefault() { 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);