rusackas commented on code in PR #44880:
URL: https://github.com/apache/superset/pull/44880#discussion_r4174042103
##########
superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx:
##########
@@ -2052,30 +2051,152 @@ test('pending LIKE debounce is canceled when
operatorType switches back to Exact
reduxState,
);
- fireEvent.change(
- screen.getByPlaceholderText('Type to search (contains)...'),
- {
- target: { value: 'Jen' },
+ const input = screen.getByPlaceholderText('Type to search (contains)...');
+ const callsBeforeSwitch = setDataMaskMock.mock.calls.length;
+
+ // Same batching as the sibling test above: the keystroke and the switch
+ // away from the LIKE operator land in the same update, before the
+ // deferred value has a chance to settle on its own.
+ act(() => {
+ fireEvent.change(input, { target: { value: 'Jen' } });
+ rerender(
+ <SelectFilterPlugin
+ {...buildSelectFilterProps({
+ formData: { operatorType: SelectFilterOperatorType.Exact },
+ filterState: { value: undefined },
+ setDataMask: setDataMaskMock,
+ })}
+ />,
+ );
+ });
+
+ await act(async () => {});
+
+ // Rerendering with fresh (non-memoized) props can independently re-run
+ // this plugin's own default-value initialisation, unrelated to the LIKE
+ // debounce this test is pinning -- so assert no *new* call carries the
+ // 'Jen' ILIKE payload, rather than that setDataMask was never called again.
+ const callsSinceSwitch = setDataMaskMock.mock.calls
+ .slice(callsBeforeSwitch)
+ .map(call => call[0]);
+ expect(callsSinceSwitch).not.toContainEqual(
+ expect.objectContaining({
+ extraFormData: expect.objectContaining({
+ filters: [expect.objectContaining({ op: 'ILIKE', val: '%Jen%' })],
+ }),
+ }),
+ );
+});
+
+test('switching Exact then back to LIKE does not replay the discarded pending
edit', async () => {
+ jest.useFakeTimers({ advanceTimers: true });
+ const setDataMaskMock = jest.fn();
+ const buildProps = (operatorType: SelectFilterOperatorType) =>
+ buildSelectFilterProps({
+ formData: { operatorType },
+ 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 {...buildProps(SelectFilterOperatorType.Contains)} />,
+ reduxState,
);
+ const input = screen.getByPlaceholderText('Type to search (contains)...');
- setDataMaskMock.mockClear();
+ act(() => {
+ fireEvent.change(input, { target: { value: 'Jen' } });
+ rerender(
+ <SelectFilterPlugin {...buildProps(SelectFilterOperatorType.Exact)} />,
+ );
+ });
+ await act(async () => {});
+ const callsBeforeReturn = setDataMaskMock.mock.calls.length;
rerender(
- <SelectFilterPlugin
- {...buildSelectFilterProps({
- formData: { operatorType: SelectFilterOperatorType.Exact },
- filterState: { value: undefined },
- setDataMask: setDataMaskMock,
- })}
- />,
+ <SelectFilterPlugin {...buildProps(SelectFilterOperatorType.Contains)} />,
+ );
+ await act(async () => {});
+
+ expect(
+ setDataMaskMock.mock.calls.slice(callsBeforeReturn).map(call => call[0]),
+ ).not.toContainEqual(
+ expect.objectContaining({
+ extraFormData: expect.objectContaining({
+ filters: [expect.objectContaining({ op: 'ILIKE', val: '%Jen%' })],
+ }),
+ }),
+ );
+});
+
+test('clear-all does not restore a LIKE edit still pending in the deferred
value', async () => {
+ jest.useFakeTimers({ advanceTimers: true });
+ 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,
);
+ const input = screen.getByPlaceholderText('Type to search (contains)...');
+ // Type and fire clear-all in one update, then release the trigger, so the
+ // deferred value still holds the typed text when the trigger goes away.
act(() => {
- jest.advanceTimersByTime(500);
+ fireEvent.change(input, { target: { value: 'Jen' } });
+ rerender(
+ <SelectFilterPlugin
+ {...likeProps}
+ clearAllTrigger={{ 'test-filter': true }}
+ />,
+ );
});
+ const callsBeforeRelease = setDataMaskMock.mock.calls.length;
+ rerender(<SelectFilterPlugin {...likeProps} clearAllTrigger={undefined} />);
+ await act(async () => {});
Review Comment:
@sadpandajoe agreed. The clear-all and operator-switch tests now advance
through the 500 ms debounce before asserting no stale mask is published. Pushed
c2e2874.
##########
superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx:
##########
@@ -200,6 +221,15 @@ export default function PluginFilterSelect(props:
PluginFilterSelectProps) {
const [likeInputValue, setLikeInputValue] = useState<string>(
filterState.value?.[0] != null ? String(filterState.value[0]) : '',
);
+ // null until the user types in the LIKE input; distinguishes a real edit
+ // from the clearAllTrigger reset below, which also clears likeInputValue
+ // but must not re-trigger a dispatch once the deferred value catches up.
+ const [pendingLikeValue, setPendingLikeValue] = useState<string |
null>(null);
+ const settleLikeValue = useMemo(
+ () => debounce(setPendingLikeValue, Constants.SLOW_DEBOUNCE),
Review Comment:
@sadpandajoe good catch. Any parent-driven change to `filterState.value` now
also discards the settled pending edit, so re-entering `Jen` after the
multi-select toggle registers as a fresh edit. Pushed c2e2874; jest does not
run locally here, so CI will confirm.
##########
superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx:
##########
@@ -525,8 +585,13 @@ export default function PluginFilterSelect(props:
PluginFilterSelectProps) {
});
updateDataMask(null);
- setSearch('');
+ settleSearch.cancel();
Review Comment:
@sadpandajoe right. Clear All now publishes the empty server search when a
search had run with "Search all filter's options" on, so the cleared filter
lists every option again. Pushed c2e2874.
--
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]
