fix(backend): rename users table to app_users (closes #418) #419
Reference in New Issue
Block a user
Delete Branch "feat/issue-418-rename-users-table-to-app-users"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #418. Unblocks #407 (REFACTOR-1).
What
Renames the auth-account table from
userstoapp_usersso that the database schema aligns with theAppUserJava entity and the canonicaluserpackage coming in REFACTOR-1. Removes the schema-level confusion with thepersonstable that AUDIT-4 (#391) flagged as the top blocker for the big-bang restructure.Migration scope
V60__rename_users_to_app_users.sqlperforms five renames:usersapp_usersusers_groupsapp_users_groupsapp_users_groups.user_idapp_users_groups.app_user_idcomment_mentions.user_idcomment_mentions.app_user_idpassword_reset_tokens.user_idpassword_reset_tokens.app_user_idOnly columns whose name is literally
user_idare renamed. Semantic FK columns keep their names because the role they describe is the documentation, not the type:audit_log.actor_id(actor of an audit event — also referenced byidx_audit_log_actor_idandidx_audit_log_rollup)notifications.recipient_iddocument_versions.editor_iddocument_comments.author_idtranscription_blocks.created_by/updated_bytranscription_block_versions.changed_bydocument_annotations.created_byocr_training_runs.triggered_byinvite_tokens.created_bygeschichten.author_idPostgreSQL automatically updates FK constraint metadata when the referenced table is renamed, so no
DROP CONSTRAINT … ADD CONSTRAINTcycle is needed.Code changes
AppUser.java—@Table(name = "app_users")and@JoinTable(name = "app_users_groups", joinColumns = @JoinColumn(name = "app_user_id"), …)DocumentComment.java— mention join column →app_user_idPasswordResetToken.java— FK join column →app_user_idAuditLogQueryRepository.java— three native queryLEFT JOIN users u …→LEFT JOIN app_users u …MigrationIntegrationTest,AuditLogQueryRepositoryIntegrationTest,AuditLogQueryRepositoryContributorsTest,AuditLogQueryRepositoryRolledUpTestrewritten fromINSERT INTO users→INSERT INTO app_usersOut of scope
AppUseris not changed — theAppprefix exists deliberately to keep auth accounts from being conflated with historical Persons (per project convention)usersbecause the rename hadn't run yet at that point in the migration timeline; Flyway runs them in order, so those references remain validscripts/schema.sqlsnapshot (separate audit-flagged stale-snapshot item, out of scope here)Test plan
RenameUsersToAppUsersMigrationTest(Testcontainers + JdbcTemplate) verifies the post-V60 schema:app_usersexists,usersdoes not, the threeapp_user_idcolumns exist, andaudit_log.actor_idis not touched (regression guard)./mvnw test— 1489 tests across 92 classes, 0 failures, 0 errors./mvnw clean package -DskipTests— BUILD SUCCESSDecision context
This implements Option A from the decision matrix in #418. Rationale and trade-offs vs Options B (rename entity to
User) and C (glossary-only) are recorded as a comment on #418.Markus Keller — Senior Application Architect
Verdict: Approved
This is exactly the kind of low-risk, high-clarity refactor I want to see before bigger restructures land. The schema-level confusion between
usersandpersonswas a real navigation hazard for new readers, and the rename eliminates it without expanding scope.What I like
DROP CONSTRAINT … ADD CONSTRAINTcycle is needed. Correct.actor_id,recipient_id,editor_id,author_id,created_by, etc.). This is the level of in-migration documentation I want — future me will not have to dig through PR descriptions to understand the boundary.user_idcolumns and leaving role-named FKs alone is the right call.audit_log.actor_iddescribes the role (the actor of the event), not the type — the column name is documentation, not redundant typing. The migration test assertsactor_idsurvives, which is a clean regression guard.AppUserJava entity name (rather than renaming toUser) is the right trade-off given the project convention thatPerson≠AppUser. This was the deliberate choice in #418's Option A and the PR body is explicit about it. Good decision discipline.Minor observations (not blockers)
userstable name. This is correct — Flyway runs migrations in order, so those statements were valid at the time. PR body calls this out. No action needed; flagging only because someone scanninggrep -r users backend/src/main/resources/db/migrationwill see them and need this context. The migration header comment already explains it implicitly.scripts/schema.sqlsnapshot is stale and out of scope. PR body acknowledges this with a forward reference to the audit-flagged item. Approved as-deferred — do not expand scope here.spring.jpa.hibernate.ddl-auto=validate(ornone) in the prod-ish profiles, which would catch a mismatch between the entity@Table(name = "app_users")and the DB if either side drifted. Worth confirming once viagrep ddl-autobut not a PR blocker.Architecture-tier checklist
AppUser,DocumentComment,PasswordResetToken)AuditLogQueryRepository)Ship it.
Felix Brandt — Senior Fullstack Developer
Verdict: Approved
Clean, focused, well-tested. The PR does exactly what the title says — nothing more, nothing less. TDD evidence is present (the migration test class exists alongside the migration), and the test names read as sentences. Good work.
TDD evidence
RenameUsersToAppUsersMigrationTesthas five tests, each with a single behavior:v60_appUsersTableExists_andUsersTableIsGonev60_appUsersGroupsJoinTableExists_withRenamedColumnv60_commentMentionsColumnRenamedv60_passwordResetTokensColumnRenamedv60_auditLogActorIdRemains— the regression guard for the deliberately-untouched columnsv60_<thing>_<expected>pattern. They read as sentences. When one fails in CI, the developer knows exactly what broke without opening the file.tableExists()andcolumnExists()helpers are extracted — no duplicatedinformation_schemaqueries. Clean.Naming and clarity
V60__rename_users_to_app_users.sqlreveals intent at the directory listing.persons), not what (which is obvious from the SQL). Exactly the kind of comment that should exist.@Table(name = "app_users")annotation is updated.@JoinTable(name = "app_users_groups", joinColumns = @JoinColumn(name = "app_user_id"), …)is consistent.Test fixture rewrites
The four test classes that had
INSERT INTO users (…)literals were all updated toINSERT INTO app_users (…). This is mechanical but necessary, and the diff confirms every reference flipped:AuditLogQueryRepositoryContributorsTestAuditLogQueryRepositoryIntegrationTestAuditLogQueryRepositoryRolledUpTestMigrationIntegrationTest(V44 email constraint tests +insertUser()helper)Minor — non-blocking
AuditLogQueryRepository— threeLEFT JOIN users ulines becameLEFT JOIN app_users u. The aliasuis fine since it stays local to the query, but if you ever extract this to a helper or constant, considerLEFT JOIN app_users aufor readability when the query grows. Don't change it now../mvnw testruns and stays green on this branch before merge. (Confirmed by the test plan checkbox.)What I'd flag if I saw it (and didn't)
Implementation code without a failing test first.The migration test class was added in this PR alongside the migration. Red→green discipline preserved.Comments explaining what the SQL does.The header comment explains why, not what. Correct.Optional.get()or raw exceptions.LGTM. Merge it.
Tobias Wendt — DevOps & Platform Engineer
Verdict: Approved with one operational note
Pure DDL rename via Flyway, single forward migration, no infra changes. From an ops perspective this is about as low-risk as a schema change gets — but I want to flag one operational consideration so future-me (or future-you at 2am) is not surprised.
What's solid
MigrationIntegrationTest). Standard Familienarchiv pattern, no surprises.postgres:16-alpineis pinned everywhere. No version drift concerns../mvnw testruns all 92 test classes including the newRenameUsersToAppUsersMigrationTest. If the migration breaks, CI catches it before prod.Operational note (not a blocker)
Rollback strategy is forward-only. If you deploy this to prod and need to revert, you cannot just redeploy the previous JAR — Flyway will see V60 has already run and the previous JAR's entities still expect
users/users_groups/user_id. A real rollback requires either:ALTER TABLE app_users RENAME TO users; ...on the prod DB followed by aflyway repairto clear the V60 entry from the history table.This is normal for Flyway forward-only migrations and not a reason to block the PR — but it does mean the deploy needs:
app_users_groupslookups during the renameThe rename itself takes milliseconds —
ALTER TABLE … RENAMEis metadata-only in Postgres, no table rewrite. So the application downtime window is essentially the JAR restart, not the migration runtime. Good.Backup verification
Worth running your monthly pg_dump → restore test on a backup taken after this deploys, just to confirm the new schema names round-trip cleanly through your backup pipeline. This is normal practice for any migration that renames a table — not specific to this PR.
Self-hosted compose / production checklist
Approved. Take a fresh pg_dump before deploying to prod and you're set.
Elicit — Senior Requirements Engineer
Verdict: Approved
Working in Brownfield mode. The PR is a refactor with a single, well-bounded requirement: align the schema name with the entity name to remove a documented source of reader confusion (issue #418). I am evaluating it against the Definition of Ready / Definition of Done and the issue-quality lens.
Issue-quality audit (#418 → PR #419)
personstable is named explicitlyapp_usersexists,usersdoes not, three FK columns renamed, semantic FKs untouchedThis is the kind of issue-to-PR traceability I wish more refactors had.
Definition of Done check
RenameUsersToAppUsersMigrationTest)fix(backend): rename users table to app_users)v60_auditLogActorIdRemainsproves we did NOT touch what we said we would not touch)Glossary impact
This PR strengthens the project glossary by removing a long-standing ambiguity:
Person(historical entity, never has an account) — tablepersonsAppUser(auth account, never a historical entity) — wasusers, nowapp_usersThe rename makes the boundary visible at the schema layer, which is what the project's persona separation guidance (Person ≠ AppUser) has always asked for. Approved as a glossary-tightening change.
Open questions / TBD
AppUsereventually be renamed for further symmetry? Per the PR body and #418's decision matrix, the answer is "no" —AppUserkeeps its name deliberately to prevent conflation withPerson. This is the correct, documented decision. No follow-up needed.scripts/schema.sqlsnapshot is stale (called out in PR body). This is a separate audit-tracked item, not a blocker for this PR. Defer.Scope discipline
The PR resists scope creep. It does not:
scripts/schema.sqlsnapshot (separate tracked item)Resisting these temptations is exactly the right move for a refactor PR. Ship it.
Nora "NullX" Steiner — Application Security Engineer
Verdict: Approved
This PR is a pure DDL rename with no security-relevant logic changes. I checked the security-sensitive surfaces: authentication lookups, password reset tokens, comment mentions (which determine who is notified), and the audit log query layer (which is read by the activity dashboard). All correct.
Security surface review
1.
AppUserentity rename — auth-account integrity preserved@Table(name = "app_users")updated,@JoinTable(name = "app_users_groups", joinColumns = @JoinColumn(name = "app_user_id"), ...)updated. The relationship betweenAppUserandUserGroup(which carries theSet<String> permissions) is intact.@RequirePermissionenforcement.PermissionAspectcontinues to readcurrentUser.getGroups().stream().flatMap(g -> g.getPermissions().stream())exactly as before. Authorization is not weakened.2.
PasswordResetToken.user_id→app_user_idrename@JoinColumn(name = "app_user_id", nullable = false)is preserved.nullable = falseis critical — a nulluseron a password reset token would be exploitable. This constraint survives the rename. Good.3.
comment_mentions.user_id→app_user_idrename@JoinTableinDocumentCommentis updated toinverseJoinColumns = @JoinColumn(name = "app_user_id"). Correct mapping preserved.4.
AuditLogQueryRepositorynative queriesLEFT JOIN users u ON u.id = a.actor_idjoins were updated toLEFT JOIN app_users u ON u.id = ag.actor_id. The join condition (u.id = ag.actor_id) is unchanged — this is the actor-of-event resolution that the activity dashboard renders.@Query(nativeQuery = true)queries with named parameters (:limit,:documentIds). No string concatenation. SQL-injection-safe both before and after the rename.What I checked and confirmed clean
Optional.get()introduced on auth lookupsnullable = falseonPasswordResetToken.userjoin column preservedcomment_mentionsjoin table cardinality preserved (composite key still oncomment_id+app_user_id)Migration safety
The migration uses
ALTER TABLE … RENAME TOandALTER TABLE … RENAME COLUMN. Postgres updates FK constraint metadata in place — no constraint is dropped, so there is no transient window where referential integrity is unenforced. This is important: aDROP CONSTRAINT / ADD CONSTRAINTcycle would briefly allow orphan rows;RENAMEdoes not. Good DDL choice.Regression coverage
RenameUsersToAppUsersMigrationTest.v60_auditLogActorIdRemainsis a security-relevant regression guard. If a future migration accidentally renamedactor_idtoapp_user_id, the audit log would lose the ability to attribute events to actors — which would silently degrade the audit trail. This test prevents that.CWE check (none triggered)
Approved from a security standpoint. No findings.
Sara Holt — Senior QA Engineer
Verdict: Approved
Test coverage for this PR is exactly where it should be: a dedicated migration test asserts the schema delta, a regression guard asserts what was deliberately not touched, and the existing test suite was updated to match the new column names. The pyramid is honored — this is integration-layer coverage via Testcontainers, which is the only layer that can actually verify a Flyway DDL.
Test pyramid placement — correct
RenameUsersToAppUsersMigrationTest(5 tests, real Postgres)MigrationIntegrationTest,AuditLogQueryRepositoryIntegrationTest,AuditLogQueryRepositoryContributorsTest,AuditLogQueryRepositoryRolledUpTestupdatedINSERT INTO users→INSERT INTO app_users.Test quality
v60_appUsersTableExists_andUsersTableIsGone,v60_auditLogActorIdRemains. When CI fails, the failure name tells the developer exactly what broke. Good.tableExists()andcolumnExists()factor out theinformation_schemalookups. No duplicated query strings across tests. Good.@DataJpaTest+PostgresContainerConfig+FlywayConfig. No H2 substitute (which would not have caught a real DDL bug). Good.@AutoConfigureTestDatabase(replace = NONE)preserves the Testcontainers Postgres rather than swapping in an embedded DB. Correct annotation.The regression guard — this is the test I most wanted to see
v60_auditLogActorIdRemainsasserts:This is a permanent fixture. If a future PR accidentally extends the rename to
actor_id(because someone thought "we should be consistent"), this test fails with a clear name. The deliberate decision documented in the PR body is now codified as a test. Exactly right.Test fixture updates — clean and complete
I cross-checked the diff against every
INSERT INTO usersliteral that needed flipping:MigrationIntegrationTest.v44_emailNotNullConstraint_rejectsInsertWithNullEmail— flippedMigrationIntegrationTest.v44_emailUniqueConstraint_rejectsDuplicateEmail— both INSERTs flippedMigrationIntegrationTest.insertUser()helper — flipped (this propagates to every test using the helper)AuditLogQueryRepositoryContributorsTest— both@Sqlblocks flipped (single-user and 5-user fixtures)AuditLogQueryRepositoryIntegrationTest— all four@Sqlblocks flippedAuditLogQueryRepositoryRolledUpTest.insertUserAndDocs()— bothjdbcTemplate.update()calls flippedI do not see any
INSERT INTO usersleft lurking in test code. If the full suite goes green (PR body claims 1489/1489), the diff is mechanically complete.Quality gate verification — what I want to see in CI
./mvnw test) — claimed in PR bodyRenameUsersToAppUsersMigrationTestruns and passes againstpostgres:16-alpineTestcontainer@DisabledannotationsThread.sleep()introduced (none expected for DDL test)What I'd ask Felix to add if it weren't already there
A regression guard for the columns we deliberately did NOT rename.Already there:v60_auditLogActorIdRemains.A test that the V60 migration itself runs cleanly.Implicit — Testcontainers runs all migrations including V60 before any test executes; if V60 throws, every test in the file fails.LGTM. The test plan in the PR body lines up with the test code on the branch. Approved.
Leonie Voss — Senior UX Designer & Accessibility Strategist
Verdict: Approved
Backend-only schema rename. No template changes, no CSS, no i18n keys, no user-visible surface. From a UI/UX perspective this PR is invisible to every user — which is the correct outcome for a refactor of this nature.
What I checked
.sveltefiles in the diffmessages/de.json,en.json,es.jsonfrontend/src/lib/errors.ts(no newErrorCodeto map)tailwind.configorlayout.cssAppUseris preserved per the PR body, so generated TypeScript types do not shift)Indirect UX impact
The audit-log dashboard (
AuditLogQueryRepository) renders contributor avatars and names. The native query was updated toLEFT JOIN app_users u ON u.id = ag.actor_idand continues to selectactorColor,actorName, etc. — the column projection is unchanged, so the frontend continues to receive the same shape. No re-render glitches, no missing avatars, no broken contributor strips.If the projection had silently dropped
actorColorduring the rename, contributor avatars would render as blank circles — a visible bug. Confirmed it did not.Accessibility
Nothing to evaluate. No focus order, no ARIA, no contrast, no touch targets in scope.
Brand / typography / responsive
Nothing to evaluate. No visual tokens touched.
LGTM from the design side. Approved.