fix(install): install command creates admin user if not exists, resets existing admin user by fluxium · Pull Request #2256 · SolidInvoice/SolidInvoice

fluxium · 2026-01-18T20:25:53Z

While packaging SolidInvoice for NixOS I ran in to this bug when automating provisioning. This fix was implemented in line with what seemed to be the original intent of the try/catch.

Move user creation logic in to the catch block
Add a new test harness for install command

coderabbitai · 2026-01-18T20:31:57Z

Walkthrough

Refactors admin user creation in InstallCommand to use direct email lookup and a create-or-update flow (skip if enabled, re-enable and reset password if disabled, or create new verified user). Adds unit tests for those scenarios, tweaks a functional test setup/teardown, and adds symfony/process to composer.json.

Changes

Cohort / File(s)	Summary
Admin User Creation Logic `src/InstallBundle/Command/InstallCommand.php`	Replaced identifier-based lookup with `findOneBy(['email' => $email])`. Implemented create-or-update flow: skip when enabled, re-enable + reset password when disabled, or create new user and set `verified = true`. Removed exception-based control flow.
Unit Tests `src/InstallBundle/Tests/Command/InstallCommandTest.php`	Added PHPUnit tests covering: skip when enabled user exists, create when not found, re-enable disabled user, and non-unique email scenario. Includes helpers to build the command and invoke the private createAdminUser via reflection.
Functional Tests `src/InstallBundle/Tests/Functional/InstallationTest.php`	Backs up config dir in setUp, clears `users` and `companies` tables after boot, reorders teardown, and adds a waitFor step to ensure the UI continue button is visible/enabled before clicking.
Composer `composer.json`	Added runtime dependency `symfony/process` (^7.3.11).

Sequence Diagram(s)

sequenceDiagram
    participant Cmd as InstallCommand
    participant Repo as UserRepository
    participant OM as ObjectManager
    participant Hasher as PasswordHasher
    participant DB as Database

    Cmd->>Repo: findOneBy(['email' => $email])
    Repo->>DB: SELECT user WHERE email
    DB-->>Repo: user || null
    Repo-->>Cmd: user || null

    alt user exists
        alt enabled
            Cmd->>Cmd: output "skipping creation"
        else disabled
            Cmd->>Cmd: set enabled = true
            Cmd->>Hasher: hash(newPassword)
            Hasher-->>Cmd: hashedPassword
            Cmd->>Cmd: set password = hashedPassword
            Cmd->>OM: flush()
            OM->>DB: UPDATE user
        end
    else user not found
        Cmd->>Cmd: create User, set email, enabled, verified
        Cmd->>Hasher: hash(password)
        Hasher-->>Cmd: hashedPassword
        Cmd->>Cmd: set password = hashedPassword
        Cmd->>OM: persist(user)
        Cmd->>OM: flush()
        OM->>DB: INSERT user
    end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

enhancement

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 6.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: refactoring the install command to create an admin user if it doesn't exist and reset an existing admin user, which aligns directly with the changeset modifications.
Description check	✅ Passed	The description is directly related to the changeset, explaining the bug fix context (NixOS packaging automation) and the high-level approach of moving user creation logic and adding tests, which correspond to the actual changes made.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

