Resolves front-end bug in 'customize_form.js', triggered by delete actions on child-tables #26344

karanwilson · 2024-05-06T16:17:50Z

closes #26041

The following video (uploaded by the user who opened the issue) demonstrates the bug:-
https://github.com/frappe/frappe/assets/48678570/09351117-66bd-4934-a58a-9630f836c522

The bug affects the following 3 child tables in the Customize Form view:-
"DocType Link"
"DocType Action"
"DocType State"

The issue emerges when we try to delete any rows that are added to the above child tables in the Doctype opened via the Customize Form.

After debugging and tracing the ecosystem of the issue, it is found that:-
the exception occurs when the delete function removes rows of the above mentioned child tables (of their respective parent doctype) from the local copy of the browser.
As per design, this works fine in the case of Doctype edits, but in the case of Customise Form edits, the 'parent' doc of the opened Form view, is 'Customize Form', which is different from the 'parent' of the child table that is being edited.
Hence the delete does not reflect on the opened form, and the grid_row of the child table become 'undefined'.

The fix:-
Replicate the changed child-table field of the 'parent' doctype, to the opened child-table field of 'Customise-Form'.
(frm.doc.links = parent_doc.links;)
Please refer to the commit message, for full details.

karanwilson · 2024-05-07T12:59:44Z

Hi @akhilnarang,
based on the recent workflow feedback, I have defined the variables 'parenttype' and 'parent' locally within customize_form.js
Requesting a re-run of the workflow.

karanwilson · 2024-05-10T17:06:05Z

Dear @akhilnarang,
Please advise if any more changes are needed.

frappe#26041 In refresh_field(fieldname, txt) function of grid_row.js, added the comments :- // the below if statement is added to factor in the exception when this.doc is undefined - // - after row removals via customize_form.js on links, actions and states child-tables if (this.doc) field.docname = this.doc.name;

In customize_form.js :- defined parent and parenttype as local variables in the event functions for child tables links, actions and states

This reverts commit 6732f0a.

karanwilson · 2024-05-11T09:18:18Z

Hi @akhilnarang, thanks for the re-run of the workflows;

I have fixed the spacing for 'Prettier', and the 'Commit Titles' now.

Requesting help in understanding the 3rd test that hadn't succeeded:-
UI, Attempt # 2 / UI Tests (Cypress) (3)

Running: cypress/integration/control_link.js
9 passing (3m)
1 failing

Control Link
show text for Gender link field with language en:
CypressError: Timed out retrying after 30000ms: cy.wait() timed out waiting 30000ms for the 1st response to the route: search_link. No response ever occurred.

I have not made any changes relating to this UI (3rd) test - please advise.

…per 'prettier' in precommit

karanwilson · 2024-05-15T18:48:53Z

Dear @akhilnarang,
Now all checks were successful;
Please advise whether I should troubleshoot the cypress UI error while running:-
cypress/integration/control_link.js

akhilnarang · 2024-05-16T07:20:51Z

frappe/public/js/frappe/form/grid_row.js

-			field.docname = this.doc.name;
+			// the below if statement is added to factor in the exception when this.doc is undefined -
+			// - after row removals via customize_form.js on links, actions and states child-tables
+			if (this.doc) field.docname = this.doc.name;


@karanwilson the rest seems fine, in what case was this needed? (field being defined but doc undefined)

Hi @akhilnarang, when a grid-row/child-table row (for links, actions, states) is deleted using a 'Customize Form' view, then the deleted grid-row document (this.doc) in locals (browser copy of the doctype) becomes undefined, because that particular row gets deleted - in this case when the refresh_field method in grid_row.js is called, the browser console throws a silent error mentioning that this.doc is undefined (in statement 1441 above), where field.docname is assigned the value from this.doc.name

Having the if condition does not run the field.docname assignment statement, when the refresh_field method is called after row-deletes.

stale · 2024-06-02T07:46:45Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing.

karanwilson · 2024-06-02T15:24:37Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing.

Hi @akhilnarang, request your advise on the next steps for this PR.

