rusackas commented on code in PR #44880:
URL: https://github.com/apache/superset/pull/44880#discussion_r4163340685
##########
superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx:
##########
@@ -564,32 +629,32 @@ export default function PluginFilterSelect(props:
PluginFilterSelectProps) {
updateDataMaskRef.current = updateDataMask;
}, [updateDataMask]);
- const debouncedLikeChange = useMemo(
- () =>
- debounce((text: string) => {
- if (text) {
- updateDataMaskRef.current([text]);
- } else {
- updateDataMaskRef.current(null);
- }
- }, Constants.SLOW_DEBOUNCE),
- [],
- );
-
+ // Leaving a LIKE operator discards the pending edit so returning to one
+ // doesn't replay text typed under the previous mode.
useEffect(() => {
- if (!isLikeOperator || clearAllTrigger) {
- debouncedLikeChange.cancel();
+ if (!isLikeOperator) {
+ settleLikeValue.cancel();
+ setPendingLikeValue(null);
}
- }, [clearAllTrigger, debouncedLikeChange, isLikeOperator]);
+ }, [isLikeOperator, settleLikeValue]);
Review Comment:
Good catch. The pending LIKE edit is now discarded on any operator change,
not just when leaving a LIKE mode, so pasting the same text after switching
Contains to Starts with registers as a fresh edit and sets the default. Pushed
45af057. I could not run jest locally, so leaning on CI.
##########
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={{}} />);
Review Comment:
Right, `{}` is truthy so that rerender never actually released the trigger.
It now passes `undefined`, so the test exercises Clear All completing and still
asserts the typed predicate is not restored. Pushed 45af057.
--
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]
