-
Notifications
You must be signed in to change notification settings - Fork 1
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
MPDX-8492 - Enhance loadSession to Redirect Placeholder _ to Default Account List #1225
Conversation
Preview branch generated at https://salary-page-error.d3dytjb8adxkk5.amplifyapp.com |
Bundle sizes [mpdx-react]Compared against 8cd426f No significant changes found |
I think we did something similar in this PR. |
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.
Do we need to add this logic to the anywhere enforceAdmin
is used and to pages/accountLists/[accountListId].page.tsx
?
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.
I tested on the salary report page and it works.
We did, but to redirect, you have to wait for a failed request before you are redirected to the correct one. This caused many people issues. Find out more in the slack message I link in the description. In my code, we catch it on the server and then redirect it, making it a quicker and smoother experience for the user. The code in the ttps://github.com//pull/1084 will be an excellent backup, or if someone is trying to access the account list, they don't have access to it. |
I will retest on the preview link and then merge |
export const enforceAdmin: GetServerSideProps<PagePropsWithSession> = async ( | ||
export const enforceAdmin = async ( |
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.
Sorry if I wasn't clear the types on enforceAdmin
and ensureSessionAnddAccountList
were fine and probably better how they were because they include the session
on the props. I was just wanting to add some kind of return type to handleUnderscoreAccountListRedirect
.
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.
Because I changed handleUnderscoreAccountListRedirect
I also had to update the other functions as they had TS errors.
…ountList Id if they have an underscore as their accountList ID
b26cef5
to
44daada
Compare
577ba7d
to
421b484
Compare
Description
The
loadSession
function has been updated to automatically redirect users to their defaultaccountListID
when the placeholder_
is detected.This enhancement enables the sharing of links with users without requiring knowledge of their specific
accountListID
.For example, a link like
https://mpdx.org/accountLists/_/reports/salaryCurrency
will seamlessly redirect the user to the correct account list context.Test:
https://salary-page-error.d3dytjb8adxkk5.amplifyapp.com/accountLists/_/contacts?filters=%257B%2522status%2522%253A%255B%2522NEVER_CONTACTED%2522%255D%257D
https://salary-page-error.d3dytjb8adxkk5.amplifyapp.com/accountLists/_/settings/preferences
Checklist: