Add remove organisation member modal

[tudddorrr]

No description provided.

[claude]

PR Review: Add remove organisation member modal

This PR adds the ability for organisation owners to remove members, consisting of a new API function, a confirmation modal, and integration into the Organisation page. The implementation is clean and follows existing conventions well.

---

1. Code Quality and Best Practices

🟡 Missing test coverage

Every other modal in src/modals/__tests__/ has a corresponding test file (e.g. NewGame.test.tsx, ConfirmPlanChange.test.tsx), but RemoveMember.tsx ships without one. At minimum, it should cover:

• Rendering the modal with a member
• Disabling the Remove button until confirm is typed
• A successful removal (API call + mutate + toast + modal close)
• An API error displaying ErrorMessage

Similarly, canPerformAction.test.ts currently only tests the owner early-return path. The new REMOVE_ORGANISATION_MEMBER case (which returns false for non-owners) should be explicitly asserted.

---

🔵 Semantically misleading modalState setter

In Organisation.tsx the modal is mounted as:

<RemoveMember
  modalState={[true, () => setRemovingMember(null)]}
  ...
/>

The prop type is [boolean, Dispatch<SetStateAction<boolean>>], so inside RemoveMember any call to setOpen(false) silently ignores the false argument and instead calls setRemovingMember(null). TypeScript permits this due to parameter compatibility rules, but it is a maintenance footgun — a reader seeing setOpen(false) in the modal reasonably expects a boolean setter, not a nullifying callback.

The existing NewInvite pattern uses a real boolean state pair. A small improvement would be to keep a dedicated boolean alongside removingMember, making the intent explicit:

const [showRemoveMemberModal, setShowRemoveMemberModal] = useState(false)
const [removingMember, setRemovingMember] = useState<User | null>(null)

// to open:
setRemovingMember(member)
setShowRemoveMemberModal(true)

// in JSX:
{removingMember && showRemoveMemberModal && (
  <RemoveMember
    modalState={[showRemoveMemberModal, setShowRemoveMemberModal]}
    member={removingMember}
    ...
  />
)}

---

2. Potential Bugs or Issues

No issues found.

The success path correctly closes the modal before any further state updates, so there is no risk of a state update on an unmounted component. The optimistic mutate with false (no revalidation) is appropriate here since the delete has already been confirmed by the server.

---

3. Performance Considerations

No issues found.

tableColumns is correctly memoised with canRemoveMembers as its dependency. canRemoveMembers itself is a cheap boolean derived from a Recoil atom and does not need further memoisation.

---

4. Security Concerns

No issues found.

Only organisation owners can see or trigger the Remove button (canPerformAction returns false for all non-owner types). The email confirmation guard is consistent with the pattern used elsewhere in the codebase, and the confirmation text input adds appropriate friction for an irreversible action.

---

Summary

The implementation is solid and follows project conventions. The two items worth addressing before merge are the missing test coverage (especially for a destructive action) and the misleading modalState setter — the latter being lower priority since it works correctly at runtime.