Skip to content
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

fix: Recursive routing for Form route #15539

Merged
merged 2 commits into from Jan 18, 2022

Conversation

pruthvi145
Copy link
Contributor

@pruthvi145 pruthvi145 commented Jan 5, 2022

Issue

In many places, to a make new document, we just do set_route to /new which eventually redirects to the actual form having local docname (e.g new-item-1). However, this leaves the history for /new in browser. Now, when we try to go back from new-item-1 to previous page, it redirects to /new page which again redirects to new document page /new-item-2 forming a recursive behaviour which never allows user to go back.

Main cause for this is that set_route always push the route internally and there is no support for replacing the existing route.

routing-bug

Changes

  • Introduced new route flag called replace_route.
  • Automatically reset routes_flags after set_route.
  • Used this flag in formview.js to solve mentioned issue.

routing-bug-fixed


(This issue occurs in Workspace - when there is any Shortcut has New Document View)

To Replicate

In ERPNext Installation...

Step 1: Change Workspace Config (temporary)

  • Go to Workspace List > Home

  • In Shortcuts section, Change the Document to View to New for Item

    image

Step 2: Replicate

  • Go to home page and click on Item shortcut (as shown in the above GIFs)
  • Click the back button

Step 3: Reset Workspace Config

  • Go to Workspace List > Home
  • In Shortcut section, Change the Document to View back to unset

@stale stale bot added the inactive label Jan 12, 2022
@frappe frappe deleted a comment from stale bot Jan 13, 2022
@pruthvi145 pruthvi145 marked this pull request as ready for review January 13, 2022 08:48
@pruthvi145 pruthvi145 requested a review from a team as a code owner January 13, 2022 08:48
@pruthvi145 pruthvi145 requested review from hasnain2808 and removed request for a team January 13, 2022 08:48
@hasnain2808
Copy link
Contributor

@pruthvi145
I am unable to replicate the issue shown in the gif in the latest develop
Can you provide more info on how to replicate?
some snippet or steps would be great

@pruthvi145
Copy link
Contributor Author

Hi @hasnain2808, I updated the description for replicating this issue. Let me know if you still can not be able to replicate it.

@surajshetty3416 surajshetty3416 changed the title fix: recursive routing for Form route fix: Recursive routing for Form route Jan 18, 2022
@surajshetty3416 surajshetty3416 merged commit 53d4ddc into frappe:develop Jan 18, 2022
@surajshetty3416 surajshetty3416 deleted the fix-recursive-routing branch January 18, 2022 07:54
@surajshetty3416
Copy link
Member

@Mergifyio backport version-13-hotfix

mergify bot pushed a commit that referenced this pull request Jan 18, 2022
Co-authored-by: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com>
(cherry picked from commit 53d4ddc)
@mergify
Copy link
Contributor

mergify bot commented Jan 18, 2022

backport version-13-hotfix

✅ Backports have been created

surajshetty3416 added a commit that referenced this pull request Jan 18, 2022
Co-authored-by: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com>
Co-authored-by: Pruthvi Patel <pruthvipatel145@gmail.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants