-
Notifications
You must be signed in to change notification settings - Fork 3
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(ui): enable super user ID editing #663
Conversation
Code Climate has analyzed commit 6109b46 and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 16.8% (50% is the threshold). This pull request will bring the total coverage in the repository to 39.9% (0.1% change). View more on Code Climate. |
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.
Looks a lot cleaner to me. just make sure it gets tested before merging. and then please do not start any more tickets today.
🎉 This PR is included in version 1.5.0-val.66 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Purpose
This PR addresses a critical bug which prevented super users from submitting ID edits.
However, since I've changed the way the user is routed to
/details
in #644, this meant that redirecting the super user back to the/details/:authority/:updatedId
needed to be addressed as well.After studying the logic, I realized that it can be simplified to a single function called
getOriginForm
. It will determine where the action originated from and append the appropriatetab
if needed. This new direction required me to change theorigin
params and update multiple forms.Linked Issues to Close
https://qmacbis.atlassian.net/browse/OY2-29122
Approach
useOrigin
hook a new identity – introducinggetFormOrigin
:new URLSearchParams(window.location.search)
in any submit function would serve the present URL params. The old code had to deal with hook limitations (can't call in other functions, cannot be conditional, etc.) for no reason.ORIGIN
param had to be mapped to a route. This decision added another complexity since theORIGIN
param itself could just be a route, circumventing the mapping entirely.:authority
wasn't a param and thus inaccessible in some cases,useOrigin
wasn't equipped to redirect the user to the appropriate route.getFormOrigin
, thanks toORIGIN
param changes and improvements from fix(ui): waivers tab fix #644, can now determine where to redirect the user. Redirecting a user to/details
means we have to provide the correct:authority
and:id
params. Redirecting a user to the correct dashboard tab means we have to provide the correctauthority
. The function isn't that intelligent, but it's a small price to pay, IMO, for simplicity and maintainability.useOrigin
to usegetFormOrigin
CheckDocumentFunction
for updated IDs to poll until a new timestamp is detectedsubmitActionForm
intoonSubmit
inActionForm
for separation of concernsid
property