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

feat: Edit a User #3664

Merged
merged 21 commits into from
Sep 17, 2024
Merged

feat: Edit a User #3664

merged 21 commits into from
Sep 17, 2024

Conversation

RODO94
Copy link
Contributor

@RODO94 RODO94 commented Sep 12, 2024

What does this PR do?

This PR deals with the Trello ticket: https://trello.com/c/7MAKUC2N/3011-edit-user-in-editor

And follows on from work done in this ticket: https://trello.com/c/piMcGsJa/2973-add-user-in-editor

I am adding an edit feature to the Team Members area, where a platformAdmin can update the first name, last name, and email of a user.

journey
title Permission Levels
   section PlatformAdmin
     Edit all users: 5
     Add a teamEditor:5 
   section TeamEditor
     Edit all users: 1
     Add a teamEditor: 5 
Loading

The work has used things built on for adding a user, and changes the relevant modal component from AddTeamEditorModal to EditorUpsertModal relying on the update/insert portmanteau.

I have also added a Permissions check at the edit button component level, incase we want more nuanced permissions in the future.

Test Coverage:

After chatting to @jamdelion about it, we said there's probably Modal specific tests here which are not contextually linked to editing user, and more about how the modal operates. Scope to add this to a Modal specific test file which would make more sense if we were to reuse the component for other things like Adding a Team, or Adding a Flow in future.

@RODO94
Copy link
Contributor Author

RODO94 commented Sep 12, 2024

Edit user in Editor

Copy link

github-actions bot commented Sep 12, 2024

🤖 Hasura Change Summary compared a subset of table metadata including permissions:

Updated Tables (1)

  • public.users permissions:

    insert select update delete
    platformAdmin /
    2 added column permissions
    insert select update
    platformAdmin ➕ first_name
    ➕ last_name

Copy link

github-actions bot commented Sep 12, 2024

Removed vultr server and associated DNS entries

Copy link
Contributor

@DafyddLlyr DafyddLlyr left a comment

Choose a reason for hiding this comment

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

@RODO94 Just had a very quick pass through this - appreciate it's still in draft!

I'd take a look at an approach which uses a single modal here as opposed to a modal for add and a modal for edit.

Here's my thinking on this one -

  • Duplication of code, and associated maintenance - a user is the same if being added or edited, and if we need to add a field (e.g. DOB) we'd now need to remember to update this in multiple places. Likewise, we'd need to visually maintain the two modals (sizes, colour, spacing).
  • We should be able to just populate the initialValues with either a "new" user, or an existing user. We may also need a prop to indicate if the modal is in "create" or "edit" mode.

Let me know if you want to talk this through 👍

@RODO94
Copy link
Contributor Author

RODO94 commented Sep 13, 2024

@DafyddLlyr picking up the refactors now around this, thanks for the pointers! Will keep things D.R.Y...

@RODO94 RODO94 marked this pull request as ready for review September 16, 2024 10:43
@RODO94 RODO94 requested a review from a team September 16, 2024 11:31
@jamdelion
Copy link
Contributor

LOVE seeing some mermaid in the PR explanation!! 🧜‍♀️

@RODO94 RODO94 requested a review from jamdelion September 16, 2024 13:52
Copy link
Contributor

@jamdelion jamdelion left a comment

Choose a reason for hiding this comment

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

Nice job 👍 Works well for me on the pizza!

@RODO94 RODO94 merged commit 3389060 into main Sep 17, 2024
12 checks passed
@RODO94 RODO94 deleted the rory/edit-user branch September 17, 2024 11:45
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.

3 participants