fix(sources): Fix broken cases in omission of 'owner' property

CartoDB · Aug 22, 2024 · afeef42 · afeef42
1 parent 4a67dee
commit afeef42
Show file tree

Hide file tree

Showing 3 changed files with 116 additions and 34 deletions.
diff --git a/src/types.ts b/src/types.ts
@@ -27,7 +27,7 @@ export type SpatialFilter = GeoJSON.Polygon | GeoJSON.MultiPolygon;
 
 /** @internalRemarks Source: @carto/react-api, @deck.gl/carto */
 export interface Filter {
-  [FilterType.IN]?: {owner?: string; values: number[]};
+  [FilterType.IN]?: {owner?: string; values: number[] | string[]};
   /** [a, b] both are included. */
   [FilterType.BETWEEN]?: {owner?: string; values: number[][]};
   /** [a, b) a is included, b is not. */

diff --git a/src/utils.ts b/src/utils.ts
@@ -23,7 +23,8 @@ export function getApplicableFilters(
       if (!isFilterType(type)) continue;
 
       const filter = filters[column][type];
-      if (filter && filter.owner !== owner) {
+      const isApplicable = !owner || !filter?.owner || filter?.owner !== owner;
+      if (filter && isApplicable) {
         applicableFilters[column] ||= {};
         applicableFilters[column][type] = filter as $TODO;
       }

diff --git a/test/sources/widget-base-source.test.ts b/test/sources/widget-base-source.test.ts
@@ -1,7 +1,6 @@
 import {afterEach, expect, test, vi} from 'vitest';
 import {
   FilterType,
-  MapType,
   WidgetBaseSource,
   WidgetBaseSourceProps,
 } from '@carto/api-client';
@@ -15,9 +14,11 @@ class WidgetTestSource extends WidgetBaseSource<WidgetBaseSourceProps> {
   protected override getModelSource(owner: string) {
     return {
       ...super._getModelSource(owner),
-      type: 'test' as MapType,
+      type: 'test',
       data: 'test-data',
-    };
+    } as unknown as ReturnType<
+      WidgetBaseSource<WidgetBaseSourceProps>['getModelSource']
+    >;
   }
 }
 
@@ -89,58 +90,138 @@ test('getCategories', async () => {
  * filters
  */
 
-test('filters', async () => {
+test('filters - global', async () => {
+  // Filters without 'owners' must affect API calls with or without
+  // a 'filterOwner' assigned. When 'filterOwner' is null, undefined,
+  // or an empty string, all filters are applied.
+
   const widgetSource = new WidgetTestSource({
     accessToken: '<token>',
     connectionName: 'carto_dw',
-    filters: {
-      store_type: {
-        [FilterType.IN]: {
-          owner: 'a',
-          values: [1, 2, 3],
-        },
+    filters: {country: {[FilterType.IN]: {values: ['Spain']}}},
+  });
+
+  const mockFetch = vi
+    .fn()
+    .mockResolvedValue(createMockResponse({rows: [{value: 123}]}));
+
+  vi.stubGlobal('fetch', mockFetch);
+
+  const expectedParams = {
+    filters: JSON.stringify({country: {in: {values: ['Spain']}}}),
+    filtersLogicalOperator: 'and',
+  };
+
+  expect(await getActualParams('mywidget')).toMatchObject(expectedParams);
+  expect(await getActualParams(null)).toMatchObject(expectedParams);
+  expect(await getActualParams(undefined)).toMatchObject(expectedParams);
+  expect(await getActualParams('')).toMatchObject(expectedParams);
+
+  async function getActualParams(
+    filterOwner: string | null | undefined
+  ): Promise<Record<string, string>> {
+    await widgetSource.getFormula({
+      column: 'store_name',
+      operation: 'count',
+      filterOwner,
+    });
+    const params = new URL(mockFetch.mock.lastCall[0]).searchParams.entries();
+    return Object.fromEntries(params);
+  }
+});
+
+test('filters - owner', async () => {
+  // Filters with 'owner' must affect all API calls *except*
+  // those with a matching 'filterOwner'. When 'owner' is null,
+  // undefined, or an empty string, it affects all calls.
+
+  const filters = {
+    a: {
+      [FilterType.IN]: {
+        owner: 'a',
+        values: [1, 2, 3],
       },
-      country: {
-        [FilterType.IN]: {
-          owner: 'b',
-          values: [4, 5, 6],
-        },
+    },
+    b: {
+      [FilterType.IN]: {
+        owner: 'b',
+        values: [4, 5, 6],
       },
     },
+    c: {
+      [FilterType.IN]: {
+        owner: null,
+        values: [7, 8, 9],
+      },
+    },
+    d: {
+      [FilterType.IN]: {
+        owner: undefined,
+        values: [10, 11, 12],
+      },
+    },
+    e: {
+      [FilterType.IN]: {
+        owner: '',
+        values: [13, 14, 15],
+      },
+    },
+  };
+
+  const widgetSource = new WidgetTestSource({
+    accessToken: '<token>',
+    connectionName: 'carto_dw',
+    filters,
   });
 
   const mockFetch = vi
     .fn()
     .mockResolvedValue(createMockResponse({rows: [{value: 123}]}));
+
   vi.stubGlobal('fetch', mockFetch);
 
-  await widgetSource.getFormula({
-    filterOwner: 'a',
-    column: 'store_type',
-    operation: 'count',
+  expect(await getActualParams('a')).toMatchObject({
+    filters: JSON.stringify({
+      ...filters,
+      a: undefined,
+    }),
+    filtersLogicalOperator: 'and',
   });
 
-  expect(mockFetch).toHaveBeenCalledOnce();
+  expect(await getActualParams('b')).toMatchObject({
+    filters: JSON.stringify({
+      ...filters,
+      b: undefined,
+    }),
+    filtersLogicalOperator: 'and',
+  });
 
-  let params = new URL(mockFetch.mock.lastCall[0]).searchParams.entries();
-  expect(Object.fromEntries(params)).toMatchObject({
-    filters: JSON.stringify({country: {in: {owner: 'b', values: [4, 5, 6]}}}),
+  expect(await getActualParams('c')).toMatchObject({
+    filters: JSON.stringify(filters),
     filtersLogicalOperator: 'and',
   });
 
-  await widgetSource.getFormula({
-    column: 'store_type',
-    operation: 'count',
+  expect(await getActualParams('d')).toMatchObject({
+    filters: JSON.stringify(filters),
+    filtersLogicalOperator: 'and',
   });
 
-  params = new URL(mockFetch.mock.lastCall[0]).searchParams.entries();
-  expect(Object.fromEntries(params)).toMatchObject({
-    filters: JSON.stringify({
-      store_type: {in: {owner: 'a', values: [1, 2, 3]}},
-      country: {in: {owner: 'b', values: [4, 5, 6]}},
-    }),
+  expect(await getActualParams('e')).toMatchObject({
+    filters: JSON.stringify(filters),
     filtersLogicalOperator: 'and',
   });
+
+  async function getActualParams(
+    filterOwner: string | null | undefined
+  ): Promise<Record<string, string>> {
+    await widgetSource.getFormula({
+      column: 'store_name',
+      operation: 'count',
+      filterOwner,
+    });
+    const params = new URL(mockFetch.mock.lastCall[0]).searchParams.entries();
+    return Object.fromEntries(params);
+  }
 });
 
 /******************************************************************************