frappe icon indicating copy to clipboard operation
frappe copied to clipboard

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

Open karanwilson opened this issue 9 months ago • 4 comments

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 avatar May 06 '24 16:05 karanwilson

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 avatar May 07 '24 12:05 karanwilson

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

karanwilson avatar May 10 '24 17:05 karanwilson

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

  1. 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.

karanwilson avatar May 11 '24 09:05 karanwilson

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

karanwilson avatar May 15 '24 18:05 karanwilson

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.

stale[bot] avatar Jun 02 '24 07:06 stale[bot]

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.

karanwilson avatar Jun 02 '24 15:06 karanwilson

:tada: This PR is included in version 15.29.1 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket:

frappe-pr-bot avatar Jun 04 '24 11:06 frappe-pr-bot