fix: JSONForm select/multiselect filterText value synchronous update with onFilterUpdate action #12578

ashit-rath · 2022-04-05T07:36:11Z

Description

For a serverside enabled select/multiselect field type when an onFilterUpdate action is added, and within that action if the select/multiselect field's filterText property ({{fieldState.select.filterText}}) is used, when the action is trigger the action' function does not get the latest value of the filterText.

Fixes #12545

Steps

Drop a JSONForm
Convert a field to select type
Enable Filterable and Server Side Filtering
Enable JS binding for onFilterUpdate
Add the following binding {{showAlert('Filter update:'}}{{fieldState?.<fieldName>?.filterText)}} (make sure to modify the fieldName)
Type in the input of the select menu, the alert will always show the previous value.

Problem:
The update to the filterText always happened after the action was executed (the after update hook for updating meta was not being used).

Fix:
Refactored the code to make the update of filterText in sync with the tigger of the action.

Type of change

Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Manual
Cypress

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes

Test coverage results 🧪

⚪ Total coverage has not changed

// Code coverage diff between base branch:release and head branch: fix/12545-jsonform-filter-text

Status	File	% Stmts	% Branch	% Funcs	% Lines
🟢	total	56.35 (0.01)	37.8 (0)	35.96 (0)	56.57 (-0.01)
🔴	app/client/src/utils/autocomplete/TernServer.ts	52.71 (-0.23)	40.83 (-0.84)	36.21 (0)	56.74 (-0.25)
🔴	app/client/src/widgets/JSONFormWidget/schemaParser.ts	98.17 (0)	89.7 (-0.6)	100 (0)	98.74 (0)
🟢	app/client/src/widgets/JSONFormWidget/fields/MultiSelectField.tsx	28.77 (1.5)	5.41 (0)	0 (0)	33.33 (0.52)
🟢	app/client/src/widgets/JSONFormWidget/fields/SelectField.tsx	30.36 (1.05)	4.55 (0)	0 (0)	32.69 (0.61)
🔴	app/client/src/widgets/JSONFormWidget/fields/useUpdateInternalMetaState.ts	35.29 (-0.42)	0 (0)	0 (0)	35.29 (-0.42)
🟢	app/client/src/widgets/JSONFormWidget/widget/helper.ts	98.92 (0.01)	90.91 (0.43)	100 (0)	98.82 (0)
🔴	app/client/src/widgets/JSONFormWidget/widget/index.tsx	30.1 (0.29)	0 (0)	21.43 (-0.79)	30.61 (-1.3)

…nFilterUpdate action

vercel · 2022-04-05T07:36:14Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/get-appsmith/appsmith/AHNQx1brRigMusnjTi4LJKJXbsnU
✅ Preview: https://appsmith-git-fix-12545-jsonform-filter-text-get-appsmith.vercel.app

ashit-rath · 2022-04-05T07:37:14Z

/ok-to-test sha=d30740e

github-actions · 2022-04-05T07:37:51Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/2094685892.
Workflow: Appsmith External Integration Test Workflow.
Commit: d30740e.
PR: 12578.

github-actions · 2022-04-05T08:16:45Z

UI Performance test run logs and artifacts: https://github.com/appsmithorg/appsmith/actions/runs/2094685892.
Commit: d30740e.
Results:

Click to view performance test results

	Median	Mean	SD.Sample	SD.Population

sbalaji1192 · 2022-04-05T17:39:10Z

app/client/src/widgets/JSONFormWidget/fields/useUpdateInternalMetaState.ts

