diff --git a/.coderabbit.yaml b/.coderabbit.yaml index 3cc5119b..e6be6bb2 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -1,9 +1,50 @@ +# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json language: "en-US" +tone_instructions: "chill" reviews: profile: "chill" high_level_summary: true + collapse_walkthrough: true + suggested_labels: false + high_level_summary_in_walkthrough: false + poem: false + path_instructions: + - path: "src/Domain/**" + instructions: | + 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. + + - path: "src/**/Command/**" + instructions: | + 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. + + - path: "src/**/MessageHandler/**" + instructions: | + Background jobs/workers may perform persistence. + - ✅ Allow $entityManager->flush() here when the job is the orchestration boundary. + - Verify idempotency and that flush frequency is appropriate (batching where practical). + - Ensure no domain-layer code invoked by the job performs flush/transaction control. + auto_review: enabled: true base_branches: - ".*" drafts: false +# ignore_title_keywords: +# - '' + +#knowledge_base: +# code_guidelines: +# filePatterns: +# - ".github/AGENT.md" diff --git a/.github/AGENT.md b/.github/AGENT.md new file mode 100644 index 00000000..6fab2120 --- /dev/null +++ b/.github/AGENT.md @@ -0,0 +1,103 @@ +# AGENT.md — Code Review Knowledge Base for phpList/core + +## 🧭 Repository Overview + +This repository is the **core package** of **phpList 4**, a modular and extensible email campaign management system. + +- **Purpose:** Provides the reusable foundation and framework for phpList applications and modules. +- **Consumers:** `phpList/web-frontend`, `phpList/rest-api`, and `phpList/base-distribution`. +- **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 + +> **Note:** This repository does *not* contain UI or REST endpoints. Those are part of other phpList packages. + +--- + +## ⚙️ Tech Stack + +| Category | Technology | +|-----------|-------------| +| Language | PHP ≥ 8.1 | +| Framework | Symfony 6.x components | +| ORM | Doctrine ORM 3.x | +| Async / Queue | Symfony Messenger | +| Tests | PHPUnit | +| Static analysis | PHPStan | +| Docs | PHPDocumentor | +| License | AGPL-3.0 | + +--- + +## 📁 Project Structure +- .github/ CI/CD and PR templates +- bin/ Symfony console entrypoints +- config/ Application & environment configs +- docs/ Developer documentation and generated API docs +- public/ Public entrypoint for local web server +- resources/Database/ Canonical SQL schema +- src/ Core PHP source code +- tests/ PHPUnit test suites +- composer.json Package metadata and dependencies +- phpunit.xml.dist Test configuration +- phpstan.neon Static analysis configuration + +--- + +## 💡 Code Design Principles + +1. **Modularity:** + The core remains framework-like — decoupled from frontend or API layers. + +2. **Dependency Injection:** + Use Symfony’s service container; avoid static/global dependencies. + +3. **Strict Typing:** + Always use `declare(strict_types=1);` and explicit type declarations. + +4. **Doctrine Entities:** + - Keep entities simple (no business logic). + - Mirror schema changes in `resources/Database/Schema.sql`. + - Maintain backward compatibility with phpList 3. + +5. **Symfony Best Practices:** + Follow Symfony structure and naming conventions. Use annotations or attributes for routing. + +6. **Error Handling & Logging:** + - Prefer structured logging via Graylog. + - Catch and handle exceptions at service or command boundaries. + +7. **Async Email:** + - Uses Symfony Messenger. + - Handlers must be idempotent and retry-safe. + - Avoid blocking or synchronous email sending. + +--- + +## 🧪 Testing Guidelines + +- **Framework:** PHPUnit +- **Database:** SQLite or mocks for unit tests; MySQL for integration tests. +- **Coverage target:** ≥85% for core logic. +- **Naming:** Mirror source structure (e.g., `Mailer.php` → `MailerTest.php`). + + +## 🧱 Code Style + +- Follow PSR-12 and Symfony coding conventions. +- Match the current codebase’s formatting and spacing. +- Use meaningful, consistent naming. +- Apply a single responsibility per class. + + +## 🔄 Pull Request Review Guidelines +### 🔐 Security Review Notes + +- Do not log sensitive data (passwords, tokens, SMTP credentials). +- Sanitize all user and external inputs. +- Always use parameterized Doctrine queries. +- Async jobs must be retry-safe and idempotent. diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index b4c9b3f7..a04f6ffa 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,10 +1,4 @@ -### Summary - -Provide a general description of the code changes in your pull request … -were there any bugs you had fixed? If so, mention them. If these bugs have open -GitHub issues, be sure to tag them here as well, to keep the conversation -linked together. - +"@coderabbitai summary" ### Unit test @@ -17,7 +11,7 @@ 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 +Have you checked that your code is well-documented and follows the PSR-2 coding style? You can check for this using this command: diff --git a/src/Domain/Identity/Command/ImportDefaultsCommand.php b/src/Domain/Identity/Command/ImportDefaultsCommand.php new file mode 100644 index 00000000..a1ff627e --- /dev/null +++ b/src/Domain/Identity/Command/ImportDefaultsCommand.php @@ -0,0 +1,100 @@ +allPrivilegesGranted(); + + $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('Password must not be empty.'); + return Command::FAILURE; + } + } + + $dto = new CreateAdministratorDto( + loginName: $login, + password: $password, + email: $email, + isSuperUser: true, + privileges: $allPrivileges, + ); + $admin = $this->administratorManager->createAdministrator($dto); + $this->entityManager->flush(); + + $output->writeln(sprintf( + 'Default admin created: login="%s", email="%s", superuser=yes, privileges=all', + $admin->getLoginName(), + $admin->getEmail() + )); + } else { + $output->writeln(sprintf( + 'Default admin already exists: login="%s", email="%s"', + $existing->getLoginName(), + $existing->getEmail(), + )); + } + + return Command::SUCCESS; + } + + /** + * @return array + * @SuppressWarnings(PHPMD.StaticAccess) + */ + private function allPrivilegesGranted(): array + { + $all = []; + foreach (PrivilegeFlag::cases() as $flag) { + $all[$flag->value] = true; + } + return $all; + } +} diff --git a/src/Migrations/Version20251103SeedInitialAdmin.php b/src/Migrations/Version20251103SeedInitialAdmin.php deleted file mode 100644 index c1d95889..00000000 --- a/src/Migrations/Version20251103SeedInitialAdmin.php +++ /dev/null @@ -1,49 +0,0 @@ -connection->getDatabasePlatform(); - $this->skipIf(!$platform instanceof PostgreSQLPlatform, sprintf( - 'Unsupported platform for this migration: %s', - get_class($platform) - )); - - $this->addSql(<<<'SQL' - INSERT INTO phplist_admin (id, created, modified, loginname, namelc, email, password, passwordchanged, disabled, superuser, privileges) - VALUES (1, NOW(), NOW(), 'admin', 'admin', 'admin@example.com', :hash, CURRENT_DATE, FALSE, TRUE, :privileges) - ON CONFLICT (id) DO UPDATE - SET - modified = EXCLUDED.modified, - privileges = EXCLUDED.privileges - SQL, [ - 'hash' => hash('sha256', 'password'), - 'privileges' => 'a:4:{s:11:"subscribers";b:1;s:9:"campaigns";b:1;s:10:"statistics";b:1;s:8:"settings";b:1;}', - ]); - } - - public function down(Schema $schema): void - { - $platform = $this->connection->getDatabasePlatform(); - $this->skipIf(!$platform instanceof PostgreSQLPlatform, sprintf( - 'Unsupported platform for this migration: %s', - get_class($platform) - )); - - $this->addSql('DELETE FROM phplist_admin WHERE id = 1'); - } -}