decide(backend): resolve users-table / AppUser-entity naming mismatch before REFACTOR-1 #418
Reference in New Issue
Block a user
Delete Branch "%!s()"
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?
Context
Precursor for REFACTOR-1 (#407) under Epic #406. Surfaced by AUDIT-4 (#391) as a C3.3 Critical fail and named explicitly as the audit's "top blocker":
The mismatch:
users(created inbackend/src/main/resources/db/migration/V1__initial_schema.sql) — pluralised, sits next topersons.AppUser(backend/src/main/java/org/raddatz/familienarchiv/model/AppUser.java) — explicitly not calledUserto honour the project's hard rule that historical Persons ≠ AppUser accounts (see user MEMORY:project_person_appuser_separation).users/personsand assumes they're related; code-first reader seesAppUserand looks for anapp_userstable that doesn't exist. Both are wrong-footed.REFACTOR-1 (#407) moves
AppUser.javafrommodel/intouser/package. The right time to also align the table name (or commit to a documentation-only fix) is before REFACTOR-1 runs — bundling a table rename inside the same PR as a 200+ class move would be uncomfortable to review, but committing to not renaming requires the same upfront decision because it changes how the newuser/package gets documented.Decision required
Option A — Rename table
users→app_users(DB migration)Method. New Flyway migration
V60__rename_users_to_app_users.sql:Plus add
@Table(name = "app_users")toAppUser.java(and update other@JoinColumnreferences).Pros.
user/package now maps cleanly: packageuser, entityAppUser, tableapp_users— three names, one concept).usersandpersonspermanently.Cons.
comment_mentions.user_id,audit_log.actor_id,users_groups,password_reset_tokens.user_id,invite_tokensconsumers,notificationsconsumers — exact list to be confirmed by\d usersbefore writing the migration).CLAUDE.mdandscripts/CLAUDE.md, the only external consumer is thescripts/schema.sqlsnapshot — which AUDIT-4 also flags as stale and slated for either regeneration or deletion. So the breakage cost is essentially zero in practice.Effort estimate. Half a day for the migration + entity annotation update + run full test suite + verify Hibernate's generated DDL still matches.
Option B — Rename entity
AppUser→UserMethod. IntelliJ rename refactor on the Java class. Add
@Table(name = "users")if not already implicit. Rename fileAppUser.java→User.java. Update all 100+ usages (IDE handles it).Pros.
UserService,UserController,UserRepositoryall gain a sisterUserentity instead of the awkwardAppUser.Cons.
org.springframework.security.core.userdetails.User). Risk of accidental wrong import in IDE autocomplete is real and recurring.project_person_appuser_separation) — theAppprefix exists specifically to keep historical Persons from being conflated with accounts. Removing the prefix re-opens the door to that confusion every time someone readsUserand mentally pictures a Person.users/personsside by side.Effort estimate. Two hours including verification.
Option C — Keep both names; document the mismatch
Method. Add a glossary entry to
docs/GLOSSARY.md(which DOC-3 (#397) is already creating) that explicitly explains:Optionally also add a
COMMENT ON TABLE users IS 'AppUser accounts — NOT historical persons. See persons.';migration so\dt+shows the disambiguation directly in psql.Pros.
Cons.
userwould be the only one with a documented mismatch.Effort estimate. 30 minutes (glossary + optional
COMMENT ON TABLE).Recommendation
Option A (table rename).
Rationale:
user, entityAppUser, tableapp_usersall align after REFACTOR-1.project_solo_llm_driven) makes a one-time DB migration low-cost — there's no other team to coordinate with, no external BI consumer, no API contract that exposes the table name.Method (once decided)
./mvnw test→ verify schema diff viapg_dump --schema-onlyis exactly the rename → open PR → merge before REFACTOR-1's branch is created.Userimport collisions → run./mvnw test→ open PR → merge before REFACTOR-1's branch is created.COMMENT ON TABLEmigration. No PR blocks REFACTOR-1.Acceptance criteria
COMMENT ON TABLEmigration merged, both before REFACTOR-1's branch is created../mvnw testis green after the change.npm run generate:apiand confirm the only diff is the type name).Definition of Done
Cross-references
tagtable as a sibling C3.3 issue (not in scope here).project_person_appuser_separation(the reasonAppUserexists at all).🏗️ Markus Keller — Application Architect
Observations
userpackage →AppUserentity →userstable) is the clean post-REFACTOR-1 target. Right now only one layer is misaligned (the table).users(id)across 11 migration files (V8, V9, V10, V12, V16, V17, V18, V19, V30, V45, V46, V58 — plus the join tableusers_groupsfrom V1). The issue estimates "6+ FK columns" — the actual number is closer to twice that. This doesn't change the recommendation, but it does mean the migration script needs a careful\d userssweep before writing.MigrationIntegrationTestdirectly hard-codesINSERT INTO users(line 449 of that file). A table rename without updating that test will break CI.users_groupsjoin table also needs renaming toapp_users_groupsand itsuser_idFK column renamed toapp_user_id. TheAppUserentity at line 72 explicitly names@JoinTable(name = "users_groups", joinColumns = @JoinColumn(name = "user_id"))— both must be updated in lock-step with the migration.users. That concern is off the list.Recommendations
user, entityAppUser, tableapp_users) is the clearest and most enforceable outcome. The solo + LLM workflow means the FK migration is a one-afternoon task, not a multi-team coordination.V60__rename_users_to_app_users.sql. Every rename in that file must happen together — table, join table, FK column names — to keep Hibernate's DDL validation green on restart.\d+ usersagainst the live schema to enumerate all FK constraints by their generated names (e.g.,fkg6fu0mfuj9eqfd9aro1nc400nn). PostgreSQL does not automatically rename FK constraint names onALTER TABLE RENAME— those constraints survive pointing at the right table, but the old name becomes confusing. Consider adding explicitALTER CONSTRAINTrenames for the FK constraints onusers_groupsto complete the cleanup.MigrationIntegrationTest— anyINSERT INTO usersorINSERT INTO users_groupshelper in that file must be updated to the new table names after V60 runs.@Table(name = "app_users")and@JoinTable(name = "app_users_groups", joinColumns = @JoinColumn(name = "app_user_id"))toAppUser.javain the same PR as V60, before REFACTOR-1 touches the package.Open Decisions (omit this section entirely if none)
None — Option A is the architecturally correct answer for this project's scale and workflow. The FK scope is larger than estimated but still finite and testable within a single PR.
👨💻 Felix Brandt — Senior Fullstack Developer
Observations
AppUser.javacurrently has@Table(name = "users")with no comment explaining the mismatch. Future readers (including future-me) will spend time wondering whether this is intentional or forgot to be cleaned up. That's the exact cost the issue is trying to eliminate.@JoinTableannotation at line 72 inAppUser.javaexplicitly references both the join table name (users_groups) and the FK column name (user_id). These are compile-time strings — if V60 renames the table but this annotation isn't updated, Hibernate's schema validation (spring.jpa.hibernate.ddl-auto=validate) will fail loudly on startup. That's the right failure mode; just make sure it's caught in the PR's test run, not in a surprise restart.CustomUserDetailsService.javaimportsorg.springframework.security.core.userdetails.User. This confirms the Option B concern is real: naming our domain entityUsercreates a genuine import collision risk right there in the security layer. Option B would force every file in the security package to either use the fully-qualified Spring class name or alias one of the twoUsertypes. That's the kind of friction that leads to wrong-import bugs three months later.UserService,UserController,UserRepositoryalready exist and read fine. TheAppprefix on the entity is the only awkward spot — and it's preferable to the import collision risk.Recommendations
@Table(name = "app_users")+@JoinTableupdate +MigrationIntegrationTeststring updates. Keep it narrow — no other changes../mvnw testbefore touching anything to get a green baseline. Write V60. UpdateAppUser.javaannotations. UpdateMigrationIntegrationTest. Run./mvnw testagain. That's the full red→green cycle for a migration change.spring.jpa.hibernate.ddl-auto=validateshould produce zero errors. Make that explicit in the PR description as the smoke test command.AppUserinto theuser/package. At that point, all three layers align:user/package,AppUserclass,app_userstable. No further naming work needed in that domain.🔒 Nora "NullX" Steiner — Security Engineer
Observations
audit_logtable usesactor_id UUID REFERENCES users(id) ON DELETE SET NULLwith an explicit comment: "GDPR right-to-erasure. Deleted users' events are retained but anonymised." The rename migration must not change thisON DELETE SET NULLsemantics. The migration should rename the column (actor_id → app_user_id) while keepingON DELETE SET NULLintact. Verify this is explicit in theALTER TABLEstatement, not left to PostgreSQL to infer.ALTER TABLE users RENAME TO app_users, the existing FK constraint names (e.g.,fkg6fu0mfuj9eqfd9aro1nc400nn) become stale-named references toapp_users. This is functionally harmless but creates the same "schema-first reader is confused" problem the issue is trying to fix. Consider explicitly renaming the FK constraints withALTER TABLE ... RENAME CONSTRAINTin V60 so\d+ app_usersshows clean, legible FK names.Recommendations
actor_id → app_user_idrename line in V60 explicitly preserving the GDPR intent:-- Renamed from actor_id; ON DELETE SET NULL is intentional (GDPR anonymisation).\d+ usersbefore writing V60 to get the exact FK constraint names, then decide whether to rename them too. Consistent naming here pays forward when auditing the schema.app_userssits next topersonsis self-documenting about the domain separation, reducing the chance that a future developer queries the wrong table in a privileged context.🧪 Sara Holt — QA Engineer
Observations
MigrationIntegrationTest.javacontains two test methods that INSERT directly into theuserstable:v44_emailNotNullConstraint_rejectsInsertWithNullEmailandv44_emailUniqueConstraint_rejectsDuplicateEmail, plus a private helperinsertUser(String email)that is also called from the V51 backfill test. All three of these will fail withrelation "users" does not existafter V60 runs — and they should, because that's the correct test failure that confirms the migration is needed.MigrationIntegrationTestto useapp_usersafter V60 lands. This is part of the same PR as the migration.actor_id,created_by,recipient_id) are independent column names in other tables and don't need to change unless the issue author chooses to do so for consistency. Worth clarifying scope in the PR description.backend/CLAUDE.md) will still pass after the migration change — no business logic is changing, just structural metadata.Recommendations
MigrationIntegrationTestthat verifies theapp_userstable exists and theuserstable does not, after all migrations run. Something like:assertThat(jdbc.queryForObject("SELECT to_regclass('app_users')", String.class)).isNotNull()and the inverse forusers. This locks in the rename permanently and will catch any rollback attempt.spring.jpa.hibernate.ddl-auto=validateis set in the test profile. If it is, a missing@Table(name = "app_users")annotation onAppUser.javawill cause the full test suite to fail with a schema validation error — which is the right gate.insertUserhelper is used from V51 tests. When renaming the table, rename that helper too or extract a named constant for the table name to prevent drift.Open Decisions (omit this section entirely if none)
comment_mentions.user_idand V8'spassword_reset_tokens.user_idreference the oldusers(id)FK. The FK target changes with the rename, but the column names (user_id) remain. Should those columns be renamed toapp_user_idfor consistency, or is the FK target rename sufficient? This is a style question, not a correctness question — both are valid. Deciding now prevents a future "why is this column calleduser_idif the table isapp_users?" question. (Raised by: Sara)⚙️ Tobias Wendt — DevOps & Platform Engineer
Observations
ALTER TABLE RENAMEin PostgreSQL acquires anAccessExclusiveLockbriefly — on a table this size (family archive, not millions of rows) that's milliseconds, not a concern.scripts/schema.sqlas the only external consumer. I checked —scripts/CLAUDE.mdconfirms this is a documentation artifact, not a runtime dependency. No external BI tool, no ETL, no replica reading the table by name. The breakage cost is truly zero.outOfOrderMigrations=falsemeans V60 must have a higher number than the current last migration (V59). That's correctly handled — V60 is the next free slot.expose: ["5432"]), so there's no concern about external tooling connecting directly to the database during the migration window.ALTER TABLE RENAMEin a production deployment, theAccessExclusiveLockwill wait for them. For a family archive with a handful of users, this is not a real risk, but worth noting in the deploy runbook.Recommendations
docker compose up -d backend→ Flyway runs V60 on startup → Hibernate validates schema with new@Table(name = "app_users")→ health check passes. This is a normal rolling deployment with no special steps.pg_dumpbefore deploying V60, even though the rename is reversible with another migration. Standard pre-deploy backup hygiene.No open decisions from my angle — this is a by-the-book Flyway rename migration. The only ops question is "did we take a backup first?" and the answer should always be yes.
📋 Elicit — Requirements Engineer
Observations
AppUser), which doesn't change. The AC item is correct but should be annotated "N/A for Option A" to prevent confusion during review.Closes #418in its description so it auto-closes on merge, and a manual cross-reference comment should be posted on #407 after merge.Recommendations
🎨 Leonie Voss — UX Designer
No UX concerns from my angle — this issue is entirely backend/database naming with no user-facing surface.
I verified: the
AppUserentity fields exposed via the API (email,firstName,lastName,color,groups,enabled, etc.) are not affected by a table rename. The OpenAPI spec, the generated TypeScript types, and any UI components consuming user data remain unchanged under Option A. No display strings, labels, or i18n keys reference the DB table name.The one indirect note: the
colorfield onAppUserdrives avatar colors in the comment thread UI. This field and its computation logic are untouched by this change — the visual identity of users in the archive UI is safe.🗳️ Decision Queue — Action Required
1 decision needs your input before implementation starts.
Schema
users → app_users, the FK target changes, but the FK column names in referencing tables (e.g.,comment_mentions.user_id,password_reset_tokens.user_id,notifications.recipient_id,audit_log.actor_id) stay as they are. Option A: rename only the table and join table, leave referencing column names alone (V60 is smaller and lower risk). Option B: also rename alluser_idcolumns in referencing tables toapp_user_idin V60 (more complete, but touches ~8 additional ALTER TABLE statements and adds more rows to update inMigrationIntegrationTest). Both are correct — it's a consistency vs. scope trade-off. (Raised by: Sara)Decision: Option A (table rename)
Implemented in PR. Branch:
feat/issue-418-rename-users-table-to-app-users.Rationale
Option A produces the cleanest post-refactor state — package
user, entityAppUser, tableapp_usersall align after REFACTOR-1 lands, and stack-symmetry is enforceable in both the Java side and the DB side.The headline downside of A (FK migration scope) turned out to be modest: PostgreSQL automatically updates FK constraint metadata when the referenced table is renamed, so the migration is just five
ALTER TABLE … RENAMEstatements. No FK drop/recreate cycle, no semantic content change, no API surface change.Options B and C were rejected:
User) would have collided withorg.springframework.security.core.userdetails.Useron every IDE autocomplete, and worse — would have removed the deliberateAppprefix that exists to stop readers from conflating auth accounts with historical Persons (perproject_person_appuser_separation). It also wouldn't have fixed the original audit finding, which is that schema-first readers seeusersnext topersonsand assume they're related.userwould be the only one with a documented mismatch, and Tobias-class evaluators (the audit's framing) skip glossaries.Scope decision: which columns get renamed
Only columns whose name is literally
user_idwere renamed toapp_user_id. Semantic FK columns kept their names:user_id→app_user_id)app_users_groups.user_id(wasusers_groups.user_id)audit_log.actor_idcomment_mentions.user_idnotifications.recipient_idpassword_reset_tokens.user_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_idactor_idwas the borderline case — the issue body's draft migration suggested renaming it, but the same body's guidance ("update column names where they say user_id") and the deliberate audit-event semantics of "actor" (referenced byidx_audit_log_actor_id,idx_audit_log_rollup, and the GDPR-erasure comment in V46) argue for leaving it. The newRenameUsersToAppUsersMigrationTestincludes a regression assertion thataudit_log.actor_idis not renamed, so any future drift is caught immediately.Verification
RenameUsersToAppUsersMigrationTestcovers all five renames + theactor_idnon-rename./mvnw clean package -DskipTests— BUILD SUCCESSAppUserkeeps its name)REFACTOR-1 (#407) is now unblocked once this and #417 (cross-domain repo violations) merge.