codeant-ai-for-open-source[bot] commented on code in PR #38470:
URL: https://github.com/apache/superset/pull/38470#discussion_r2935869261
##########
superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx:
##########
@@ -1249,3 +1284,399 @@ test('resets dependent filter to first item when value
does not exist in data',
);
});
});
+
+test('renders text input instead of dropdown when operatorType is ILIKE
contains', () => {
+ jest.useFakeTimers();
+ const setDataMaskMock = jest.fn();
+ const props = buildSelectFilterProps({
+ formData: { operatorType: SelectFilterOperatorType.Contains },
+ filterState: { value: undefined },
+ setDataMask: setDataMaskMock,
+ });
+
+ render(<SelectFilterPlugin {...props} />, {
+ useRedux: true,
+ initialState: {
+ nativeFilters: {
+ filters: { 'test-filter': { name: 'Test Filter' } },
+ },
+ dataMask: {
+ 'test-filter': {
+ extraFormData: {},
+ filterState: { value: undefined },
+ },
+ },
+ },
+ });
+
+ expect(screen.queryByRole('combobox')).not.toBeInTheDocument();
+ expect(
+ screen.getByPlaceholderText('Type to search (contains)...'),
+ ).toBeInTheDocument();
+});
+
+test('renders text input with starts-with placeholder', () => {
+ jest.useFakeTimers();
+ const setDataMaskMock = jest.fn();
+ const props = buildSelectFilterProps({
+ formData: { operatorType: SelectFilterOperatorType.StartsWith },
+ filterState: { value: undefined },
+ setDataMask: setDataMaskMock,
+ });
+
+ render(<SelectFilterPlugin {...props} />, {
+ useRedux: true,
+ initialState: {
+ nativeFilters: {
+ filters: { 'test-filter': { name: 'Test Filter' } },
+ },
+ dataMask: {
+ 'test-filter': {
+ extraFormData: {},
+ filterState: { value: undefined },
+ },
+ },
+ },
+ });
+
+ expect(
+ screen.getByPlaceholderText('Type to search (starts with)...'),
+ ).toBeInTheDocument();
+});
+
+test('typing in LIKE input calls setDataMask with ILIKE Contains payload',
async () => {
+ jest.useFakeTimers();
+ const setDataMaskMock = jest.fn();
+ const props = buildSelectFilterProps({
+ formData: { operatorType: SelectFilterOperatorType.Contains },
+ filterState: { value: undefined },
+ setDataMask: setDataMaskMock,
+ });
+
+ render(<SelectFilterPlugin {...props} />, {
+ useRedux: true,
+ initialState: {
+ nativeFilters: {
+ filters: { 'test-filter': { name: 'Test Filter' } },
+ },
+ dataMask: {
+ 'test-filter': {
+ extraFormData: {},
+ filterState: { value: undefined },
+ },
+ },
+ },
+ });
+
+ setDataMaskMock.mockClear();
+ const input = screen.getByPlaceholderText('Type to search (contains)...');
+ fireEvent.change(input, { target: { value: 'Jen' } });
+ act(() => {
+ jest.advanceTimersByTime(500);
+ });
+
+ expect(setDataMaskMock).toHaveBeenCalledWith(
+ expect.objectContaining({
+ extraFormData: {
+ filters: [
+ {
+ col: 'gender',
+ op: 'ILIKE',
+ val: '%Jen%',
+ },
+ ],
+ },
+ }),
+ );
+});
+
+test('typing in LIKE input with inverse selection calls setDataMask with NOT
ILIKE payload', async () => {
+ jest.useFakeTimers();
+ const setDataMaskMock = jest.fn();
+ const props = buildSelectFilterProps({
+ formData: {
+ operatorType: SelectFilterOperatorType.Contains,
+ inverseSelection: true,
+ },
+ filterState: { value: undefined },
+ setDataMask: setDataMaskMock,
+ });
+
+ render(<SelectFilterPlugin {...props} />, {
+ useRedux: true,
+ initialState: {
+ nativeFilters: {
+ filters: { 'test-filter': { name: 'Test Filter' } },
+ },
+ dataMask: {
+ 'test-filter': {
+ extraFormData: {},
+ filterState: { value: undefined },
+ },
+ },
+ },
+ });
+
+ setDataMaskMock.mockClear();
+ const input = screen.getByPlaceholderText('Type to search (contains)...');
+ fireEvent.change(input, { target: { value: 'Jen' } });
+ act(() => {
+ jest.advanceTimersByTime(500);
+ });
+
+ expect(setDataMaskMock).toHaveBeenCalledWith(
+ expect.objectContaining({
+ extraFormData: {
+ filters: [
+ {
+ col: 'gender',
+ op: 'NOT ILIKE',
+ val: '%Jen%',
+ },
+ ],
+ },
+ }),
+ );
+});
+
+test('clear-all resets LIKE input value and calls setDataMask with empty
state', async () => {
+ jest.useFakeTimers();
+ const setDataMaskMock = jest.fn();
+ const likeProps = buildSelectFilterProps({
+ formData: { operatorType: SelectFilterOperatorType.Contains },
+ filterState: { value: ['Jen'] },
+ setDataMask: setDataMaskMock,
+ });
+
+ const reduxState = {
+ useRedux: true,
+ initialState: {
+ nativeFilters: {
+ filters: { 'test-filter': { name: 'Test Filter' } },
+ },
+ dataMask: {
+ 'test-filter': {
+ extraFormData: {
+ filters: [{ col: 'gender', op: 'ILIKE', val: '%Jen%' }],
+ },
+ filterState: { value: ['Jen'] },
+ },
+ },
+ },
+ };
+
+ const { rerender } = render(
+ <SelectFilterPlugin {...likeProps} />,
+ reduxState,
+ );
+
+ const input = screen.getByPlaceholderText('Type to search (contains)...');
+ expect(input).toHaveValue('Jen');
+
+ setDataMaskMock.mockClear();
+
+ rerender(
+ <SelectFilterPlugin
+ {...likeProps}
+ clearAllTrigger={{ 'test-filter': true }}
+ />,
+ );
+
+ await waitFor(() => {
+ expect(input).toHaveValue('');
+ });
+
+ await waitFor(() => {
+ expect(setDataMaskMock).toHaveBeenCalledWith(
+ expect.objectContaining({
+ filterState: expect.objectContaining({
+ value: null,
+ }),
+ }),
+ );
+ });
+
+ act(() => {
+ jest.advanceTimersByTime(500);
+ });
+
+ expect(setDataMaskMock).not.toHaveBeenCalledWith(
+ expect.objectContaining({
+ extraFormData: {
+ filters: [
+ {
+ col: 'gender',
+ op: 'ILIKE',
+ val: '%Jen%',
+ },
+ ],
+ },
+ }),
+ );
+});
+
+test('pending LIKE debounce still applies after rerender recreates
updateDataMask', async () => {
+ jest.useFakeTimers();
+ const setDataMaskMock = jest.fn();
+ const likeProps = buildSelectFilterProps({
+ formData: { operatorType: SelectFilterOperatorType.Contains },
+ filterState: { value: undefined },
+ setDataMask: setDataMaskMock,
+ });
+
+ const reduxState = {
+ useRedux: true,
+ initialState: {
+ nativeFilters: {
+ filters: { 'test-filter': { name: 'Test Filter' } },
+ },
+ dataMask: {
+ 'test-filter': {
+ extraFormData: {},
+ filterState: { value: undefined },
+ },
+ },
+ },
+ };
+
+ const { rerender } = render(
+ <SelectFilterPlugin {...likeProps} />,
+ reduxState,
+ );
+
+ fireEvent.change(
+ screen.getByPlaceholderText('Type to search (contains)...'),
+ {
+ target: { value: 'Jen' },
+ },
+ );
+
+ setDataMaskMock.mockClear();
+
+ rerender(
+ <SelectFilterPlugin
+ {...buildSelectFilterProps({
+ formData: { operatorType: SelectFilterOperatorType.Contains },
+ filterState: { value: undefined, label: 'external change' },
+ setDataMask: setDataMaskMock,
+ })}
+ />,
+ );
+
+ act(() => {
+ jest.advanceTimersByTime(500);
+ });
+
+ expect(setDataMaskMock).toHaveBeenCalledWith(
+ expect.objectContaining({
+ extraFormData: {
+ filters: [
+ {
+ col: 'gender',
+ op: 'ILIKE',
+ val: '%Jen%',
+ },
+ ],
+ },
+ }),
+ );
+});
+
+test('pending LIKE debounce is canceled when operatorType switches back to
Exact', async () => {
+ jest.useFakeTimers();
+ const setDataMaskMock = jest.fn();
+ const likeProps = buildSelectFilterProps({
+ formData: { operatorType: SelectFilterOperatorType.Contains },
+ filterState: { value: undefined },
+ setDataMask: setDataMaskMock,
+ });
+
+ const reduxState = {
+ useRedux: true,
+ initialState: {
+ nativeFilters: {
+ filters: { 'test-filter': { name: 'Test Filter' } },
+ },
+ dataMask: {
+ 'test-filter': {
+ extraFormData: {},
+ filterState: { value: undefined },
+ },
+ },
+ },
+ };
+
+ const { rerender } = render(
+ <SelectFilterPlugin {...likeProps} />,
+ reduxState,
+ );
+
+ fireEvent.change(
+ screen.getByPlaceholderText('Type to search (contains)...'),
+ {
+ target: { value: 'Jen' },
+ },
+ );
+
+ setDataMaskMock.mockClear();
+
+ rerender(
+ <SelectFilterPlugin
+ {...buildSelectFilterProps({
+ formData: { operatorType: SelectFilterOperatorType.Exact },
+ filterState: { value: undefined },
+ setDataMask: setDataMaskMock,
+ })}
+ />,
+ );
+
+ act(() => {
+ jest.advanceTimersByTime(500);
+ });
+
+ expect(setDataMaskMock).not.toHaveBeenCalledWith(
+ expect.objectContaining({
+ extraFormData: {
+ filters: [
+ {
+ col: 'gender',
+ op: 'ILIKE',
+ val: '%Jen%',
+ },
+ ],
+ },
+ }),
+ );
Review Comment:
**Suggestion:** This assertion only checks that an `ILIKE` payload was not
emitted, but a stale debounced callback could still fire after switching to
exact mode and emit an `IN` payload, making the test pass even when
cancellation is broken. Assert that no call happens after the timer flush to
actually verify debounce cancellation. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Debounce-cancel regressions can bypass SelectFilterPlugin test coverage.
- ❌ CI may miss stale filter updates after operator switch.
```
</details>
```suggestion
expect(setDataMaskMock).not.toHaveBeenCalled();
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run test `pending LIKE debounce is canceled when operatorType switches
back to Exact`
in
`superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx:1585`;
it
types into LIKE input at lines 1614-1619, which triggers
`handleLikeInputChange` in
`SelectFilterPlugin.tsx:405-409`.
2. That input schedules `debouncedLikeChange`
(`SelectFilterPlugin.tsx:385-393`), and the
callback uses `updateDataMaskRef.current([text])`
(`SelectFilterPlugin.tsx:389`), not a
fixed operator payload.
3. After rerender to Exact mode (`SelectFilterPlugin.test.tsx:1623-1631`),
`updateDataMaskRef` is refreshed in `SelectFilterPlugin.tsx:381-383`; if
debounce
cancellation regresses, timer flush at test lines 1633-1635 still executes
pending
callback.
4. Callback then builds Exact-mode filter via `updateDataMask`
(`SelectFilterPlugin.tsx:87-101`) and `getSelectExtraFormData`
(`superset-frontend/src/filters/utils.ts:67-83`), producing `IN` instead of
`ILIKE`.
5. Current assertion (`SelectFilterPlugin.test.tsx:1637-1649`) only rejects
an `ILIKE`
payload, so a stale `IN` call still passes; replacing with
`not.toHaveBeenCalled()`
correctly validates cancellation.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx
**Line:** 1637:1649
**Comment:**
*Logic Error: This assertion only checks that an `ILIKE` payload was
not emitted, but a stale debounced callback could still fire after switching to
exact mode and emit an `IN` payload, making the test pass even when
cancellation is broken. Assert that no call happens after the timer flush to
actually verify debounce cancellation.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38470&comment_hash=9fe22f8648c0468f4bf74ccc40e374dac2131db4c78e73661b16d6053560191e&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38470&comment_hash=9fe22f8648c0468f4bf74ccc40e374dac2131db4c78e73661b16d6053560191e&reaction=dislike'>👎</a>
--
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]
