bito-code-review[bot] commented on code in PR #37260:
URL: https://github.com/apache/superset/pull/37260#discussion_r2737379164


##########
superset-frontend/src/dashboard/components/nativeFilters/selectors.test.ts:
##########
@@ -142,3 +147,421 @@ describe('getCrossFilterIndicator', () => {
     });
   });
 });
+
+test('extractLabel returns label when filter has label and it does not include 
undefined', () => {
+  expect(extractLabel({ label: 'My Label', value: 'x' })).toBe('My Label');
+  expect(extractLabel({ label: 'Only label' })).toBe('Only label');
+});
+
+test('extractLabel returns value joined by ", " when filter has non-empty 
array value and no label', () => {
+  expect(extractLabel({ value: ['a', 'b'] })).toBe('a, b');
+  expect(extractLabel({ value: ['single'] })).toBe('single');
+});
+
+test('extractLabel returns value when filter has non-array value 
(ensureIsArray wraps it)', () => {
+  expect(extractLabel({ value: 'scalar' })).toBe('scalar');
+  expect(extractLabel({ value: 42 })).toBe('42');
+});
+
+test('extractLabel returns null when filter is undefined or has no label and 
no value', () => {
+  expect(extractLabel(undefined)).toBe(null);
+  expect(extractLabel({})).toBe(null);
+  expect(extractLabel({ label: '' })).toBe(null);
+});
+
+test('extractLabel returns null when filter.value is null or undefined', () => 
{
+  expect(extractLabel({ value: null })).toBe(null);
+  expect(extractLabel({ value: undefined })).toBe(null);
+});
+
+test('extractLabel does not return ", " or "null, null" for arrays of only 
null, undefined, or empty string', () => {
+  expect(extractLabel({ value: [null, null] })).toBe(null);
+  expect(extractLabel({ value: [null] })).toBe(null);
+  expect(extractLabel({ value: [''] })).toBe(null);
+  expect(extractLabel({ value: ['', ''] })).toBe(null);
+  expect(extractLabel({ value: [null, ''] })).toBe(null);
+  expect(extractLabel({ value: [undefined, undefined] })).toBe(null);
+  expect(extractLabel({ value: [null, undefined, ''] })).toBe(null);
+  expect(extractLabel({ value: [null, null] })).not.toBe(', ');
+  expect(extractLabel({ value: [null, ''] })).not.toBe(', ');
+});
+
+test('extractLabel returns only non-empty items when array has mix of empty 
and non-empty', () => {
+  expect(extractLabel({ value: [null, 'a', '', 'b', undefined] })).toBe('a, 
b');
+  expect(extractLabel({ value: ['', 'x', ''] })).toBe('x');
+});
+
+test('extractLabel uses value when label is undefined', () => {
+  expect(extractLabel({ label: undefined, value: ['a'] })).toBe('a');
+});
+
+test('getAppliedColumnsWithFallback returns columns from query response when 
available', () => {
+  const chart = {
+    queriesResponse: [
+      {
+        applied_filters: [{ column: 'age' }, { column: 'name' }],
+      },
+    ],
+  };
+  const result = getAppliedColumnsWithFallback(chart);
+  expect(result).toEqual(new Set(['age', 'name']));
+});
+
+test('getAppliedColumnsWithFallback returns empty set when query response has 
no applied_filters and no fallback params', () => {
+  const chart = {
+    queriesResponse: [{ applied_filters: [] }],
+  };
+  const result = getAppliedColumnsWithFallback(chart);
+  expect(result).toEqual(new Set());
+});
+
+test('getAppliedColumnsWithFallback derives columns from native filters when 
query response is empty', () => {
+  const chart = {
+    queriesResponse: [{ applied_filters: [] }],
+  };
+  const nativeFilters = {
+    filter1: {
+      id: 'filter1',
+      type: NativeFilterType.NativeFilter,
+      chartsInScope: [123],
+      targets: [{ column: { name: 'age' } }],
+    },
+    filter2: {
+      id: 'filter2',
+      type: NativeFilterType.NativeFilter,
+      chartsInScope: [123],
+      targets: [{ column: { name: 'name' } }],
+    },
+  } as any;
+  const dataMask = {
+    filter1: {
+      id: 'filter1',
+      filterState: { value: '25' },
+      extraFormData: {},
+    },
+    filter2: {
+      id: 'filter2',
+      filterState: { value: 'John' },
+      extraFormData: {},
+    },
+  } as any;
+  const result = getAppliedColumnsWithFallback(
+    chart,
+    nativeFilters,
+    dataMask,
+    123,
+  );
+  expect(result).toEqual(new Set(['age', 'name']));
+});

Review Comment:
   <!-- Bito Reply -->
   Understood. Removing tests for `getStatus` since the function isn't modified 
helps keep the PR focused and avoids unnecessary LOC.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to