🐛 #701 - fix: fix a bug that prevented users from adding additional zaaktypen to a destruction list

Author: svenvandescheur

backend/src/openarchiefbeheer/destruction/tests/e2e/features/test_feature_list_create.py

@@ -109,6 +109,8 @@ def create_data():
                     }
                 }, 
             )
+
+            # This zaaktype should not be shown as its only zaak gets assigned to a destruction list.
             zaak = ZaakFactory.create(
                 identificatie="ZAAK-111-2",
                 post___expand={

frontend/src/hooks/useFields.tsx

@@ -38,11 +38,20 @@ type FilterTransformReturnType<T> = Record<
 
 /**
  * Hook resolving the base fields for lists.
+ * s
+ * @param [destructionList] - The destruction list to return fields for.
+ * @param [review] - The destruction list to return fields for.
+ * @param [extraFields] - Additional fields to add.
+ * @param [restrictFilterChoices="list"] - Allows restricting choices in filters
+ *  to either:
+ *    - The destruction list or review (`list`).
+ *    - To zaken not already assigned to a destruction list (`unassigned`).
  */
 export function useFields<T extends Zaak = Zaak>(
   destructionList?: DestructionList,
   review?: Review,
   extraFields?: TypedField<T>[],
+  restrictFilterChoices: "list" | "unassigned" | false = "list",
 ): [
   TypedField<T>[],
   (fields: TypedField<T>[]) => void,
@@ -61,11 +70,27 @@ export function useFields<T extends Zaak = Zaak>(
   }, []);
   const [searchParams, setSearchParams] = useSearchParams();
   const selectielijstKlasseChoices = useSelectielijstKlasseChoices();
-  const zaaktypeChoices = useZaaktypeChoices(
-    destructionList,
-    review,
-    searchParams,
-  );
+
+  // Fetch the zaaktype choices, if `restrictFilterChoices==="list"`:  only  zaaktype
+  // choices of zaken belonging to either the destruction list or review  are loaded.
+  // If `restrictFilterChoices==="unassigned"`: only zaaktype choices belonging to
+  // zaken not assigned to any destruction list are loaded. If `restrictFilterChoices==="false"`:
+  // all zaaktype choices are loaded.
+  const zaaktypeParams = new URLSearchParams(searchParams);
+  zaaktypeParams.delete("zaaktype");
+
+  if (restrictFilterChoices === "list") {
+    if (destructionList) {
+      zaaktypeParams.set("inDestructionList", destructionList.uuid);
+    }
+    if (review?.pk) {
+      zaaktypeParams.set("inReview", review.pk.toString());
+    }
+  }
+  if (restrictFilterChoices === "unassigned") {
+    zaaktypeParams.set("not_in_destruction_list", "true");
+  }
+  const zaaktypeChoices = useZaaktypeChoices(zaaktypeParams, false, null);
 
   // The raw, unfiltered configuration of the available base fields.
   // Both filterLookup AND filterLookups will be used for clearing filters.
@@ -82,11 +107,26 @@ export function useFields<T extends Zaak = Zaak>(
       name: "zaaktype",
       filterLookup: "zaaktype",
       filterValue: searchParams.get("zaaktype") || "",
-      valueTransform: (v: ExpandZaak) =>
-        zaaktypeChoices.find(
-          (c) => c.value === v._expand?.zaaktype?.identificatie,
-        )?.label || <Placeholder />,
-      options: zaaktypeChoices,
+      valueTransform: (zaak) => {
+        const expandZaak = zaak as T & {
+          _expand: { zaaktype: { identificatie: string } };
+        };
+        const zaaktype = expandZaak._expand.zaaktype.identificatie;
+
+        // Zaaktype choices not yet loaded.
+        if (zaaktypeChoices === null) {
+          return <Placeholder />;
+        }
+
+        // Find label by zaaktype choice.
+        const zaaktypeChoice = zaaktypeChoices.find(
+          ({ value }) => value === zaaktype,
+        );
+
+        // Return label, or null.
+        return zaaktypeChoice?.label || zaaktype;
+      },
+      options: zaaktypeChoices || [],
       type: "string",
       width: "150px",
     },

frontend/src/hooks/useZaaktypeChoices.ts

@@ -1,41 +1,40 @@
 import { Option } from "@maykin-ui/admin-ui";
 import { useEffect, useState } from "react";
 
-import { DestructionList } from "../lib/api/destructionLists";
 import { listZaaktypeChoices } from "../lib/api/private";
-import { Review } from "../lib/api/review";
+import { params2CacheKey } from "../lib/format/params";
 import { useAlertOnError } from "./useAlertOnError";
 
 /**
  * Hook resolving zaaktype choices
- * @param destructionList
- * @param review
- * @param searchParams
+ * @param params
  * @param external
+ * @param initalData
  */
-export function useZaaktypeChoices(
-  destructionList?: DestructionList,
-  review?: Review,
-  searchParams?: URLSearchParams,
+export function useZaaktypeChoices<T = Option[]>(
+  params?: Parameters<typeof listZaaktypeChoices>[0],
   external = false,
-): Option[] {
+  initalData?: T,
+): T | Option[] {
+  const initial = typeof initalData === "undefined" ? [] : initalData;
   const alertOnError = useAlertOnError(
     "Er is een fout opgetreden bij het ophalen van zaaktypen!",
   );
 
-  const [zaaktypeChoicesState, setZaaktypeChoicesState] = useState<Option[]>(
-    [],
-  );
+  const [zaaktypeChoicesState, setZaaktypeChoicesState] = useState<
+    T | Option[]
+  >(initial);
+
   useEffect(() => {
-    listZaaktypeChoices(
-      destructionList?.uuid,
-      review?.pk,
-      searchParams,
-      external,
-    )
+    setZaaktypeChoicesState(initial);
+    const abortController = new AbortController();
+
+    listZaaktypeChoices(params, external, abortController.signal)
       .then((z) => setZaaktypeChoicesState(z))
       .catch(alertOnError);
-  }, [destructionList?.uuid, review?.pk, searchParams?.toString()]);
+
+    return () => abortController.abort();
+  }, [params2CacheKey(params || {}), external]);
 
   return zaaktypeChoicesState;
 }

frontend/src/lib/api/private.ts

@@ -2,9 +2,8 @@ import { Option } from "@maykin-ui/admin-ui";
 
 import { Zaak } from "../../types";
 import { cacheMemo } from "../cache/cache";
-import { DestructionList } from "./destructionLists";
+import { params2CacheKey, params2Object } from "../format/params";
 import { request } from "./request";
-import { Review } from "./review";
 
 /**
  * Retrieve informatieobjecttypen from Open Zaak and return a value and a label per informatieobjecttype. The label is
@@ -98,52 +97,56 @@ export async function listSelectielijstKlasseChoices(
 }
 
 /**
- * Retrieve zaaktypen from Open Zaak and return a value and a label per zaaktype. The label is the 'omschrijving' field
- * an the value is the URL. The response is cached for 15 minutes.
+ * Retrieve zaaktypen from Open Zaak and return a value and a label per zaaktype.
+ * The label is the 'omschrijving' field, and the value is the URL. The response is cached for 15 minutes.
+ * @param [params] - Additional search parameters for filtering (this keeps filters in sync with objects on page).
+ * @param [external=false] - Fetch zaaktypen from ZRC Service (Open Zaak) (slower/can't be combined with other filtering options).
+ * @param signal - Abort signal, should be called in cleanup function in React `useEffect()` hooks.
+ * @returns {Promise<Option[]>} A promise resolving to an array of options with `value` and `label`.
  */
 export async function listZaaktypeChoices(
-  destructionListUuid?: DestructionList["uuid"],
-  reviewPk?: Review["pk"],
-  searchParams?: URLSearchParams,
+  params?:
+    | URLSearchParams
+    | {
+        inReview: string;
+        inDestructionList: string;
+        notInDestructionList: boolean;
+      },
   external = false,
+  signal?: AbortSignal,
 ) {
-  const params = [destructionListUuid, reviewPk, searchParams]
-    .filter((param) => !!param)
-    .map((param) => String(param));
+  const cacheParams = params2CacheKey(params || {});
 
   return cacheMemo(
     "listZaaktypeChoices",
     async () => {
-      if (!searchParams) searchParams = new URLSearchParams();
-
-      const params = new URLSearchParams({
-        ...Object.fromEntries(searchParams),
-      });
-
-      if (reviewPk) {
-        params.set("inReview", String(reviewPk));
-      } else if (destructionListUuid) {
-        params.set("inDestructionList", destructionListUuid);
-      } else {
-        params.set("notInDestructionList", "true");
-      }
-
+      const data = params2Object(params || {});
       let response;
+
       if (external) {
         response = await request(
           "GET",
           "/_external-zaaktypen-choices/",
-          params,
+          data,
+          undefined,
+          undefined,
+          signal,
         );
       } else {
-        const filters = Object.fromEntries(params.entries());
-        response = await request("POST", "/_zaaktypen-choices/", {}, filters);
+        response = await request(
+          "POST",
+          "/_zaaktypen-choices/",
+          {},
+          data,
+          undefined,
+          signal,
+        );
       }
 
       const promise: Promise<Option[]> = response.json();
 
       return promise;
     },
-    external ? [...params, "external"] : params,
+    external ? [cacheParams, "external"] : [cacheParams],
   );
 }

frontend/src/lib/format/error.test.ts

@@ -1,6 +1,6 @@
 import { collectErrors } from "./error";
 
-describe("collectErrors", () => {
+describe("collectErrors()", () => {
   test("should return an array with a single string when the input is a string", () => {
     const errors = "This is an error";
     const result = collectErrors(errors);

frontend/src/lib/format/params.test.ts

@@ -0,0 +1,58 @@
+import {
+  entries2CacheKey,
+  params2CacheKey,
+  params2Entries,
+  params2Object,
+} from "./params";
+
+describe("params2Object()", () => {
+  test("should return correct object when URLSearchParams instance is passed", () => {
+    const obj = params2Object(new URLSearchParams("?foo=bar"));
+    expect(obj).toEqual({ foo: "bar" });
+  });
+
+  test("should return correct object when object is passed", () => {
+    const obj = params2Object({ foo: true, bar: 5 });
+    expect(obj).toEqual({ foo: "true", bar: "5" });
+  });
+});
+
+describe("params2Entries()", () => {
+  test("should return correct entries when URLSearchParams instance is passed", () => {
+    const obj = params2Entries(new URLSearchParams("?foo=bar&bar=baz"));
+    expect(obj).toEqual([
+      ["foo", "bar"],
+      ["bar", "baz"],
+    ]);
+  });
+
+  test("should return correct entries when object is passed", () => {
+    const obj = params2Entries({ foo: true, bar: 5 });
+    expect(obj).toEqual([
+      ["foo", "true"],
+      ["bar", "5"],
+    ]);
+  });
+});
+
+describe("params2CacheKey()", () => {
+  test("should return correct string when URLSearchParams instance is passed", () => {
+    const obj = params2CacheKey(new URLSearchParams("?foo=bar&bar=baz"));
+    expect(obj).toEqual("bar=baz:foo=bar");
+  });
+
+  test("should return correct object when object is passed", () => {
+    const obj = params2CacheKey({ foo: true, bar: 5 });
+    expect(obj).toEqual("bar=5:foo=true");
+  });
+});
+
+describe("entries2CacheKey()", () => {
+  test("should return correct string when entries are passed", () => {
+    const obj = entries2CacheKey([
+      ["foo", "bar"],
+      ["bar", "baz"],
+    ]);
+    expect(obj).toEqual("bar=baz:foo=bar");
+  });
+});

frontend/src/lib/format/params.ts

@@ -0,0 +1,42 @@
+/**
+ * Converts URLSearchParams/Record to object.
+ * @param params - URLSearchParams or an object.
+ */
+export function params2Object(
+  params: URLSearchParams | Record<string | number, unknown>,
+): Record<string, string> {
+  const entries = params2Entries(params);
+  return Object.fromEntries(entries);
+}
+/**
+ * Converts URLSearchParams/Record to cache key string.
+ * @param params - URLSearchParams or an object.
+ */
+export function params2CacheKey(
+  params: URLSearchParams | Record<string | number, unknown>,
+): string {
+  const entries = params2Entries(params);
+  return entries2CacheKey(entries);
+}
+
+/**
+ * Converts URLSearchParams/Record to entries array.
+ * @param params - URLSearchParams or an object.
+ */
+export function params2Entries(
+  params: URLSearchParams | Record<string | number, unknown>,
+): [string, string][] {
+  return params instanceof URLSearchParams
+    ? [...params.entries()]
+    : Object.entries(params || {}).map(([k, v]) => [String(k), String(v)]);
+}
+/**
+ * Converts entries array to cache key string;
+ * @param entries - An array of `[string, string]` entries.
+ */
+export function entries2CacheKey(entries: [string, string][]): string {
+  return entries
+    .map((entry) => entry.join("="))
+    .sort()
+    .join(":");
+}

frontend/src/lib/format/user.test.ts

@@ -1,7 +1,7 @@
 import { User } from "../api/auth";
 import { formatUser } from "./user";
 
-describe("formatUser", () => {
+describe("formatUser()", () => {
   test("should return an empty string if the user is null or undefined", () => {
     // @ts-expect-error - testing untyped behavior.
     expect(formatUser(null)).toBe("");

frontend/src/pages/destructionlist/abstract/BaseListView.tsx

@@ -54,6 +54,13 @@ export type BaseListViewProps<T extends Zaak = Zaak> = React.PropsWithChildren<{
   filterSelectionZaken?: ZaakSelectionZaakFilter;
   getSelectionDetail?: ZaakSelectionDetailGetter;
 
+  /**
+   * Allows restricting choices in filters to either:
+   *  - The destruction list or review (`list`).
+   *  - To zaken not already assigned to a destruction list (`unassigned`).
+   */
+  restrictFilterChoices?: false | "list" | "unassigned";
+
   dataGridProps?: Partial<DataGridProps<T>>;
 
   enableUseZaakSelection?: boolean;
@@ -81,6 +88,7 @@ export function BaseListView<T extends Zaak = Zaak>({
   extraFields,
   filterSelectionZaken,
   getSelectionDetail,
+  restrictFilterChoices,
 
   dataGridProps,
 
@@ -102,7 +110,7 @@ export function BaseListView<T extends Zaak = Zaak>({
 
   // Fields.
   const [fields, setFields, filterTransform, activeFilters, resetFilters] =
-    useFields<T>(destructionList, review, extraFields);
+    useFields<T>(destructionList, review, extraFields, restrictFilterChoices);
   type FilterTransformData = ReturnType<typeof filterTransform>;
 
   // Filter.