fix: update successful save redirects [DHIS2-15431]

[tomzemp]

I noticed when making some updates in the user app that the redirects after saving a user group or user role did not appear to work (this might be limited to the case of editing a user group/user role related to the current user). Moreover, since they were using history.goBack() when successful, these redirects would navigate to the page you were previously on rather than the main menu for the relevant user metadata (users, user groups, or user roles). Since we use deep links, this behaviour seems counter-intuitive (e.g. if I am on dhis2.org and navigate to a user edit page and save, I think this should when successful go to the list of users, not back to dhis2.org)

I have used our internal navigateTo function which is using history.push()

[dhis2-bot]

🚀 Deployed on https://pr-1218--dhis2-user.netlify.app

[KaiVandivier]

Tested on Netlify 👍

I noticed "Cancel without saving" would still use history.goBack(), but because of the way the <Form> component is designed, there would be some extra steps to refactor. Maybe the <Form> component could take a navigateToOnCancel prop

In some way, I can see some merit to using history.goBack() if you click "Cancel", because it seems a bit like an "undo/go back" action... but maybe navigating to the relevant list view is better and more consistent with the "Save" behavior. What do you think?

Approved because this is also good as-is 🙂

[tomzemp]

thanks @KaiVandivier! Good catch re: cancel button.

I went and made an update to have the cancel navigate back to the main list rather than use history.goBack() and then I discovered that the e2e tests failed. It turns out that history.goBack() was being used to preserve the filtering (e.g. if you type admin in the user list filter, click edit for the admin/John Traore and the click cancel, you should go back to a page where that admin filter is still preserved. So, using history.goBack() keeps this filtering when you're navigating from within the app, but causes problems when you've followed a direct link.

TL;DR: I implemented code to deal with this, but it's more of a refactor, so not sure if we want to implement or not. If you have thoughts, or better ideas, let me know.

Technical fix

To address this, I wanted to determine on the edit page if you had been referred from elsewhere within the app (in which case we could use history.goBack()) or not (in which case we just send back to the main list).

Unfortunately, document.referrer does not seem to be available within the app. I then thought, I could make this information available within the react-router history (e.g. more information about the within-app history). It's possible to use location.state if you're using a BrowserRouter history for this (i.e. push extra state information when we're pushing new location to history), but unfortunately this is not supported in HashRouter; from the docs IMPORTANT NOTE: Hash history does not support location.key or location.state. In previous versions we attempted to shim the behavior but there were edge-cases we couldn’t solve. Any code or plugin that needs this behavior won’t work. As this technique is only intended to support legacy browsers, we encourage you to configure your server to work with <BrowserHistory> instead.

I then thought maybe we could just navigate to the edit page and preserve the filter as a parameter (e.g. users/edit/xE7jOejl9FI?query=admin), so you could then extract the query parameter and formulate the correct url to navigate back to; however, it seems strange to push the filter term to the url just for navigating back and forth and then if people share direct links, they would be redirected back to a filtered view (which seems as though it would be confusing as they didn't enter the filter term themselves).

So, this left me with keeping track of the referral somehow in global state. Since the app was refactored to remove redux, and since I didn't want to add any other global state manager, I've implemented a provider for this. Now, if you click on "edit" from the context menu, it stores information about what list (user,user-group,user-role) you were on and then from within the form's cancel button, it will check if the form you are on matches the list last navigated from (/if there has been any navigation); if the current form matches the last list, history.goBack() is used (and the filtering is hence preserved); otherwise you are sent to the main list for the the relevant object type (user,user group, user role)

UX
Currently, the history.goBack() approach applies both for cancel and save. I have updated this to use history.goBack() (when appropriate) after cancel but to just go to the main list after save. I could change the save to match, but to me it seemed like if you search for a user and edit them, the reason for your filtering is probably over; whereas if you cancel, you probably cancel because you realized that the user you selected is not the one you wanted to search for? Anyway, I could change the save functionality to be the same as the cancel (or leave it as is), depending on whether we want to make changes or not.

[KaiVandivier]

Nice! I think this design is very good, and seems to be fairly simple to implement 🙌

I noticed that clicking on a row in a list view (e.g. opening the "Users" list, then clicking on the name of a user, instead of using context menu) doesn't get the same "referrer" treatment that clicking on the "Edit" button in the context menu does -- I think those behaviors should match

One subjective suggestion is to reorganize the Referrer context a bit: I think all the Referrer context pieces could go in one file, and I would call the directory context

[KaiVandivier]

This looks like the kind of place that could use setReferrer():

user-app/src/pages/UserList/UserTable.js

Lines 104 to 110 in a97a5bd

	const handleClick = () => {
	if (access.update) {
	navigateTo(`/users/edit/${id}`)
	} else if (access.read) {
	navigateTo(`/users/view/${id}`)
	}
	}

[tomzemp]

Thanks @KaiVandivier 🤦‍♂️. I think I always click on the context menu and then edit, so I forgot that you can click on the row 😅.

Updated to use setReferrer() when clicking on rows

[dhis2-bot]

🎉 This PR is included in version 1.5.29 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