Fix null product query parameters

src/components/session/token/SessionUser/Objects/ObjectsGlobal.vue

@@ -32,6 +32,23 @@ const normalizeDateTimeLocalValue = (value) => {
   return `${datePart}T${timePart.slice(0, 5)}`;
 };
 
+const sanitizeQueryParams = (params = {}) => {
+  if (!params || typeof params !== 'object') {
+    return {};
+  }
+
+  return Object.fromEntries(
+    Object.entries(params).filter(([, value]) => value !== null && value !== undefined)
+  );
+};
+
+const buildQueryString = (params = {}) => {
+  const queryParams = sanitizeQueryParams(params);
+  const queryString = new URLSearchParams(queryParams).toString();
+
+  return queryString ? `?${queryString}` : '';
+};
+
 /**
  * The Global Objects object, which contains global functions for objects.
  */
@@ -431,20 +448,15 @@ export const ObjectsGlobal = {
      * @returns {Promise} The promise
      */
     object: async (endpoint, id, optionsObject = {}) => {
-      const cacheKey = JSON.stringify({endpoint, id, optionsObject});
+      const requestParams = typeof id === 'object' && id !== null
+        ? sanitizeQueryParams(id)
+        : {id};
+      const cacheKey = JSON.stringify({endpoint, id: requestParams, optionsObject});
       if (ObjectsGlobal.cache.has(cacheKey)) {
         return ObjectsGlobal.cache.get(cacheKey);
       }
 
-      let requestEndpoint = endpoint;
+      const requestEndpoint = `${endpoint}${buildQueryString(requestParams)}`;
-      if (typeof id === 'object') {
-        // If the id is an object, we need to convert it to a query string
-        const params = new URLSearchParams(id).toString();
-        requestEndpoint = `${endpoint}?${params}`;
-      } else {
-        // If the id is a number, we can just append it to the endpoint
-        requestEndpoint = `${endpoint}?id=${id}`;
-      }
       let options = ObjectsGlobal.requestOptions.applyOptions(optionsObject);
       let promise;
       if (options.authenticated) {
@@ -499,23 +511,24 @@ export const ObjectsGlobal = {
      * @returns {Promise} The promise
      */
     objects: async (endpoint, data = {}) => {
-      const cacheKey = JSON.stringify({endpoint, data});
+      const requestData = sanitizeQueryParams(data);
-      if (ObjectsGlobal.cache.has(cacheKey)) {
+      if (requestData.filters && typeof requestData.filters === 'object') {
-        return ObjectsGlobal.cache.get(cacheKey);
+        requestData.filters = Object.entries(requestData.filters).flatMap(([key, value]) => {
-      }
-
-      if (data.filters && typeof data.filters === 'object') {
-        data.filters = Object.entries(data.filters).flatMap(([key, value]) => {
           if (Array.isArray(value)) {
             return value.map(v => `${key}:${v}`);
           }
           return `${key}:${value}`;
         }).join(',');
       }
+      const cacheKey = JSON.stringify({endpoint, data: requestData});
+      if (ObjectsGlobal.cache.has(cacheKey)) {
+        return ObjectsGlobal.cache.get(cacheKey);
+      }
+
       const promise = authenticatedRequest(
           endpoint,
           "GET",
-          data
+          requestData
       ).then((response) => {
         return response.data.data;
       }).catch((error) => {

src/components/session/token/SessionUser/Objects/Products.vue

@@ -8,6 +8,17 @@ import i18n from '@/i18n';
 
 const t = (key) => i18n.global.t(key);
 
+const normalizeProductQueryOptions = (options = {}) => {
+  const queryOptions = { ...options };
+  if (!Object.prototype.hasOwnProperty.call(queryOptions, "final_price")) {
+    queryOptions.final_price = false;
+  }
+
+  return Object.fromEntries(
+    Object.entries(queryOptions).filter(([, value]) => value !== null && value !== undefined)
+  );
+};
+
 /**
  * Local products cache
  * @type {ref<null>}
@@ -269,19 +280,19 @@ export const Products = {
     },
   },
   get: {
-    all: async (options = {department_id: null, customer_id: null, category_id: null, final_price: false}) => {
+    all: async (options = {}) => {
-      return ObjectsGlobal.get.objects(Products.meta.endpoint, {...options});
+      return ObjectsGlobal.get.objects(Products.meta.endpoint, normalizeProductQueryOptions(options));
     },
-    single: async (id, options = {department_id: null, customer_id: null, category_id: null, final_price: false}) => {
+    single: async (id, options = {}) => {
-      return ObjectsGlobal.get.object(Products.meta.endpoint, {id: id, ...options});
+      return ObjectsGlobal.get.object(Products.meta.endpoint, {id: id, ...normalizeProductQueryOptions(options)});
     },
     category: async (category_id, department_id, customer_id = null, final_price = false) => {
-      return ObjectsGlobal.get.object(Products.meta.endpoint, {
+      return ObjectsGlobal.get.object(Products.meta.endpoint, normalizeProductQueryOptions({
         category: category_id,
         department_id: department_id,
         ...(customer_id !== null ? {customer_id: customer_id} : {}),
         ...{final_price: final_price}
-      });
+      }));
     },
   },
   delete: async (id) => {
@@ -324,4 +335,4 @@ export const Products = {
     );
   },
 };
-</script>
+</script>

tests/e2e/superuser-products-layout.spec.ts

@@ -15,6 +15,7 @@ type ProductRouteOptions = {
   categories?: unknown;
   productOptions?: unknown;
   economicProducts?: unknown;
+  onProductsRequest?: (url: URL) => void;
 };
 
 const collectBrowserErrors = (page: Page) => {
@@ -42,6 +43,7 @@ const stubSuperuserProductRoutes = async (
     categories = [],
     productOptions = [],
     economicProducts = [],
+    onProductsRequest,
   }: ProductRouteOptions
 ) => {
   await seedAuthenticatedState(page, token);
@@ -154,6 +156,7 @@ const stubSuperuserProductRoutes = async (
     }
 
     if (pathname.endsWith("/products") && method === "GET") {
+      onProductsRequest?.(url);
       const perPage = Number(url.searchParams.get("limit") || "100");
 
       await route.fulfill(
@@ -273,7 +276,14 @@ test.describe("Superuser products layout", () => {
     ];
 
     await page.setViewportSize({ width: 1280, height: 720 });
-    await stubSuperuserProductRoutes(page, { products, categories, productOptions, economicProducts });
+    const productRequestUrls: string[] = [];
+    await stubSuperuserProductRoutes(page, {
+      products,
+      categories,
+      productOptions,
+      economicProducts,
+      onProductsRequest: (url) => productRequestUrls.push(url.toString()),
+    });
 
     await page.goto("/superuser/products");
 
@@ -316,6 +326,16 @@ test.describe("Superuser products layout", () => {
     await expect(page.getByTestId("superuser-product-name")).toContainText("Trækker");
     await expect(page.getByRole("heading", { name: "Trækker" })).toBeVisible();
 
+    const detailRequest = productRequestUrls
+      .map((url) => new URL(url))
+      .find((url) => url.searchParams.get("id") === "1");
+
+    expect(detailRequest).toBeDefined();
+    expect(detailRequest?.searchParams.get("department_id")).toBeNull();
+    expect(detailRequest?.searchParams.get("customer_id")).toBeNull();
+    expect(detailRequest?.searchParams.get("category_id")).toBeNull();
+    expect(detailRequest?.searchParams.get("final_price")).toBe("false");
+
     expect(pageErrors).toEqual([]);
     expect(consoleErrors).toEqual([]);
   });

tests/unit/objects-global-select-xss.spec.js

@@ -25,14 +25,125 @@ vi.mock("@/components/session/token/SessionUser/Objects/systemUserIds.js", () =>
   getSystemUserIds: () => [],
 }));
 
+vi.mock("@/components/session/token/SessionUser.vue", () => ({
+  SessionUser: {
+    objects: {
+      global: {
+        language: {
+          no_data: "No data",
+        },
+      },
+      products: {
+        get: {
+          all: vi.fn(),
+        },
+      },
+    },
+  },
+}));
+
 import Swal from "sweetalert2";
+import { authenticatedRequest } from "@/components/session/authenticatedRequest.vue";
 import { ObjectsGlobal } from "@/components/session/token/SessionUser/Objects/ObjectsGlobal.vue";
+import { Products } from "@/components/session/token/SessionUser/Objects/Products.vue";
 
 beforeEach(() => {
   vi.clearAllMocks();
+  ObjectsGlobal.clearCache();
   document.body.innerHTML = "";
 });
 
+describe("ObjectsGlobal query parameters", () => {
+  it("omits nullish object GET parameters while preserving false and zero", async () => {
+    authenticatedRequest.mockResolvedValueOnce({
+      data: {
+        data: {
+          id: 40,
+        },
+      },
+    });
+
+    await expect(
+      ObjectsGlobal.get.object("/products", {
+        id: 40,
+        department_id: null,
+        customer_id: undefined,
+        category_id: null,
+        final_price: false,
+        page: 0,
+      })
+    ).resolves.toEqual({
+      id: 40,
+    });
+
+    expect(authenticatedRequest).toHaveBeenCalledWith("/products?id=40&final_price=false&page=0", "GET");
+  });
+
+  it("omits nullish object list parameters while preserving false", async () => {
+    authenticatedRequest.mockResolvedValueOnce({
+      data: {
+        data: [],
+      },
+    });
+
+    await expect(
+      ObjectsGlobal.get.objects("/products", {
+        department_id: null,
+        customer_id: undefined,
+        final_price: false,
+      })
+    ).resolves.toEqual([]);
+
+    expect(authenticatedRequest).toHaveBeenCalledWith("/products", "GET", {
+      final_price: false,
+    });
+  });
+});
+
+describe("Products query parameters", () => {
+  it("omits nullish default pricing parameters for single product GETs", async () => {
+    authenticatedRequest.mockResolvedValueOnce({
+      data: {
+        data: {
+          id: 40,
+        },
+      },
+    });
+
+    await expect(Products.get.single(40)).resolves.toEqual({
+      id: 40,
+    });
+
+    expect(authenticatedRequest).toHaveBeenCalledWith("/products?id=40&final_price=false", "GET");
+  });
+
+  it("preserves explicit product pricing parameters", async () => {
+    authenticatedRequest.mockResolvedValueOnce({
+      data: {
+        data: {
+          id: 40,
+        },
+      },
+    });
+
+    await expect(
+      Products.get.single(40, {
+        department_id: 2,
+        customer_id: 12345,
+        category_id: 8,
+        final_price: true,
+      })
+    ).resolves.toEqual({
+      id: 40,
+    });
+
+    expect(authenticatedRequest).toHaveBeenCalledWith(
+      "/products?id=40&department_id=2&customer_id=12345&category_id=8&final_price=true",
+      "GET"
+    );
+  });
+});
+
 describe("ObjectsGlobal select editor escaping", () => {
   it("escapes option ids and names before rendering SweetAlert HTML", async () => {
     const object = {