feat(user): add name and status fields per ADR 006 (#45, #83)#86
Open
martinydeAI wants to merge 5 commits into
Open
feat(user): add name and status fields per ADR 006 (#45, #83)#86martinydeAI wants to merge 5 commits into
martinydeAI wants to merge 5 commits into
Conversation
Lands the User-entity work of ADR 006 — a `UserStatus` PHP enum (`pending | approved | blocked`) and a matching `status` column on the entity, plus the long-pending `name` display column. The `UserManager::createUser()` service grows a required `name` argument and an optional `status` (defaults to `Approved` for the console / fixture path; the registration flow in #62 will pass `Pending`). The `app:user:create` console command grows a third `name` argument; fixtures seed Alice + Bob with display names. A single migration adds both columns with a default value so existing rows in long-running environments backfill cleanly: `name = ''` and `status = 'approved'` (historic rows are assumed already-trusted accounts). Combines #45 (User.name) and #83 (UserStatus enum + status field) in one PR, since the migration is most coherent as a single ALTER TABLE and the entity tests fight if either field lands without the other. Sequenced as PR 1 of the User management milestone plan (`docs/plans/user-management-milestone.md`, written locally) — PR 3 (UserChecker, #63) and PR 4 (Registration, #62) stack on top of this branch with `do-not-merge` labels until it merges. Closes #45. Closes #83. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Jun 19, 2026
martinyde
requested changes
Jun 19, 2026
| { | ||
| $user = $schema->getTable('user'); | ||
| $user->addColumn('name', Types::STRING, ['length' => 255, 'notnull' => true, 'default' => '']); | ||
| $user->addColumn('status', Types::STRING, ['length' => 32, 'notnull' => true, 'default' => 'approved']); |
Contributor
There was a problem hiding this comment.
Should default be "approved" in db context?
@tuj ?
Updated migration comment to remove ADR reference.
Per review on PR #86 - keep the CHANGELOG entry to what was added and the issue references; details belong in the PR description. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
martinyde
requested changes
Jun 19, 2026
Per review on PR #86 - apply the test-comment convention to every test method in UserTest, UserManagerTest, and UserRepositoryTest. Each method now opens with a single-line "Tests ...", "Ensures ...", or "Verifies ..." comment naming what it asserts. Pure documentation change - no test logic touched. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
2 participants
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.
Links to issues
Closes #45.
Closes #83.
Description
Adds the User-entity work of ADR 006:
App\Enum\UserStatus(pending | approved | blocked).App\Entity\User:name: string(display name)and
status: UserStatus.UserManager::createUser()now requires anameargument andaccepts an optional
status(defaults toApprovedfor theconsole + fixture path; the registration flow in feat: anonymous self-signup with email-domain allow-list #62 will pass
Pendingexplicitly).app:user:createconsole command grows a thirdnamepositional argument.
UserFixturesseeds Alice + Bob with display names.(
name = '',status = 'approved') so existing rows inlong-running environments backfill cleanly.
Combines two issues (#45 User.name + #83 UserStatus enum + status
field) in one PR since the migration is most coherent as a single
ALTER TABLE and the entity tests would fight if either landed
without the other.
Screenshot of the result
n/a — schema + service-signature change, no UI.
Checklist
Verified locally:
task coding-standards-check— PHP CS Fixer, Twig CS Fixer,Prettier (YAML / JS / CSS), markdownlint, composer validate +
normalize — all green.
task test-coverage— 51 tests, 186 assertions, 100 %coverage.
Additional comments or questions
The migration uses column-level SQL defaults so that environments
already carrying user rows backfill without a separate UPDATE step.
The ORM metadata does not declare those defaults (so new rows go
through the constructor / setters as before); the discrepancy is
intentional and only visible to
doctrine:schema:validate— notenforced in CI.
UserManager::createUser()'snameargument is requiredrather than nullable-with-default. The alternative (empty-string
default) was considered and rejected: a blank display name would
bleed into the admin user list (PR 5, #64+#85) as empty cells with
no way to tell apart a deliberately-empty row from an
incorrectly-created one. Required-with-empty-allowed is the
strictest contract that still keeps the call sites ergonomic.
Details - AI specificities
docs/adr/006-user-approval-and-account-state.md(lands via PR docs: add ADR 004 — user registration, approval, and account state #61). This PR realises the schema decision from the
ADR — the rest of the milestone work (UserChecker, registration,
approval queue, scoped admin list, profile UI) stacks on top.
docs/plans/user-management-milestone.mdwas writtenlocally (not committed in this PR) and walks through all six
stacked PRs and their dependency graph.
test edit is either a signature update forced by
UserManager::createUser()'s newnameargument(
UserManagerTest× 4,UserCreateCommandTest× 2), or apure-additive extension of assertions on the new fields
(
UserTest,UserFixturesTest,UserRepositoryTest).User— deferred per ADR 005 / 006.PR 5 (feat: approval queue for domain managers #64 + feat: scoped user-management list view for admins and domain managers #85).
#63), PR 4(
#62), PR 5 (#64 + #85), PR 6 (#13). They will be openedwith the
do-not-mergelabel and rebased ontodeveloponcethis PR merges.
UserStatusmapping: uses Doctrine's first-class enumsupport via
enumType:on the column attribute. No custom DBALtype. Reload round-trip is asserted in
UserRepositoryTest.