-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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
Hi @akhilnarang, |
Dear @akhilnarang, |
0dfba85
to
aaa626f
Compare
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.
aaa626f
to
4cfc8bf
Compare
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:- Running: cypress/integration/control_link.js
I have not made any changes relating to this UI (3rd) test - please advise. |
aded64e
to
df146c1
Compare
…per 'prettier' in precommit
Dear @akhilnarang, |
177fc5b
to
4860c58
Compare
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; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@karanwilson the rest seems fine, in what case was this needed? (field being defined but doc undefined)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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) * 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 <[email protected]>
## [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))
🎉 This PR is included in version 15.29.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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.