-
Notifications
You must be signed in to change notification settings - Fork 31
Insert initial admin command #368
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughAdds repository review configuration and documentation, introduces a Symfony console command to ensure a default admin user, and removes a prior database migration that seeded the initial admin. No runtime behavior beyond initialization mechanisms is changed. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Cmd as ImportDefaultsCommand
participant Repo as AdministratorRepository
participant Manager as AdministratorManager
participant EM as EntityManager
User->>Cmd: run phplist:defaults:import
Cmd->>Repo: findByLogin("admin")
alt admin exists
Cmd-->>User: "Default admin exists"
else admin missing
Cmd->>User: prompt hidden password
User-->>Cmd: provide password
Cmd->>Cmd: assemble privileges (PrivilegeFlag::cases)
Cmd->>Manager: createAdministrator(dto)
Manager->>EM: persist(entity)
Cmd->>EM: flush()
Cmd-->>User: "Default admin created"
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/AGENT.md (1)
10-15: Consider consistent list indentation.The static analysis tool notes that the nested list uses 4-space indentation instead of the expected 2 spaces. While this doesn't break functionality, consistent indentation helps with markdown readability and tooling compatibility.
If you'd like to align with markdown linting standards, adjust the indentation:
- **Core responsibilities:** - - Application bootstrapping and service configuration - - Database ORM (Doctrine) integration - - Command-line utilities (Symfony Console) - - Async email sending via Symfony Messenger - - Logging, configuration, routing, dependency injection - - Schema definition and updates + - Application bootstrapping and service configuration + - Database ORM (Doctrine) integration + - Command-line utilities (Symfony Console) + - Async email sending via Symfony Messenger + - Logging, configuration, routing, dependency injection + - Schema definition and updatessrc/Domain/Identity/Command/ImportDefaultsCommand.php (1)
36-70: Flush placement is correct, but consider adding transaction handling.The
flush()call at line 70 is properly placed in the application layer (Command), which is acceptable per the coding guidelines. The manager correctly delegates onlypersist()to the domain layer, leaving orchestration to this handler.However, the guidelines recommend: "Prefer $em->transactional(...) or explicit try/catch with rollback on failure."
Consider wrapping the admin creation in a transaction:
$existing = $this->administratorRepository->findOneBy(['loginName' => $login]); if ($existing === null) { - // If creating the default admin, require a password. Prefer env var, else prompt for input. - $password = $envPassword; - if ($password === null) { - /** @var QuestionHelper $helper */ - $helper = $this->getHelper('question'); - $question = new Question('Enter password for default admin (login "admin"): '); - $question->setHidden(true); - $question->setHiddenFallback(false); - $password = (string) $helper->ask($input, $output, $question); - if (trim($password) === '') { - $output->writeln('<error>Password must not be empty.</error>'); - return Command::FAILURE; - } - } - - $dto = new CreateAdministratorDto( - loginName: $login, - password: $password, - email: $email, - isSuperUser: true, - privileges: $allPrivileges, - ); - $admin = $this->administratorManager->createAdministrator($dto); - $this->entityManager->flush(); + try { + // If creating the default admin, require a password. Prefer env var, else prompt for input. + $password = $envPassword; + if ($password === null) { + /** @var QuestionHelper $helper */ + $helper = $this->getHelper('question'); + $question = new Question('Enter password for default admin (login "admin"): '); + $question->setHidden(true); + $question->setHiddenFallback(false); + $password = (string) $helper->ask($input, $output, $question); + if (trim($password) === '') { + $output->writeln('<error>Password must not be empty.</error>'); + return Command::FAILURE; + } + } + + $dto = new CreateAdministratorDto( + loginName: $login, + password: $password, + email: $email, + isSuperUser: true, + privileges: $allPrivileges, + ); + $admin = $this->administratorManager->createAdministrator($dto); + $this->entityManager->flush(); + } catch (\Exception $e) { + $output->writeln(sprintf('<error>Failed to create default admin: %s</error>', $e->getMessage())); + return Command::FAILURE; + }As per coding guidelines.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.coderabbit.yaml(1 hunks).github/AGENT.md(1 hunks).github/PULL_REQUEST_TEMPLATE.md(2 hunks)src/Domain/Identity/Command/ImportDefaultsCommand.php(1 hunks)src/Migrations/Version20251103SeedInitialAdmin.php(0 hunks)
💤 Files with no reviewable changes (1)
- src/Migrations/Version20251103SeedInitialAdmin.php
🧰 Additional context used
📓 Path-based instructions (2)
src/Domain/**
⚙️ CodeRabbit configuration file
src/Domain/**: You are reviewing PHP domain-layer code. Enforce strict domain purity:
- ❌ Do not allow infrastructure persistence side effects here.
- Flag ANY usage of Doctrine persistence APIs, especially:
- $entityManager->flush(...), $this->entityManager->flush(...)
- $em->persist(...), $em->remove(...)
- direct transaction control ($em->beginTransaction(), commit(), rollback())
- If found, request moving these calls to application-layer Command handlers or background Jobs.
- Also flag repositories in Domain that invoke flush/transactional logic; Domain repositories should be abstractions without side effects.
- Encourage domain events/outbox or return-values to signal write-intent, leaving orchestration to Commands/Jobs.
Files:
src/Domain/Identity/Command/ImportDefaultsCommand.php
src/**/Command/**
⚙️ CodeRabbit configuration file
src/**/Command/**: Application layer (Commands/Handlers) is the right place to coordinate persistence.
- ✅ It is acceptable to call $entityManager->flush() here.
- Check that flush is used atomically (once per unit of work) after all domain operations.
- Ensure no domain entity or domain service is calling flush; only the handler orchestrates it.
- Prefer $em->transactional(...) or explicit try/catch with rollback on failure.
Files:
src/Domain/Identity/Command/ImportDefaultsCommand.php
🧬 Code graph analysis (1)
src/Domain/Identity/Command/ImportDefaultsCommand.php (3)
src/Domain/Identity/Model/Dto/CreateAdministratorDto.php (1)
CreateAdministratorDto(7-20)src/Domain/Identity/Service/AdministratorManager.php (2)
AdministratorManager(13-61)createAdministrator(24-37)src/Domain/Identity/Model/Administrator.php (2)
isSuperUser(151-154)getLoginName(90-93)
🪛 LanguageTool
.github/AGENT.md
[uncategorized] ~37-~37: The official name of this software platform is spelled with a capital “H”.
Context: ...-3.0 | --- ## 📁 Project Structure - .github/ CI/CD and PR templates - bin/ Symfony ...
(GITHUB)
🪛 markdownlint-cli2 (0.18.1)
.github/AGENT.md
10-10: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
11-11: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
12-12: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
13-13: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
14-14: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
15-15: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: phpList on PHP 8.1 [Build, Test]
🔇 Additional comments (7)
.github/PULL_REQUEST_TEMPLATE.md (2)
1-1: LGTM - Clean template simplification.Replacing the verbose summary block with the
@coderabbitai summaryplaceholder streamlines the template.
14-14: Good catch on the grammar fix."your code" reads much better than "you code".
.github/AGENT.md (1)
37-37: Minor capitalization: "GitHub" vs ".github/"The static analysis tool flagged line 37 where it says ".github/" - it's suggesting "GitHub" with a capital H. However, in this context you're referring to the
.github/directory, not the GitHub platform, so the lowercase is actually correct for the directory path..coderabbit.yaml (2)
1-10: Clean configuration setup.The schema reference, tone setting, and review options are properly configured. Disabling poems and collapsing walkthroughs will keep reviews focused.
11-38: Solid architectural guidance via path_instructions.The three path-specific instruction blocks clearly define boundaries:
- Domain layer: strict purity, no flush/persist
- Command handlers: acceptable flush location with atomic usage
- Message handlers: persistence with idempotency checks
This aligns perfectly with DDD/hexagonal architecture principles and matches the coding guidelines.
src/Domain/Identity/Command/ImportDefaultsCommand.php (2)
28-34: Constructor DI looks good.Clean dependency injection following Symfony best practices.
88-99: Helper method is clean.The privilege enumeration logic is straightforward and well-documented with the
@returnannotation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Domain/Identity/Command/ImportDefaultsCommand.php (1)
69-70: Consider wrapping flush in transactional block for resilience.The flush() call here is fine per coding guidelines for application-layer Commands. However, wrapping it in a transactional block would improve error handling and ensure proper rollback on failure.
Apply this pattern for better resilience:
- $admin = $this->administratorManager->createAdministrator($dto); - $this->entityManager->flush(); + $admin = $this->entityManager->transactional(function () use ($dto) { + return $this->administratorManager->createAdministrator($dto); + });As per coding guidelines.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Domain/Identity/Command/ImportDefaultsCommand.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/Domain/**
⚙️ CodeRabbit configuration file
src/Domain/**: You are reviewing PHP domain-layer code. Enforce strict domain purity:
- ❌ Do not allow infrastructure persistence side effects here.
- Flag ANY usage of Doctrine persistence APIs, especially:
- $entityManager->flush(...), $this->entityManager->flush(...)
- $em->persist(...), $em->remove(...)
- direct transaction control ($em->beginTransaction(), commit(), rollback())
- If found, request moving these calls to application-layer Command handlers or background Jobs.
- Also flag repositories in Domain that invoke flush/transactional logic; Domain repositories should be abstractions without side effects.
- Encourage domain events/outbox or return-values to signal write-intent, leaving orchestration to Commands/Jobs.
Files:
src/Domain/Identity/Command/ImportDefaultsCommand.php
src/**/Command/**
⚙️ CodeRabbit configuration file
src/**/Command/**: Application layer (Commands/Handlers) is the right place to coordinate persistence.
- ✅ It is acceptable to call $entityManager->flush() here.
- Check that flush is used atomically (once per unit of work) after all domain operations.
- Ensure no domain entity or domain service is calling flush; only the handler orchestrates it.
- Prefer $em->transactional(...) or explicit try/catch with rollback on failure.
Files:
src/Domain/Identity/Command/ImportDefaultsCommand.php
🧬 Code graph analysis (1)
src/Domain/Identity/Command/ImportDefaultsCommand.php (3)
src/Domain/Identity/Model/Dto/CreateAdministratorDto.php (1)
CreateAdministratorDto(7-20)src/Domain/Identity/Service/AdministratorManager.php (2)
AdministratorManager(13-61)createAdministrator(24-37)src/Domain/Identity/Model/Administrator.php (2)
isSuperUser(151-154)getLoginName(90-93)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: phpList on PHP 8.1 [Build, Test]
- GitHub Check: phpList on PHP 8.1 [Build, Test]
🔇 Additional comments (4)
src/Domain/Identity/Command/ImportDefaultsCommand.php (4)
1-27: LGTM! Clean setup and imports.The namespace, imports, and command metadata are well-structured. The constants provide clear defaults for the admin user.
28-34: LGTM! Proper dependency injection.Constructor cleanly injects the necessary services. Using readonly properties is a nice touch.
77-83: Nice! Past issue resolved.The previous review flagged line 81 for using the local
$existing->getEmail(). This has been corrected - the code now properly displays the existing admin's email.
88-99: LGTM! Clean privilege enumeration.The helper correctly iterates over all privilege flags and grants them. The implementation is clear and the PHPDoc is accurate.
…essor (#357) * Blacklist (#352) * Blacklisted * user repository methods * fix configs * add test * fix: phpmd * fix: repo configs * return a created resource --------- Co-authored-by: Tatevik <tatevikg1@gmail.com> * Subscribepage (#353) * subscriber page manager * owner entity * test * ci fix * getByPage data --------- Co-authored-by: Tatevik <tatevikg1@gmail.com> * Bounceregex manager (#354) * BounceRegexManager * Fix manager directory * Prop name update admin -> adminId --------- Co-authored-by: Tatevik <tatevikg1@gmail.com> * Bounce processing command (#355) * BounceManager * Add bounce email * Move to the processor dir * SendProcess lock service * ClientIp + SystemInfo * ProcessBouncesCommand * ProcessBouncesCommand all methods * BounceProcessingService * AdvancedBounceRulesProcessor * UnidentifiedBounceReprocessor * ConsecutiveBounceHandler * Refactor * BounceDataProcessor * ClientFactory + refactor * BounceProcessorPass * Register services + phpstan fix * PhpMd * PhpMd CyclomaticComplexity * PhpCodeSniffer * Tests * Refactor * Add tests * More tests * Fix tests --------- Co-authored-by: Tatevik <tatevikg1@gmail.com> * EventLog + translator (#356) * EventLogManager * Log failed logins + translate messages * weblate * test fix * Use translations * Fix pipeline * Weblate * Deprecate DB translation table --------- Co-authored-by: Tatevik <tatevikg1@gmail.com> * Access level check (#358) * OwnableInterface * PermissionChecker * Check related * Register service + test * Style fix --------- Co-authored-by: Tatevik <tatevikg1@gmail.com> * Message processor (#359) * MessageStatusEnum * Status validate * Embargo check * IspRestrictions * IspRestrictions * SendRateLimiter * UserMessageStatus * Refactor * RateLimitedCampaignMailer * RateLimitedCampaignMailerTest * Rate limit initialized from history * Check maintenance mode * Max processing time limiter --------- Co-authored-by: Tatevik <tatevikg1@gmail.com> * Exceptions + translations (#361) * MissingMessageIdException * MailboxConnectionException * BadMethodCallException (style fix) * Translate user facing messages * Translate * Translate PasswordResetMessageHandler texts * Translate SubscriberConfirmationMessageHandler texts * MessageBuilder exceptions * BlacklistEmailAndDeleteBounceHandler * BlacklistEmailHandler - AdvancedBounceRulesProcessor * AdvancedBounceRulesProcessor * BounceStatus * CampaignProcessor * UnidentifiedBounceReprocessor * Style fix * Test fix * ConsecutiveBounceHandler --------- Co-authored-by: Tatevik <tatevikg1@gmail.com> * Fix autowiring * Reset subscriber bounce count * Install RssFeedBundle * Update import logic, add dynamic attribute repository (#362) * Skip password and modified fields while import, do not subscribe blacklisted users * DefaultConfigProvider * Use ConfigProvider in ProcessQueueCommand * Use ConfigProvider in SubscriberCsvImporter + send email * Rename paramProvider * Test fix * AttributeValueProvider for dynamic tables * Update: config provider * Translations in default configs * Email with messageHandler * Style fix * PhpCs fix * Fix configs * replace list names * Add tests + fix * Add more tests + fix handler --------- Co-authored-by: Tatevik <tatevikg1@gmail.com> * Refactor import add subscriber history (#363) * Add blacklisted stat to import result * Add history * Add translations * addHistory for unconfirmed * Refactor * Add changeSetDto * Do not send email on creating without any list * Flush in controller * Add test --------- Co-authored-by: Tatevik <tatevikg1@gmail.com> * Em flush (#364) * createSubscriberList * ProcessQueueCommand * Dispatch CampaignProcessorMessage for processing * Fix tests * Fix style * CleanUpOldSessionTokens * Move bounce processing into the Bounce folder * delete/remove * Remove hardcoded TatevikGrRssBundle mapping * Fix configs * Add sync * Fix: DQL in MessageRepository.php * PhpMD * SubscriberBlacklistService, SubscriberBlacklistManager * AdministratorManager * BounceManager * SendProcessManager * TemplateManager * SubscribePageManager * Fix: tests * BounceManager * rest of flushes * save * fix test * CouplingBetweenObjects 15 --------- Co-authored-by: Tatevik <tatevikg1@gmail.com> * Add test * Update: docs * Migrations (mysql psql) (#366) * OnlyOrmTablesFilter * Current * Admin * Init migration * In progress * Fix mapping * Fix tests * Migrate * Separate MySql * Use psql * Rename indexes * PostgreSqlPlatform * MySqlSqlPlatform rename indexes * Fix: cs * Fix: test configs * Add migration template * PR agent (#365) * .coderabbit.yaml This reverts commit 2246e49. --------- Co-authored-by: Tatevik <tatevikg1@gmail.com> * rename template * After review 0 * After review 1 * Fix MySql migrations * After review 2 --------- Co-authored-by: Tatevik <tatevikg1@gmail.com> * Import by foreign key (#367) * Import with a foreign key --------- Co-authored-by: Tatevik <tatevikg1@gmail.com> * Insert initial admin command (#368) * Update: codeRabbit configs * Update: Use command for initial admin insert --------- Co-authored-by: Tatevik <tatevikg1@gmail.com> * Update pull request template --------- Co-authored-by: Tatevik <tatevikg1@gmail.com>
Summary
Unit test
Are your changes covered with unit tests, and do they not break anything?
You can run the existing unit tests using this command:
Code style
Have you checked that you code is well-documented and follows the PSR-2 coding
style?
You can check for this using this command:
Other Information
If there's anything else that's important and relevant to your pull
request, mention that information here. This could include benchmarks,
or other information.
If you are updating any of the CHANGELOG files or are asked to update the
CHANGELOG files by reviewers, please add the CHANGELOG entry at the top of the file.
Thanks for contributing to phpList!
Summary by CodeRabbit
New Features
Documentation
Chores