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

Filtered permissions data comp #1808

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

Filtered permissions data comp #1808

wants to merge 4 commits into from
+297 −2

Conversation

jng34
Copy link
Member

@jng34 jng34 commented Oct 29, 2024

Fixes #1752

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

  • Created "User Permission Search" page to replace "User Management" page
  • Created new permission search feature allowing users to search for Admins or Project Leads
  • Created two new slugs /users/permission-search/admin and /users/permission-search/projects corresponding to each search tab
  • By default, Admins will be filtered alphabetically by first name and Project Leads will be filtered alphabetically by Project name and by then first name
  • WHY: As per the overview in Create the "User Permission Search" screen #1752, there is no way for admins to see a list of other admins on the VRMS app, or a list of users with Project Managment (PM) access to specific projects.

Screenshots of Proposed Changes Of The Website

Visuals before changes are applied unchanged image
Visuals after changes are applied Screenshot 2024-10-23 111629 Screenshot 2024-10-23 211005 Screenshot 2024-10-24 230502

@github-actions GitHub Actions
Copy link

Want to review this pull request? Take a look at this documentation for a step by step guide!

From your project repository, check out a new branch and test the changes.

git checkout -b jng34-filteredPermissionsDataComp development
git pull https://github.com/jng34/VRMS.git filteredPermissionsDataComp

@JackHaeg JackHaeg added the time-sensitive should be solved as soon as possible label Oct 29, 2024
@vorleakyek vorleakyek self-requested a review October 31, 2024 02:36
vorleakyek
vorleakyek requested changes Oct 31, 2024
View reviewed changes
Copy link
Member

@vorleakyek vorleakyek left a comment

Choose a reason for hiding this comment

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

@jng34, the functionality of the "User Permission Search" page seems fine. The overall code looks good as well. I just added a few comments.

client/src/pages/UserAdmin.jsx Outdated
Comment on lines 69 to 70
return <UserPermissionSearch users={users} setUserToEdit={setUserToEdit} />;
// return <UserManagement users={users} setUserToEdit={setUserToEdit} />;
Copy link
Member

@vorleakyek vorleakyek Oct 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

From my understanding, there shouldn't be any changes to the "UserAdmin.jsx" file. Refer to the mockup Low-fi mockups created by Bonnie / Jack - see page 1.

So, the "UserPermissionSearch" component should render when clicking on the "User Permission Search" button on the "Users" page.

But, it looks like we do need the useState hooks and functions in the UserAdmin page (or a similar structure) to handle the User Permission Search. Not sure if creating a new page with the same code structure would be a good approach. But, that code is probably needed to handle the filter and make each data element renders a link to the associated user profile, which is part of #1754.

Copy link
Member Author

Choose a reason for hiding this comment

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

Forgot to change it back to UserManagement when testing the code earlier, so thank you for catching that.

I left the props users and setUserToEdit in the UserPermissionSearch component as placeholders for now so they can be used for integration with the filter permissions data component (#1754).

If there should be changes in the props that are appropriate for the filter component, just let me know.

client/src/pages/UserPermissionSearch.jsx Outdated Show resolved Hide resolved
client/src/pages/UserPermissionSearch.jsx Outdated Show resolved Hide resolved
@vorleakyek
Copy link
Member

vorleakyek commented Nov 4, 2024 •
edited
Loading

@trillium, can you confirm that the accessLevel in the dummy data object should be "admin" or "projectLead"?

const user = {
     _id: 1,
     name: {
         firstName: "John",
         lastName: "Doe"
     },
     accessLevel: "projectLead", // or "admin"
     email: "[email protected]",
     projects: ["Home Unite Us, VRMS"] 
}

From the user.model.js, the value "projectLead" is not in the enum.

accessLevel: { 
    type: String, 
    enum: ["user", "admin", "superadmin"], // restricts values to "user", "admin" and "superadmin"
    default: "user" 
  },

It looks like we have this roleOnProject property under the ProjectTeamMemberSchema.

const projectTeamMemberSchema = mongoose.Schema({
    userId: { type: String },                       // id of the user
    projectId: { type: String },                    // id of the project
    teamMemberStatus: { type: String },             // Active or Inactive
    vrmsProjectAdmin: { type: Boolean },            // does this team member have admin rights to the project in VRMS?
    roleOnProject: { type: String },                // Developer, Project Manager, UX, Data Science
...})

cc: @jng34, @JackHaeg

@JackHaeg JackHaeg mentioned this pull request Nov 5, 2024
Open
@JackHaeg
Copy link
Member

JackHaeg commented Nov 5, 2024

Per @trillium recommend leaving values discussed above as-is, and making necessary adjustments in: #1801

@JackHaeg JackHaeg mentioned this pull request Nov 5, 2024
Open
7 tasks
@vorleakyek vorleakyek self-requested a review November 6, 2024 22:27
@vorleakyek
Copy link
Member

image

@trillium, can you confirm this is okay, or should this PR include the changes in #1798?
It should link to "/users". But, for the purpose of testing without the changes in the other PR, it uses "/users/permission-search" instead.

cc: @JackHaeg, @jng34

@vorleakyek vorleakyek mentioned this pull request Nov 11, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vorleakyek vorleakyek Awaiting requested review from vorleakyek

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
time-sensitive should be solved as soon as possible
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Create the "User Permission Search" screen
3 participants
@jng34 @vorleakyek @JackHaeg