+
+  const debouncedUpdateProperty = useMemo(
+    () => debounce(updateProperty, DEBOUNCE_TIMEOUT),
+    [setMetaInternalFieldState, updateProperty],


Suggestion: we can remove setMetaInternalFieldState from deps, since its a dep of updateProperty

sbalaji1192 · 2022-04-05T17:44:30Z

have triggered the failed jobs to again in CI.

ashit-rath · 2022-04-06T06:45:01Z

/ok-to-test sha=cc67d32

github-actions · 2022-04-06T06:45:34Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/2100750608.
Workflow: Appsmith External Integration Test Workflow.
Commit: cc67d32.
PR: 12578.

chandannkumar · 2022-04-06T06:57:57Z

Observation:

filterText value shows alert on Page change of applications

github-actions · 2022-04-06T07:10:16Z

UI Performance test run logs and artifacts: https://github.com/appsmithorg/appsmith/actions/runs/2100750608.
Commit: cc67d32.
Results:

Click to view performance test results

	Median	Mean	SD.Sample	SD.Population

ashit-rath · 2022-04-06T08:06:18Z

@chandannkumar Can you share the app? I'm unable to reproduce it

chandannkumar · 2022-04-06T09:26:06Z

Public app

@ashit-rath

ashit-rath · 2022-04-07T04:06:54Z

@chandannkumar I think it's a problem with the multi-select widget. I don't know if this is intentional or not.
@Tooluloope Can you confirm if this behaviour is valid or a bug with multiselect widget?

Tooluloope · 2022-04-07T04:18:00Z

@ashit-rath this is definitely a bug

ashit-rath · 2022-04-07T04:20:20Z

@Tooluloope Thanks for the confirmation
@chandannkumar Can you please create an issue for this and do check if select widget also has the same behaviour.

Note: Fixing this in the individual widget would fix the issue in JSONForm as well.

ashit-rath · 2022-04-08T04:21:35Z

/ok-to-test sha=98481e3

github-actions · 2022-04-08T04:22:09Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/2113059051.
Workflow: Appsmith External Integration Test Workflow.
Commit: 98481e3.
PR: 12578.

github-actions · 2022-04-08T04:49:57Z

UI Performance test run logs and artifacts: https://github.com/appsmithorg/appsmith/actions/runs/2113059051.
Commit: 98481e3.
Results:

Click to view performance test results

	Median	Mean	SD.Sample	SD.Population

chandannkumar · 2022-04-08T06:06:53Z

Tested this PR and working as expected

fix: JSONForm select/multiselect filterText synchronous update with o…

d30740e

…nFilterUpdate action

ashit-rath requested a review from sbalaji1192 April 5, 2022 07:36

github-actions bot added Widgets Product This label groups issues related to widgets Bug Something isn't working High This issue blocks a user from building or impacts a lot of users JSON Form Issue / features related to the JSON form wiget UI Building Pod labels Apr 5, 2022

ashit-rath changed the title ~~fix: JSONForm select/multiselect filterText synchronous update with onFilterUpdate action~~ fix: JSONForm select/multiselect filterText value synchronous update with onFilterUpdate action Apr 5, 2022

sbalaji1192 previously approved these changes Apr 5, 2022

View reviewed changes

removed unused dep from useMemo

cc67d32

ashit-rath dismissed sbalaji1192’s stale review via cc67d32 April 6, 2022 06:43

vercel bot deployed to Preview April 6, 2022 06:43 View deployment

Merge branch 'release' into fix/12545-jsonform-filter-text

98481e3

vercel bot deployed to Preview April 8, 2022 04:19 View deployment

sbalaji1192 approved these changes Apr 8, 2022

View reviewed changes

ashit-rath merged commit 99ec9e8 into release Apr 8, 2022

ashit-rath deleted the fix/12545-jsonform-filter-text branch April 8, 2022 06:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: JSONForm select/multiselect filterText value synchronous update with onFilterUpdate action #12578

fix: JSONForm select/multiselect filterText value synchronous update with onFilterUpdate action #12578

ashit-rath commented Apr 5, 2022 •

edited by github-actions bot

Loading

vercel bot commented Apr 5, 2022 •

edited

Loading

ashit-rath commented Apr 5, 2022

github-actions bot commented Apr 5, 2022

github-actions bot commented Apr 5, 2022

sbalaji1192 Apr 5, 2022

ashit-rath Apr 6, 2022

sbalaji1192 commented Apr 5, 2022 •

edited

Loading

ashit-rath commented Apr 6, 2022

github-actions bot commented Apr 6, 2022

chandannkumar commented Apr 6, 2022 •

edited by loom-gif-previews bot

Loading

github-actions bot commented Apr 6, 2022

ashit-rath commented Apr 6, 2022

chandannkumar commented Apr 6, 2022 •

edited

Loading

ashit-rath commented Apr 7, 2022

Tooluloope commented Apr 7, 2022

ashit-rath commented Apr 7, 2022 •

edited

Loading

ashit-rath commented Apr 8, 2022

github-actions bot commented Apr 8, 2022

github-actions bot commented Apr 8, 2022

chandannkumar commented Apr 8, 2022

fix: JSONForm select/multiselect filterText value synchronous update with onFilterUpdate action #12578

fix: JSONForm select/multiselect filterText value synchronous update with onFilterUpdate action #12578

Conversation

ashit-rath commented Apr 5, 2022 • edited by github-actions bot Loading

Description

Type of change

How Has This Been Tested?

Checklist:

Test coverage results 🧪

vercel bot commented Apr 5, 2022 • edited Loading

ashit-rath commented Apr 5, 2022

github-actions bot commented Apr 5, 2022

github-actions bot commented Apr 5, 2022

sbalaji1192 Apr 5, 2022

Choose a reason for hiding this comment

ashit-rath Apr 6, 2022

Choose a reason for hiding this comment

sbalaji1192 commented Apr 5, 2022 • edited Loading

ashit-rath commented Apr 6, 2022

github-actions bot commented Apr 6, 2022

chandannkumar commented Apr 6, 2022 • edited by loom-gif-previews bot Loading

github-actions bot commented Apr 6, 2022

ashit-rath commented Apr 6, 2022

chandannkumar commented Apr 6, 2022 • edited Loading

ashit-rath commented Apr 7, 2022

Tooluloope commented Apr 7, 2022

ashit-rath commented Apr 7, 2022 • edited Loading

ashit-rath commented Apr 8, 2022

github-actions bot commented Apr 8, 2022

github-actions bot commented Apr 8, 2022

chandannkumar commented Apr 8, 2022

ashit-rath commented Apr 5, 2022 •

edited by github-actions bot

Loading

vercel bot commented Apr 5, 2022 •

edited

Loading

sbalaji1192 commented Apr 5, 2022 •

edited

Loading

chandannkumar commented Apr 6, 2022 •

edited by loom-gif-previews bot

Loading

chandannkumar commented Apr 6, 2022 •

edited

Loading

ashit-rath commented Apr 7, 2022 •

edited

Loading