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

User permissions search #1853

Open
wants to merge 32 commits into
base: development
Choose a base branch
from
Open

Conversation

vorleakyek
Copy link
Member

Fixes #1801

What changes did you make and why did you make them ?

  • Identified and removed all instances of dummy/placeholder data from the "User Permission Search" screen and filtered permissions data component.
  • Updated MongoDB query to return a list of projectLead to match the frontend required data as suggested by Trillium
  • Hook up the previously created APIs from
  • Ensure the API responses are correctly mapped to the UI elements and behave as expected.
  • Note: Unit test will be considered later once these changes are approved

Screenshots of Proposed Changes

Visuals after changes are applied

user-permission-search

ntrehan and others added 29 commits October 8, 2024 00:48
Added an API that fetches all the admins
@vorleakyek
Copy link
Member Author

vorleakyek commented Feb 6, 2025

@ntrehan, @jng34, and @trillium, please take a look at the changes when you have some time and let me know if there is anything I should change in the code. Thank you!
cc: @JackHaeg

@JackHaeg JackHaeg requested a review from trillium February 6, 2025 19:46
@JackHaeg
Copy link
Member

JackHaeg commented Feb 6, 2025

@trillium please prioritize this PR review if possible :)

Copy link
Member

@jng34 jng34 left a comment

Choose a reason for hiding this comment

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

The GET requests work as expected. The code and comments are detailed. Integration looks good!!

Copy link
Member

@trillium trillium left a comment

Choose a reason for hiding this comment

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

Looking good! Needs a change to the StyledButton nav element:

// client/src/components/Navbar.jsx
<StyledButton component={NavLink} to="/users/permission-search" />

Reasons referenced in comments within that code block

@@ -61,7 +61,8 @@ const Navbar = (props) => {
{/* Admin auth -> Displays 2 links -> 'Users' and 'Projects'. */}
{auth?.user?.accessLevel === 'admin' && (
<>
<StyledButton component={NavLink} to="/useradmin">
<StyledButton component={NavLink} to="/users/permission-search" />
Copy link
Member

Choose a reason for hiding this comment

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

Hey hey, this added nav is resulting in an empty nav element.

image

Can we remove this line? Results in looking like this:

image

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for pointing that out @trillium!
I made the changes. Please take another look.

@vorleakyek vorleakyek requested a review from trillium February 11, 2025 00:45
@JackHaeg
Copy link
Member

JackHaeg commented Feb 11, 2025

@vorleakyek When using the "Project Leads" search, can a user also search for a project name and get a list of users who are linked to the project that has been searched?

@vorleakyek
Copy link
Member Author

@JackHaeg, as discussed in the meeting, the current search for the project lead is based on the "name or project". So, searching for name or the project name would look like this. Thanks!
image

@trillium
Copy link
Member

trillium commented Feb 11, 2025

Chiming in here to note that there's still some issues with how this is all coming together. To be clear, @vorleakyek this isn't on you. I think there are some missing TODO items that we need to also reference. I'll add them to the epoch.

When a user is navigated from permissions search it isn't brining up the projects they're listed as leads on. That is only showing up when we navigate via User Search

image image image
file: client/src/components/user-admin/UserManagement.jsx

@vorleakyek
Copy link
Member Author

Thanks for pointing that out, @trillium! So, this issue will be handled in a follow-up ticket. Is that correct?

@JackHaeg
Copy link
Member

JackHaeg commented Feb 11, 2025

@vorleakyek I just spoke with @trillium and we'd prefer for this fix to be completed as part of the existing issue / PR. I have created an additional To-Do item on the original issue that follow's Trillium's feedback above.

Please let us know if you have additional questions or concerns! Also, great work on this so far - we are super excited to get this feature into the product :)

@vorleakyek
Copy link
Member Author

@JackHaeg, okay. I'll take a look later this week. Thanks for the confirmation!

@trillium
Copy link
Member

Hey @vorleakyek , it looks like you've passed setUserToUdit into UserPermissionsSearch already. I'm not sure why it isn't applying correctly yet, taking a look now.

@trillium
Copy link
Member

Looking into this, I think I've found the problem:

The API is returning project names, not project IDs in UserPermissionSearch

image

But in the user window that renders projects it returns project IDs

image

UserToEdit expects an array of projectId and is getting an array of projectName so it isn't displaying any of the projects

@trillium
Copy link
Member

File in backend/controllers/user.controller.js at UserController.projectLead_list needs to return both name and projectId

Frontend needs to adapt to manage an object that has { name: string, projectId: string } format.

@vorleakyek
Copy link
Member Author

Thanks for finding the issue, @trillium!
I'll update the data returned from the API and adjust the frontend accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hook in APIs to filtered permissions data component
5 participants