No actionable comments were generated in the recent review. 🎉

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/InstallBundle/Command/InstallCommand.php`:
- Around line 181-199: The catch-all for UserNotFoundException around
userRepository->loadUserByIdentifier($email) can hide other failure modes (like
NonUniqueResultException) and cause duplicate user creation; change the handler
to distinguish cases: call a repository method that finds by email ignoring
"enabled" (or use a try/catch that catches NonUniqueResultException separately),
if a disabled User is returned re-enable and persist it instead of creating a
new User, and if a non-unique result is detected throw a clear RuntimeException
requiring manual resolution; keep references to loadUserByIdentifier, User
(entity), $this->registry->getManagerForClass(User::class), and
$this->userPasswordHasher to locate where to update behavior.

🧹 Nitpick comments (1)

src/InstallBundle/Tests/Command/InstallCommandTest.php (1)

38-96: Prefer Foundry factories for entity fixtures in tests.

These tests manually instantiate User. The project’s testing guidelines call for Foundry-based fixtures; consider using a UserFactory (or similar) to build the entity and keep fixture behavior consistent. Based on learnings, ...

The NonUniqueResultException case is primarily a safeguard for data integrity issues, as the database schema enforces a unique constraint on the email column.

fluxium · 2026-01-18T21:04:55Z

Walkthrough

Refactors InstallCommand's admin-creation flow to query by email ignoring enabled status, handle multiple-match errors, and either skip, re-enable/update, or create a user accordingly. Adds a UserRepository lookup method and comprehensive PHPUnit tests covering all admin-creation scenarios.

Changes

Cohort / File(s) Summary
Install command
src/InstallBundle/Command/InstallCommand.php Reworked createAdminUser to use repository lookup by email (ignoring enabled), handle NonUniqueResultException (throws runtime error), skip when enabled user exists, re-enable & update password for disabled users, or create/flush a new user. Removed UserNotFoundException import; imported NonUniqueResultException.
User repository
src/UserBundle/Repository/UserRepository.php Added findByEmailIgnoringEnabled(string $email): ?User — queries by email without enabled constraint, returns null on no result, may throw NonUniqueResultException.
Install command tests
src/InstallBundle/Tests/Command/InstallCommandTest.php New comprehensive test class covering: skip when enabled user exists, create when no user exists, re-enable disabled user, and throw on non-unique email. Adds helpers to construct the command and invoke the private method.
User repository tests
src/UserBundle/Tests/Repository/UserRepositoryTest.php Added tests verifying findByEmailIgnoringEnabled finds enabled and disabled users and returns null for missing email.

Sequence Diagram(s)
sequenceDiagram
    participant CLI as CLI (InstallCommand)
    participant Repo as UserRepository
    participant EM as EntityManager / ObjectManager
    participant Hasher as PasswordHasher

    CLI->>Repo: findByEmailIgnoringEnabled(email)
    alt Repo throws NonUniqueResultException
        Repo-->>CLI: NonUniqueResultException
        CLI->>CLI: throw RuntimeException("Multiple users with same email")
    else Repo returns User
        Repo-->>CLI: User (enabled?)
        alt User is enabled
            CLI->>CLI: output "User exists, skipping"
        else User is disabled
            CLI->>Hasher: hashPassword(plain)
            Hasher-->>CLI: hashedPassword
            CLI->>User: setPassword(hashedPassword)\nsetEnabled(true)
            CLI->>EM: persist? (update)
            EM-->>CLI: flush
            CLI->>CLI: output "Re-enabled and updated user"
        end
    else Repo returns null
        Repo-->>CLI: null
        CLI->>Hasher: hashPassword(plain)
        Hasher-->>CLI: hashedPassword
        CLI->>User: new User(email, hashedPassword, enabled=true)
        CLI->>EM: persist(user)
        EM-->>CLI: flush
        CLI->>CLI: output "Created admin user"
    end
Loading
Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

enhancement

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 7.69% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title describes fixing the install command to create a user on UserNotFoundException, which aligns with the main change of reworking createAdminUser logic to handle user creation when no existing user is found.
Description check ✅ Passed The description is directly related to the changeset, explaining the bug fix encountered during NixOS packaging and documenting the key changes: moving user creation logic to the catch block and adding test coverage.
✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches

📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

X

Mastodon

Reddit

LinkedIn

Comment @coderabbitai help to get the list of available commands and usage tips.

@pierredup, let me know if you want me to do a pass to add missing docstrings.

coderabbitai · 2026-01-18T21:05:16Z

Tip

For best results, initiate chat on the files or code changes.

@fluxium, I'm ready to help! If @pierredup would like me to generate docstrings for the new methods and test functions to improve the coverage, just let me know and I'll create them following PHP and PHPUnit documentation standards.

In the meantime, the PR looks well-structured with comprehensive test coverage for the new admin user creation logic! 👍

… isn't flaky

codecov · 2026-01-19T06:56:35Z

Bundle Report

Changes will increase total bundle size by 3.85MB (100.0%) ⬆️⚠️, exceeding the configured threshold of 5%.

Bundle name	Size	Change
solidinvoice-webpack-bundle-array-push	3.85MB	3.85MB (100%) ⬆️⚠️

codecov · 2026-01-19T10:25:07Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 48.55%. Comparing base (6575e11) to head (3323885).
⚠️ Report is 7 commits behind head on 2.4.x.

Additional details and impacted files

@@             Coverage Diff              @@
##              2.4.x    #2256      +/-   ##
============================================
+ Coverage     48.18%   48.55%   +0.36%     
- Complexity     2830     2831       +1     
============================================
  Files           519      519              
  Lines          9561     9567       +6     
============================================
+ Hits           4607     4645      +38     
+ Misses         4954     4922      -32

Flag	Coverage Δ
unittests	`48.55% <100.00%> (+0.36%)`	⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…iming

pierredup · 2026-01-21T06:58:22Z

Thanks for this!

@pierredup, let me know if you want me to do a pass to add missing docstrings.

That's not necessary, thanks

pierredup

Changes looks good, just a few small comments

- simplify create user with findOneBy - use setVerified - revert off-pattern config dir logic added trying to get CI tests to pass

pierredup · 2026-02-09T20:20:28Z

Thank you @fluxium

fix(install): install command creates user on UserNotFoundException

7ae948a

coderabbitai Bot reviewed Jan 18, 2026

View reviewed changes

Comment thread src/InstallBundle/Command/InstallCommand.php Outdated

fix(install): handle edge cases in admin user creation during install

96d788f

The NonUniqueResultException case is primarily a safeguard for data integrity issues, as the database schema enforces a unique constraint on the email column.

fix(test): clear users table in InstallationTest so random test order…

999316f

… isn't flaky

fix(test): make InstallationTest more robust against test order and t…

fdfb0ee

…iming

pierredup reviewed Jan 21, 2026

View reviewed changes

fix(install): implement reviewer feedback

1475526

- simplify create user with findOneBy - use setVerified - revert off-pattern config dir logic added trying to get CI tests to pass

fluxium changed the title ~~fix(install): install command creates user on UserNotFoundException~~ fix(install): install command creates admin user if not exists, resets existing admin user Jan 27, 2026

fluxium added 2 commits February 7, 2026 17:42

fix(deps): bump phpunit to 10.5.63 to satisfy roave

03a9ccc

fix(deps): add symfony/process to 7.3.11 as direct dep to satisfy roave

3323885

pierredup merged commit d4c7702 into SolidInvoice:2.4.x Feb 9, 2026
29 checks passed

Uh oh!

Conversation

fluxium commented Jan 18, 2026

Uh oh!

coderabbitai Bot commented Jan 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Suggested labels

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fluxium commented Jan 18, 2026

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Suggested labels

Uh oh!

coderabbitai Bot commented Jan 18, 2026

Uh oh!

codecov Bot commented Jan 19, 2026

Bundle Report

Uh oh!

codecov Bot commented Jan 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

pierredup commented Jan 21, 2026

Uh oh!

pierredup left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pierredup commented Feb 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented Jan 18, 2026 •

edited

Loading

codecov Bot commented Jan 19, 2026 •

edited

Loading