feat: Remove a User #3706

RODO94 · 2024-09-18T15:22:48Z

What does this PR do?

This PR tackles the Trello Ticket: https://trello.com/c/0jbcXque/3012-remove-user-in-editor?filter=member:rorydoak2

It includes a range of refactors which build on the Add User and Edit User work. We are using a Styled Component called SettingsDialog to ensure consistent styling across different modal uses. This should be extended to a style override in the theme.ts file governing the whole editor. This has been outlined as a future refactoring PR.

Unit tests have been written to ensure removing a user follows a predictable path with correct messaging while being available at the right permission level.

I had to make an adjustment to the deleteUser mutation in planx-core. It wasn't breaking but was throwing an error due to a typo in return bool.

theopensystemslab/planx-core#516

editor.planx.uk/src/pages/FlowEditor/components/Team/tests/TeamMembers.archiveUser.test.tsx

editor.planx.uk/src/pages/FlowEditor/components/Team/components/MembersTable.tsx

RODO94 · 2024-09-20T13:12:33Z

The lock files have a huge amount of changes where ' become " - is there a way I can stop this?

editor.planx.uk/src/pages/FlowEditor/components/Team/components/MembersTable.tsx

github-actions · 2024-09-20T13:30:18Z

Removed vultr server and associated DNS entries

RODO94 · 2024-09-20T13:38:16Z

Remove user in Editor

DafyddLlyr

A few small nits and comments on PR, please see below. I think the main areas to focus on is how the refactor of the SettingsModal is handled, and how shared types should work.

Pizza working as expected, but I hit a bug related to the cache of existing users persisting - I think we encountered and resolved something similar before?

Screen.Recording.2024-09-23.at.07.39.53.mov

The lock files have a huge amount of changes where ' become " - is there a way I can stop this?

There's some linting inconsistencies across our devices - clearly husky or prettier is not configured correctly. Let's try and solve this IRL this week 👍

editor.planx.uk/src/pages/FlowEditor/components/Team/TeamMembers.tsx

editor.planx.uk/src/pages/FlowEditor/components/Team/components/MembersTable.tsx

editor.planx.uk/src/pages/FlowEditor/lib/store/team.ts

editor.planx.uk/src/pages/FlowEditor/components/Team/types.ts

editor.planx.uk/src/pages/FlowEditor/components/Team/components/SettingsModal.tsx

editor.planx.uk/src/pages/FlowEditor/components/Team/components/EditorUpsertModal.tsx

RODO94 · 2024-09-30T15:15:16Z

@DafyddLlyr for the bug, to solve the caching issue, would I need to trigger a re-rendering of the component? Would there be a reason this doesn't happen when I set state in the global store here:

export const optimisticallyUpdateExistingMember = async (
  values: UpdateEditorFormValues,
  userId: number
) => {
  const existingMembers = useStore.getState().teamMembers;
  const updatedMembers: TeamMember[] = existingMembers.map((member) => {
    if (member.id === userId) {
      return { ...values, id: userId, role: member.role };
    }
    return member;
  });
  await useStore.getState().setTeamMembers(updatedMembers);
};

See the bug is present when editing a users details as well, but not when adding, so I wonder why this is performing differently. Does Zustand not see a difference between the old array of TeamMembers and the new?

api.planx.uk/pnpm-lock.yaml

editor.planx.uk/src/pages/FlowEditor/components/Team/components/MembersTable.tsx

editor.planx.uk/src/pages/FlowEditor/components/Team/components/RemoveUserModal.tsx

editor.planx.uk/src/pages/FlowEditor/components/Team/components/SettingsModal.tsx

editor.planx.uk/src/pages/FlowEditor/components/Team/tests/TeamMembers.removeUser.test.tsx

editor.planx.uk/src/pages/FlowEditor/components/Team/TeamMembers.tsx

editor.planx.uk/src/pages/FlowEditor/components/Team/components/MembersTable.tsx

editor.planx.uk/src/pages/FlowEditor/lib/store/team.ts

editor.planx.uk/src/pages/FlowEditor/components/Team/components/SettingsModal.tsx

