TheCombine icon indicating copy to clipboard operation
TheCombine copied to clipboard

Implement cascading deletion of user roles and edits with project visibility

Open Copilot opened this issue 4 months ago • 2 comments

User deletion left orphaned UserRole and UserEdit records in the database. The deletion dialog provided no visibility into which projects would be affected.

Backend Changes

  • UserController: Enhanced DeleteUser to cascade delete all associated UserRole and UserEdit records. Added validation to prevent admin user deletion (returns 403).
  • New endpoint: GET /v1/users/{userId}/projects returns project names and roles for deletion confirmation.
  • UserProjectInfo DTO: Simple model containing projectId, projectName, and role.
// Cascade deletion implementation
foreach (var (projectId, userRoleId) in user.ProjectRoles)
    await _userRoleRepo.Delete(projectId, userRoleId);

foreach (var (projectId, userEditId) in user.WorkedProjects)
    await _userEditRepo.Delete(projectId, userEditId);

Frontend Changes

  • ConfirmDeletion dialog: Fetches and displays user's project memberships with roles before deletion. Shows loading/error states appropriately.
  • backend/index.ts: Added getUserProjects() function and UserProjectInfo interface.

Tests

Added 5 test cases covering:

  • Cascading deletion of roles and edits
  • Admin user deletion prevention
  • Project listing endpoint (happy path, auth, not found)
Original prompt

This section details on the original issue you should resolve

<issue_title>DeleteUser should interact with project roles and user edits</issue_title> <issue_description>A site admin, from site settings can delete any non-admin user. Several issues:

  • There's a dialog confirming deletion, but no warnings what projects the user is on or what edits they have
  • When the user is deleted, their user roles and user edits are orphaned in the database
  • The frontend gui prevents admins from deleting other admins, but the backend controller doesn't prevent it

~Currently it requires a maintenance script to delete projects and the script handles all the cleanup. Perhaps it should be the same with users. From the gui, would we want the ability to disable accounts instead of deleting them (like archiving a project)?~

Todo:

  • In the confirm-deletion dialog, list projects the user is in and what their role is in each project.
  • When the user is deleted, also:
    • delete all their user roles
    • update the projects to remove those roles
    • delete all the user edits</issue_description>

Comments on the Issue (you are @copilot in this section)

  • Fixes sillsdev/TheCombine#3875

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.


This change is Reviewable

Copilot avatar Oct 28 '25 18:10 Copilot

⚠️ Commit Message Format Issues ⚠️ commit c84025e3c1: 1: T1 Title exceeds max length (74>72): "Add frontend support for displaying user projects in deletion confirmation"

github-actions[bot] avatar Oct 28 '25 20:10 github-actions[bot]

Codecov Report

:x: Patch coverage is 77.27273% with 15 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 74.66%. Comparing base (ede2527) to head (517338a). :warning: Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
...ts/SiteSettings/UserManagement/ConfirmDeletion.tsx 0.00% 14 Missing :warning:
Backend/Controllers/UserController.cs 97.91% 0 Missing and 1 partial :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3971      +/-   ##
==========================================
+ Coverage   74.58%   74.66%   +0.07%     
==========================================
  Files         293      293              
  Lines       10817    10890      +73     
  Branches     1353     1364      +11     
==========================================
+ Hits         8068     8131      +63     
- Misses       2354     2362       +8     
- Partials      395      397       +2     
Flag Coverage Δ
backend 85.66% <98.07%> (+0.20%) :arrow_up:
frontend 66.11% <0.00%> (-0.12%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Oct 28 '25 20:10 codecov[bot]

Failing to get user info should fail the contents.

@jasonleenaylor reviewed 2 of 5 files at r2, 12 of 13 files at r3, all commit messages....

Fixed in e7d9591. When getUserProjects fails, the dialog now:

  • Sets a loadFailed state flag
  • Displays an error message in red ("Failed to load user projects...")
  • Keeps the Delete button disabled
  • Shows an error toast instead of a warning

Previously, the catch block only logged and showed a warning toast, leaving projInfo as undefined which caused the UI to show "Loading..." indefinitely while the button remained disabled without clear indication of the error state.

Copilot avatar Nov 20 '25 19:11 Copilot