Survey import stress test: delete created users - #4356
Merged
Conversation
Covers the DB-level relationships (blocking vs. cascade-deleted) that constrain deleting a user, the new deleteUser service/endpoint, and wiring the survey-import stress test to clean up its throwaway users.
DELETE /user/:userUuid (system-admin only) reassigns the target's records to the acting admin, blocks on owned surveys/messages (both a hard DB constraint), and rejects self-delete and deleting the last system admin. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
_userFetcher wraps fetchUserByEmail/fetchUserByUuid/fetchUserByUuidWithPassword and updateUser, then calls _attachAuthGroupsAndInvitationToUser to fetch auth groups and invitation status. It previously dropped the caller's client/tx, so that read always ran on a separate `db` connection outside any transaction the caller had opened -- a TOCTOU gap surfaced by deleteUser's last-system-admin check, which reads isSystemAdmin() off a user fetched inside its own tx. _userFetcher now takes a clientArgIndex (default 1, matching every wrapped fetchFn's own `(id, client = db)` shape) and forwards args[clientArgIndex] as `t`. updateUser is wired with index 4, since its wrapped _updateUser has a different signature (user, surveyId, userToUpdate, profilePicture, client) -- using the default index 1 there would have misread surveyId as the client. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Needed so the stress test can clean up the throwaway accounts it provisions, via the new DELETE /api/user/:userUuid endpoint. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
createUser's uuid is now captured as soon as creation succeeds (not just on overall success) and carried through to the run's results, so cleanupUsers can delete every account the run created, mirroring the existing survey cleanup and gated by the same --keep flag. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The warning still claimed throwaway accounts "cannot be deleted afterward" and pointed at test/load/README.md for details -- both now wrong/misleading since this same feature branch made account cleanup automatic and updated the README to say so. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- deleteUser now uses SurveyManager.countOwnedSurveys/fetchAllSurveyIds (unfiltered, FK-scoped) instead of the UI listing query (fetchUserSurveys), which excluded templates/temporary surveys and required current auth-group membership -- letting owned surveys/records slip past the pre-delete checks and hit raw FK-violation 500s instead of a clean 409. - Add the five appErrors i18n keys deleteUser throws (EN only, matching existing unregistered-key precedent for other locales). - Update the design spec's key names (flat, not dotted) and the owns-surveys error's params to match the fix above. - Log an audit line in deleteUser recording who deleted whom. - test/load/README.md: move the no-longer-a-limitation cleanup paragraph out of Limitations; fix stray "--" to em dash. - surveyImportStressTest.ts: fail CI when post-run survey/user cleanup doesn't fully succeed (not just when imports fail), skipped when --keep is passed.
Committing alongside the design spec it implements, for the same reason the spec is tracked: a durable record of what was planned. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces an admin-only user-account deletion capability on the backend and wires the load/stress-test tooling to capture and delete the throwaway users it creates, preventing user-row buildup across runs.
Changes:
- Add
UserService.deleteUserand expose it viaDELETE /api/user/:userUuid(admin-only), with checks for owned surveys/messages and record-owner reassignment. - Update the survey import stress test to capture created
userUuids, delete users after the run (unless--keep), and reflect the new behavior in docs/config. - Extend load-test HTTP client helpers and tests to support
deleteUserand returnuuidfromcreateUser.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
server/modules/user/service/userService.js |
Implements transactional admin delete logic with ownership checks and record reassignment. |
server/modules/user/api/userApi.js |
Adds admin-only DELETE /user/:userUuid API route. |
server/modules/user/manager/userManager.js |
Ensures user “enrichment” runs with the correct transaction client. |
server/modules/survey/manager/surveyManager.js |
Re-exports fetchUserSurveys from SurveyRepository. |
core/i18n/resources/en/appErrors.js |
Adds new appErrors strings for delete-user failure cases. |
test/load/lib/httpApi.ts |
Makes createUser return uuid; adds deleteUser client call. |
test/load/lib/httpApi.test.ts |
Updates/extends tests for createUser uuid parsing and deleteUser. |
test/load/lib/report.ts |
Extends load-test result type to include optional userUuid. |
test/load/surveyImportStressTest.ts |
Captures userUuid, adds cleanupUsers, and cleans up users after runs. |
test/load/surveyImportStressTest.test.ts |
Adds regression coverage for userUuid reporting and cleanupUsers. |
test/load/lib/config.ts |
Updates --keep help text and notes to include user cleanup. |
test/load/README.md |
Updates documentation to reflect user cleanup behavior and limitations. |
docs/superpowers/specs/2026-08-13-user-account-deletion-design.md |
Adds design spec for admin user deletion and stress-test cleanup. |
docs/superpowers/plans/2026-08-13-user-account-deletion.md |
Adds detailed implementation/verification plan for the feature. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This reverts commit f707a17.
|
Contributor
|
Tick the box to add this pull request to the merge queue (same as
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.