…s on child-tables (#26344) * fix: Doctype Links not updating #26041 #26041 In refresh_field(fieldname, txt) function of grid_row.js, added the comments :- // the below if statement is added to factor in the exception when this.doc is undefined - // - after row removals via customize_form.js on links, actions and states child-tables if (this.doc) field.docname = this.doc.name; * fix: Doctype Links not updating #26041 In customize_form.js :- defined parent and parenttype as local variables in the event functions for child tables links, actions and states * Revert "In customize_form.js :-" This reverts commit 6732f0a. * fix: Doctype Links not updating #26041 * style: amended spacing as per 'prettier' in precommit * style: added comma after last event definitions in child doctype, as per 'prettier' in precommit (cherry picked from commit b9f4845)

…s on child-tables (#26344) (#26644) * fix: Doctype Links not updating #26041 #26041 In refresh_field(fieldname, txt) function of grid_row.js, added the comments :- // the below if statement is added to factor in the exception when this.doc is undefined - // - after row removals via customize_form.js on links, actions and states child-tables if (this.doc) field.docname = this.doc.name; * fix: Doctype Links not updating #26041 In customize_form.js :- defined parent and parenttype as local variables in the event functions for child tables links, actions and states * Revert "In customize_form.js :-" This reverts commit 6732f0a. * fix: Doctype Links not updating #26041 * style: amended spacing as per 'prettier' in precommit * style: added comma after last event definitions in child doctype, as per 'prettier' in precommit (cherry picked from commit b9f4845) Co-authored-by: Karan Wilson <48678570+karanwilson@users.noreply.github.com>

## [15.29.1](v15.29.0...v15.29.1) (2024-06-04) ### Bug Fixes * **address_query:** show search fields in description if set ([4db0b53](4db0b53)) * **address_query:** use title field if set ([01a00e3](01a00e3)) * allow creation of workspace based on desk role perms ([fb898f7](fb898f7)) * Auto Email Report not working when Add Total Row is enabled ([#26668](#26668)) ([e585657](e585657)) * Avoid concurrent TODO updates ([#26594](#26594)) ([cee96b9](cee96b9)) * bypass IP restriction for the methods required for our socketio backend ([42011c9](42011c9)) * **checkbox:** Fix checkbox size on small screens ([#26596](#26596)) ([#26612](#26612)) ([fc2a852](fc2a852)) * delete old user notification settings when merging users ([#26604](#26604)) ([#26619](#26619)) ([af1a47d](af1a47d)) * don't copy "Standard" on dashboard chart (backport [#26649](#26649)) ([#26651](#26651)) ([719522d](719522d)) * front-end bug in 'customize_form.js', triggered by delete actions on child-tables ([#26344](#26344)) ([b9f4845](b9f4845)), closes [#26041](#26041) [#26041](https://github.com/frappe/issues/26041) [#26041](#26041) * kanban filters fixes ([#26605](#26605)) ([#26610](#26610)) ([af7f550](af7f550)) * **link-preview:** Correct synchronization of preview data on change. ([#26641](#26641)) ([#26654](#26654)) ([c34c1d1](c34c1d1)) * **make_request:** don't blindly try to check the content-type ([2fc914f](2fc914f)) * **number_card:** ensure value is returned ([0a91f04](0a91f04)) * **UX:** multi-tab experience ([#26309](#26309)) ([#26646](#26646)) ([3e4fff7](3e4fff7)) ### Performance Improvements * memory leak on kanban refresh ([#26597](#26597)) ([#26599](#26599)) ([8b8a297](8b8a297)) * rearrange some frequent jobs ([#26591](#26591)) ([#26603](#26603)) ([d9b7d73](d9b7d73))

frappe-pr-bot · 2024-06-04T11:27:34Z

🎉 This PR is included in version 15.29.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

karanwilson requested a review from a team as a code owner May 6, 2024 16:17

karanwilson requested review from akhilnarang and removed request for a team May 6, 2024 16:17

karanwilson mentioned this pull request May 6, 2024

Resolves front-end bug in 'customize_form.js', triggered by delete actions on child-tables #26319

Closed

karanwilson force-pushed the version-15-hotfix-PR branch from 0dfba85 to aaa626f Compare May 10, 2024 18:24

karanwilson added 4 commits May 11, 2024 11:44

fix: Doctype Links not updating frappe#26041

93185ea

In customize_form.js :- defined parent and parenttype as local variables in the event functions for child tables links, actions and states

Revert "In customize_form.js :-"

3c644fe

This reverts commit 6732f0a.

fix: Doctype Links not updating frappe#26041

4cfc8bf

karanwilson force-pushed the version-15-hotfix-PR branch from aaa626f to 4cfc8bf Compare May 11, 2024 06:21

style: amended spacing as per 'prettier' in precommit

df146c1

karanwilson force-pushed the version-15-hotfix-PR branch from aded64e to df146c1 Compare May 15, 2024 10:48

style: added comma after last event definitions in child doctype, as …

4860c58

…per 'prettier' in precommit

akhilnarang force-pushed the version-15-hotfix-PR branch from 177fc5b to 4860c58 Compare May 16, 2024 07:11

akhilnarang reviewed May 16, 2024

View reviewed changes

stale bot added the inactive label Jun 2, 2024

stale bot removed the inactive label Jun 2, 2024

akhilnarang added backport develop port port to develop branch backport version-14-hotfix backport to version 14 labels Jun 3, 2024

akhilnarang merged commit b9f4845 into frappe:version-15-hotfix Jun 3, 2024
33 checks passed

mergify bot mentioned this pull request Jun 3, 2024

Resolves front-end bug in 'customize_form.js', triggered by delete actions on child-tables (backport #26344) #26644

Merged

mergify bot mentioned this pull request Jun 3, 2024

Resolves front-end bug in 'customize_form.js', triggered by delete actions on child-tables (backport #26344) #26645

Open

frappe-pr-bot added the released label Jun 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Resolves front-end bug in 'customize_form.js', triggered by delete actions on child-tables #26344

Resolves front-end bug in 'customize_form.js', triggered by delete actions on child-tables #26344

karanwilson commented May 6, 2024

karanwilson commented May 7, 2024 •

edited

karanwilson commented May 10, 2024

karanwilson commented May 11, 2024 •

edited

karanwilson commented May 15, 2024 •

edited

akhilnarang May 16, 2024

karanwilson May 16, 2024 •

edited

stale bot commented Jun 2, 2024

karanwilson commented Jun 2, 2024

frappe-pr-bot commented Jun 4, 2024

Resolves front-end bug in 'customize_form.js', triggered by delete actions on child-tables #26344

Resolves front-end bug in 'customize_form.js', triggered by delete actions on child-tables #26344

Conversation

karanwilson commented May 6, 2024

karanwilson commented May 7, 2024 • edited

karanwilson commented May 10, 2024

karanwilson commented May 11, 2024 • edited

karanwilson commented May 15, 2024 • edited

akhilnarang May 16, 2024

Choose a reason for hiding this comment

karanwilson May 16, 2024 • edited

Choose a reason for hiding this comment

stale bot commented Jun 2, 2024

karanwilson commented Jun 2, 2024

frappe-pr-bot commented Jun 4, 2024

karanwilson commented May 7, 2024 •

edited

karanwilson commented May 11, 2024 •

edited

karanwilson commented May 15, 2024 •

edited

karanwilson May 16, 2024 •

edited