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.

Sign up for GitHub

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

Jump to bottom

MPDX-7772 Preferences - Admin #866

Merged
merged 20 commits into from
Feb 15, 2024
Merged

MPDX-7772 Preferences - Admin #866

merged 20 commits into from
Feb 15, 2024

Conversation

dr-bizz
Copy link
Contributor

@dr-bizz dr-bizz commented Jan 30, 2024

Description

Building the admin functionality onto the new MPDx. We removed the "Add Offline Organization" as this is now done elsewhere.
I've had to add some functionality we have on other Preferences PRs which haven't been merged yet.

Changes

  • Adding the ability to impersonate a user.
  • Adding the ability for admins to reset a user.

Checklist:

  • I have given my PR a title with the format "MPDX-(JIRA#) (summary sentence max 80 chars)"
  • I have applied the appropriate labels. (Add the label "On Staging" to get the branch automatically merged into staging.)
  • I have requested a review from another person on the project

@dr-bizz
Copy link
Contributor Author

dr-bizz commented Jan 30, 2024

I should prob add the functionality to kick the user back to preferences and show an error if they try to navigate to a Settings page which they're not authorized too.

@dr-bizz
Copy link
Contributor Author

dr-bizz commented Jan 30, 2024

I should prob add the functionality to kick the user back to preferences and show an error if they try to navigate to a Settings page which they're not authorized too.

I think I'll do this in a separate PR, as I'm thinking about adding the user permissions onto the next-auth session

@aws-amplify-us-east-1 AWS Amplify (us-east-1)
Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-866.d3dytjb8adxkk5.amplifyapp.com

@dr-bizz
Admin preferences.
b37c153 
- Impersonate user
- Reset an account
- Phone TS error fix
- Adding tests
@dr-bizz dr-bizz added the On Staging Will be merged to the staging branch by Github Actions label Jan 31, 2024
@dr-bizz dr-bizz requested a review from canac January 31, 2024 21:10
canac
canac requested changes Feb 1, 2024
View reviewed changes
Copy link
Contributor

@canac canac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also need to go over the tests and test the functionality, but here are my comments so far.

pages/accountLists/[accountListId]/settings/admin.page.tsx Show resolved Hide resolved
pages/api/auth/impersonate/impersonateHelper.ts Outdated Show resolved Hide resolved
pages/api/auth/impersonate/impersonateHelper.ts Outdated Show resolved Hide resolved
pages/api/auth/impersonate/impersonateHelper.ts Outdated Show resolved Hide resolved
pages/api/auth/impersonate/impersonateUser.page.ts Outdated Show resolved Hide resolved
src/components/Settings/Admin/ImpersonateUser/ImpersonateUserAccordion.tsx Outdated Show resolved Hide resolved
src/components/Settings/Admin/ResetAccount/ResetAccountAccordion.tsx Outdated Show resolved Hide resolved
src/components/Settings/Admin/ResetAccount/ResetAccountAccordion.tsx Outdated Show resolved Hide resolved
src/components/Settings/Admin/ResetAccount/ResetAccountAccordion.tsx Show resolved Hide resolved
src/components/Settings/Admin/ResetAccount/ResetAccountAccordion.tsx Outdated Show resolved Hide resolved
canac
canac requested changes Feb 1, 2024
View reviewed changes
Copy link
Contributor

@canac canac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't try resetting an account because that's such a destructive operation. But I was able to successfully impersonate you. Great work! 🚀

I noticed that when you're impersonating someone and stop impersonating from the profile menu, it says "Stopping Impersonating and redirecting you to the legacy MPDX". Should we change that now to keep them on MPDX React after they stop impersonating?

pages/api/auth/impersonate/impersonateHelper.ts Outdated Show resolved Hide resolved
src/components/Settings/Admin/ImpersonateUser/ImpersonateUserAccordion.test.tsx Outdated Show resolved Hide resolved
src/components/Settings/Admin/ResetAccount/ResetAccountAccordion.test.tsx Outdated Show resolved Hide resolved
src/components/Settings/Admin/ResetAccount/ResetAccountAccordion.test.tsx Outdated Show resolved Hide resolved
src/components/Settings/Admin/ResetAccount/ResetAccountAccordion.tsx Outdated Show resolved Hide resolved
src/components/Settings/Admin/ImpersonateUser/ImpersonateUserAccordion.tsx Outdated Show resolved Hide resolved
@dr-bizz dr-bizz requested a review from canac February 14, 2024 19:22
canac
canac approved these changes Feb 14, 2024
View reviewed changes
Copy link
Contributor

@canac canac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the only required change is switching the "The Key / Relay" labels to refer to Okta, so I'm going to go ahead and approve pending that. Thanks for all the extra tests you added!

pages/accountLists/[accountListId]/settings/admin.page.test.tsx
import Admin, { suggestedArticles } from './admin.page';

jest.mock('next-auth/react');
jest.mock('next/router', () => ({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of mocking useRouter, most of our tests provide a router object to <TestRouter>.

Copy link
Contributor Author

@dr-bizz dr-bizz Feb 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since I'm mocking the returned value on lines 59 & 80, I believe I need to keep mocking the next/router module.

pages/accountLists/[accountListId]/settings/admin.page.test.tsx Outdated Show resolved Hide resolved
src/components/Settings/Admin/ImpersonateUser/ImpersonateUserAccordion.tsx Outdated Show resolved Hide resolved
pages/api/auth/impersonate/impersonateHelper.ts Outdated Show resolved Hide resolved
@dr-bizz dr-bizz merged commit b1e0f72 into main Feb 15, 2024
17 of 18 checks passed
@dr-bizz dr-bizz deleted the preferences-admin branch February 15, 2024 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@canac canac canac approved these changes

Assignees
No one assigned
Labels
On Staging Will be merged to the staging branch by Github Actions
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dr-bizz @canac