-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: Edit a User #3664
Conversation
🤖 Hasura Change Summary compared a subset of table metadata including permissions: Updated Tables (1)
|
Removed vultr server and associated DNS entries |
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.
@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 👍
@DafyddLlyr picking up the refactors now around this, thanks for the pointers! Will keep things D.R.Y... |
editor.planx.uk/src/pages/FlowEditor/components/Team/components/MembersTable.tsx
Outdated
Show resolved
Hide resolved
editor.planx.uk/src/pages/FlowEditor/components/Team/components/MembersTable.tsx
Show resolved
Hide resolved
editor.planx.uk/src/pages/FlowEditor/components/Team/components/EditorUpsertModal.tsx
Outdated
Show resolved
Hide resolved
editor.planx.uk/src/pages/FlowEditor/components/Team/components/MembersTable.tsx
Outdated
Show resolved
Hide resolved
editor.planx.uk/src/pages/FlowEditor/components/Team/formSchema.tsx
Outdated
Show resolved
Hide resolved
editor.planx.uk/src/pages/FlowEditor/components/Team/tests/TeamMembers.updateEditor.test.tsx
Outdated
Show resolved
Hide resolved
editor.planx.uk/src/pages/FlowEditor/components/Team/tests/TeamMembers.updateEditor.test.tsx
Outdated
Show resolved
Hide resolved
editor.planx.uk/src/pages/FlowEditor/components/Team/tests/mocks/mockUsers.tsx
Show resolved
Hide resolved
LOVE seeing some mermaid in the PR explanation!! 🧜♀️ |
editor.planx.uk/src/pages/FlowEditor/components/Team/queries/createAndAddUserToTeam.tsx
Outdated
Show resolved
Hide resolved
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.
Nice job 👍 Works well for me on the pizza!
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.
The work has used things built on for adding a user, and changes the relevant modal component from
AddTeamEditorModal
toEditorUpsertModal
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.