editor.planx.uk/src/pages/FlowEditor/components/Team/components/EditorUpsertModal.tsx

editor.planx.uk/src/ui/editor/SettingsSection.tsx

RODO94 · 2024-10-01T13:39:59Z

editor.planx.uk/src/routes/teamMembers.tsx

@@ -56,6 +56,7 @@ const teamMembersRoutes = compose(
      } = await client.query<GetUsersForTeam>({
        query: GET_USERS_FOR_TEAM_QUERY,
        variables: { teamSlug },
+        fetchPolicy: "no-cache",


@DafyddLlyr added this to resolve the caching bug when removing a user

RODO94 · 2024-10-01T13:40:29Z

editor.planx.uk/src/pages/FlowEditor/lib/store/team.ts

@@ -157,4 +158,13 @@ export const teamStore: StateCreator<
  setTeamMembers: async (teamMembers: TeamMember[]) => {
    set(() => ({ teamMembers }));
  },
+  deleteUser: async (userId: number) => {
+    try {


trycatch() block added to throw useful error on mutation failure.

RODO94 · 2024-10-01T13:41:23Z

editor.planx.uk/src/pages/FlowEditor/components/Team/styles.ts

@@ -16,3 +17,14 @@ export const StyledTableRow = styled(TableRow)(({ theme }) => ({
    background: theme.palette.background.paper,
  },
 }));
+
+export const SettingsDialog = styled(Dialog)(({ theme }) => ({


This works for now, but a future PR should refactor this into a global Editor style override in the theme.ts file

RODO94 · 2024-10-01T13:42:19Z

editor.planx.uk/src/pages/FlowEditor/components/Team/components/MembersTable.tsx

+                  </Permission.IsPlatformAdmin>
+                </TableCell>
+                <TableCell>
+                  <Permission.IsPlatformAdmin>


Permission level moved inside <TableCell> to ensure consistent UI across different permission levels

RODO94 · 2024-10-01T14:01:51Z

editor.planx.uk/src/ui/editor/SettingsSection.tsx

 const Root = styled(Box, {
  shouldForwardProp: (prop) => prop !== "background",
-})<RootProps>(({ background, theme }) => ({
+})<BoxProps & { background?: boolean }>(({ background, theme }) => ({


Had to revert bgcolor since background was a boolean to conditionally style the ServiceSettings component

I found this when running tests and seeing the other places this component was used

DafyddLlyr

Looking good - thanks for those changes 👍

RODO94 commented Sep 19, 2024

View reviewed changes

editor.planx.uk/src/pages/FlowEditor/components/Team/tests/TeamMembers.archiveUser.test.tsx Outdated Show resolved Hide resolved

RODO94 force-pushed the rory/remove-user branch from f916143 to e0879f5 Compare September 20, 2024 11:15

ianjon3s reviewed Sep 20, 2024

View reviewed changes

editor.planx.uk/src/pages/FlowEditor/components/Team/components/MembersTable.tsx Outdated Show resolved Hide resolved

RODO94 commented Sep 20, 2024

View reviewed changes

editor.planx.uk/src/pages/FlowEditor/components/Team/components/MembersTable.tsx Outdated Show resolved Hide resolved

RODO94 marked this pull request as ready for review September 20, 2024 15:31

DafyddLlyr requested changes Sep 23, 2024

View reviewed changes

DafyddLlyr mentioned this pull request Sep 28, 2024

chore: Remove deprecated /user endpoints #3737

Merged

RODO94 added 20 commits September 30, 2024 11:17

init remove button work

5961d8e

init wiring up

470f3ea

refactor: merge modal design into one dialog comp

5850037

add showEditButton bool to remove button and fix styling

489e74a

fix: unused variables

63b362c

feat: optimistic update and type change for email:null

dd0f118

init test work

c17aa24

refactor: move from 'remove' / 'delete' to 'archive'

8e6235d

testing work for basic modal function working

13937e8

move to remove naming and run success testing

40f0fda

finish success and axe testing

e927c9e

update planx-core refs

5f111fa

fix: alter TeamMembers tests for new mock member

9e35fc4

fix: alter TeamMembers tests for new mock member

242acd8

ui-fix: change colSpan to 5

3c412f8

feat: refine naming of variables

e483c88

feat: add logic to only show remove on team editor table

7675e91

remove console log

3a3653e

refactor: content refinement

fe83df1

fix: unusued vars and testid changes:

30214e2

RODO94 force-pushed the rory/remove-user branch from fdeb6aa to 30214e2 Compare September 30, 2024 10:23

RODO94 added 8 commits September 30, 2024 11:37

alter permission conditions to focus on content

2d3ab7a

introduce showRemoveButton bool

92b0118

refactor: move onClick to fns define for each action

a5bc1d6

refactor: switch test-id using index to member.id

6cc2beb

change settingsModal into styled comp

3347877

remove settingsModal

0843ae0

refactor: move styled dialog to styles file

df879ef

refactor: update types to keep type safety in adding members

df8dc58

refactor: updateEditor test refactor for new testid

8e270d5

RODO94 requested a review from DafyddLlyr September 30, 2024 15:18

DafyddLlyr reviewed Sep 30, 2024

View reviewed changes

api.planx.uk/pnpm-lock.yaml Outdated Show resolved Hide resolved

RODO94 commented Oct 1, 2024

View reviewed changes

RODO94 added 4 commits October 1, 2024 12:08

refactor: trycatch in store and new lock file

2809806

add fetchPolicy to get users for team query

5f5f4bb

refactor: yaml file resetting

5883395

refactor: use BoxProps in SettingsSection comp

92950bc

RODO94 commented Oct 1, 2024

View reviewed changes

editor.planx.uk/src/ui/editor/SettingsSection.tsx Outdated Show resolved Hide resolved

RODO94 commented Oct 1, 2024

View reviewed changes

RODO94 requested a review from DafyddLlyr October 1, 2024 13:42

alter ServiceSection to include background boolean

1f0744b

RODO94 commented Oct 1, 2024

View reviewed changes

DafyddLlyr approved these changes Oct 1, 2024

View reviewed changes

RODO94 merged commit 12f22c3 into main Oct 2, 2024
12 checks passed

RODO94 deleted the rory/remove-user branch October 2, 2024 07:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Remove a User #3706

feat: Remove a User #3706

RODO94 commented Sep 18, 2024 •

edited

Loading

RODO94 commented Sep 20, 2024

github-actions bot commented Sep 20, 2024 •

edited

Loading

RODO94 commented Sep 20, 2024

DafyddLlyr left a comment •

edited

Loading

RODO94 commented Sep 30, 2024

RODO94 Oct 1, 2024

RODO94 Oct 1, 2024

RODO94 Oct 1, 2024

RODO94 Oct 1, 2024

RODO94 Oct 1, 2024

RODO94 Oct 1, 2024

DafyddLlyr left a comment

feat: Remove a User #3706

feat: Remove a User #3706

Conversation

RODO94 commented Sep 18, 2024 • edited Loading

What does this PR do?

RODO94 commented Sep 20, 2024

github-actions bot commented Sep 20, 2024 • edited Loading

RODO94 commented Sep 20, 2024

DafyddLlyr left a comment • edited Loading

Choose a reason for hiding this comment

RODO94 commented Sep 30, 2024

RODO94 Oct 1, 2024

Choose a reason for hiding this comment

RODO94 Oct 1, 2024

Choose a reason for hiding this comment

RODO94 Oct 1, 2024

Choose a reason for hiding this comment

RODO94 Oct 1, 2024

Choose a reason for hiding this comment

RODO94 Oct 1, 2024

Choose a reason for hiding this comment

RODO94 Oct 1, 2024

Choose a reason for hiding this comment

DafyddLlyr left a comment

Choose a reason for hiding this comment

RODO94 commented Sep 18, 2024 •

edited

Loading

github-actions bot commented Sep 20, 2024 •

edited

Loading

DafyddLlyr left a comment •

edited

Loading