-
-
Notifications
You must be signed in to change notification settings - Fork 213
Fix auto increment invoice id with prefix #2236
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
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 2.3.x #2236 +/- ##
============================================
+ Coverage 47.74% 47.83% +0.08%
- Complexity 2755 2757 +2
============================================
Files 502 502
Lines 9326 9342 +16
============================================
+ Hits 4453 4469 +16
Misses 4873 4873
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
WalkthroughThe PR modifies the billing ID generation system to properly support prefixes and suffixes. It retrieves configured prefix and suffix values from entity-specific settings and injects them into ID generation options. Additionally, auto-increment logic now trims configured prefix and suffix from existing IDs when calculating the next sequential number. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. 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 (1)
src/CoreBundle/Tests/Generator/BillingIdGenerator/AutoIncrementIdGeneratorTest.php (1)
62-117: Suggest additional test coverage for edge cases.The new tests comprehensively cover the happy path for prefix/suffix scenarios. Consider adding tests for:
First ID generation with prefix/suffix (empty database):
public function testItGeneratesFirstIdWithPrefix(): void { $generator = new AutoIncrementIdGenerator(self::getContainer()->get('doctrine')); self::assertSame('1', $generator->generate(new Invoice(), ['field' => 'invoiceId', 'prefix' => 'INV-', 'suffix' => ''])); }Migration scenario (mixed IDs - some with prefix, some without):
public function testItHandlesMixedIdsAfterPrefixEnabled(): void { $client = ClientFactory::new([]); InvoiceFactory::createOne(['client' => $client, 'invoiceId' => '5']); // Old format InvoiceFactory::createOne(['client' => $client, 'invoiceId' => 'INV-10']); // New format $generator = new AutoIncrementIdGenerator(self::getContainer()->get('doctrine')); $result = $generator->generate(new Invoice(), ['field' => 'invoiceId', 'prefix' => 'INV-', 'suffix' => '']); // Should be '11', not '6' self::assertSame('11', $result); }The migration scenario is particularly important given the SUBSTRING logic could fail when existing IDs lack the expected prefix/suffix.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
src/CoreBundle/Generator/BillingIdGenerator.phpsrc/CoreBundle/Generator/BillingIdGenerator/AutoIncrementIdGenerator.phpsrc/CoreBundle/Tests/Generator/BillingIdGenerator/AutoIncrementIdGeneratorTest.php
🧰 Additional context used
📓 Path-based instructions (5)
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Include file header comment with license and copyright information in all PHP files
Always declare strict types withdeclare(strict_types=1);at the top of PHP files
Always specify parameter and return type hints in PHP
Follow PSR-12 and Symfony coding standards checked by EasyCodingStandard (ECS)
**/*.php: Include required file header comment with MIT license attribution
Always declare strict types at the top of PHP files usingdeclare(strict_types=1);
Use type hints for all function parameters and return types in PHP
Follow PSR-12 and Symfony coding standards enforced by EasyCodingStandard
Files:
src/CoreBundle/Generator/BillingIdGenerator.phpsrc/CoreBundle/Generator/BillingIdGenerator/AutoIncrementIdGenerator.phpsrc/CoreBundle/Tests/Generator/BillingIdGenerator/AutoIncrementIdGeneratorTest.php
src/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.php: Use constructor dependency injection for all service dependencies
Preferfinalclasses unless inheritance is specifically needed
Always specify visibility modifiers (public, private, protected) on properties and methods
Never use hardcoded values; use environment variables or configuration for all configurable settings
Use Money objects from moneyphp/money library instead of raw floats for monetary values
Always use Doctrine ORM repositories instead of raw SQL queries
Run static analysis with PHPStan at Level 6 before committing code
Use Rector for automated code refactoring to modernize PHP code and apply best practices
Use Symfony Security component and voter pattern for implementing role-based access control
Use Symfony Mailer for all email sending operations
Use PDF generation manager service for all PDF output instead of direct mPDF calls
Each PHP file must contain exactly one class and the filename must match the class name
Use namespace structure that reflects directory structure with proper PSR-4 autoloading
Orderusestatements in the following order: constants, classes, functions
Integrate Symfony Workflow for state machine transitions on invoices and complex domain objects
src/**/*.php: Always use constructor injection for dependencies in classes
Always use Money objects from moneyphp/money library, never raw floats for monetary values
Validate all user input using Symfony Validator and Form validation
Preferfinalclasses unless inheritance is explicitly needed
Always specify visibility modifiers (public, private, protected) for properties and methods
Use class constants instead of global constants for configuration values
Implement one class per file with filename matching the class name
Use statement ordering: const, class, function in use declarations
Use PHPStan static analysis at Level 6 to ensure type safety and code quality
Use Rector for automated code refactoring and modernization of PHP code
Implement proper error handl...
Files:
src/CoreBundle/Generator/BillingIdGenerator.phpsrc/CoreBundle/Generator/BillingIdGenerator/AutoIncrementIdGenerator.phpsrc/CoreBundle/Tests/Generator/BillingIdGenerator/AutoIncrementIdGeneratorTest.php
src/CoreBundle/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
Use PDF manager service for generating PDFs, with proper HTML templates
Use mPDF library for all PDF generation via the PDF manager service
Files:
src/CoreBundle/Generator/BillingIdGenerator.phpsrc/CoreBundle/Generator/BillingIdGenerator/AutoIncrementIdGenerator.phpsrc/CoreBundle/Tests/Generator/BillingIdGenerator/AutoIncrementIdGeneratorTest.php
src/**/Tests/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/Tests/**/*.php: Follow strict testing requirements: use PHPUnit 10.4+, with test execution in random order and strict mode
Write unit tests for all new classes and business logic
Use Mockery for mocking dependencies in unit tests
Use Foundry for fixture management in tests instead of manual fixtures
src/**/Tests/**/*.php: Add proper PHPUnit file header with declare(strict_types=1) to all test files
Add unit tests for all business logic classes
Run tests in random order via PHPUnit executionOrder setting
Use Foundry Factory classes for generating test fixtures
Create test cases that extend appropriate base classes (TestCase, ApiTestCase, FunctionalTestCase)
Separate unit tests for individual classes from functional tests for workflows
Use strict assertions in tests (assertEquals not ==, etc.)
Test error cases and edge cases, not just happy path
src/**/Tests/**/*.php: Write unit tests for all new business logic in Tests/ directory of appropriate bundle
Use Foundry for fixture generation in tests
Use Mockery for mocking dependencies in unit tests
Use Symfony Panther for end-to-end browser testing
Files:
src/CoreBundle/Tests/Generator/BillingIdGenerator/AutoIncrementIdGeneratorTest.php
src/*/Tests/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
src/*/Tests/**/*.php: Write unit tests for individual classes in isolation using Mockery for dependencies
Use PHPUnit with random test execution order and strict mode enabled
Files:
src/CoreBundle/Tests/Generator/BillingIdGenerator/AutoIncrementIdGeneratorTest.php
🧠 Learnings (4)
📚 Learning: 2025-12-27T08:20:09.499Z
Learnt from: CR
Repo: SolidInvoice/SolidInvoice PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-27T08:20:09.499Z
Learning: Applies to src/*/Entity/*.php : Use UUID (ULID) as primary key type for all entities with `UlidType` and `UlidGenerator`
Applied to files:
src/CoreBundle/Generator/BillingIdGenerator.php
📚 Learning: 2025-12-27T08:22:06.774Z
Learnt from: CR
Repo: SolidInvoice/SolidInvoice PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T08:22:06.774Z
Learning: Applies to src/**/Tests/**/*.php : Write unit tests for all new business logic in Tests/ directory of appropriate bundle
Applied to files:
src/CoreBundle/Tests/Generator/BillingIdGenerator/AutoIncrementIdGeneratorTest.php
📚 Learning: 2025-12-27T08:21:22.816Z
Learnt from: CR
Repo: SolidInvoice/SolidInvoice PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-12-27T08:21:22.816Z
Learning: Applies to src/**/Tests/Functional/**/*.php : Write integration tests for complex workflows spanning multiple bundles
Applied to files:
src/CoreBundle/Tests/Generator/BillingIdGenerator/AutoIncrementIdGeneratorTest.php
📚 Learning: 2025-12-27T08:21:22.816Z
Learnt from: CR
Repo: SolidInvoice/SolidInvoice PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-12-27T08:21:22.816Z
Learning: Applies to src/**/Tests/**/*.php : Add unit tests for all business logic classes
Applied to files:
src/CoreBundle/Tests/Generator/BillingIdGenerator/AutoIncrementIdGeneratorTest.php
🧬 Code graph analysis (2)
src/CoreBundle/Generator/BillingIdGenerator.php (3)
src/SettingsBundle/SystemConfig.php (1)
get(40-47)src/CoreBundle/Generator/BillingIdGenerator/AutoIncrementIdGenerator.php (1)
generate(43-83)src/CoreBundle/Generator/BillingIdGenerator/IdGeneratorInterface.php (1)
generate(30-30)
src/CoreBundle/Tests/Generator/BillingIdGenerator/AutoIncrementIdGeneratorTest.php (3)
src/CoreBundle/Generator/BillingIdGenerator/AutoIncrementIdGenerator.php (2)
AutoIncrementIdGenerator(26-84)generate(43-83)src/CoreBundle/Generator/BillingIdGenerator.php (1)
generate(40-65)src/CoreBundle/Generator/BillingIdGenerator/IdGeneratorInterface.php (1)
generate(30-30)
⏰ 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). (19)
- GitHub Check: DB (MySQL pdo_mysql 8.4)
- GitHub Check: DB (MariaDB pdo_mysql 10.4)
- GitHub Check: DB (MySQL pdo_mysql 8.0)
- GitHub Check: DB (PostgreSQL pdo_pgsql 16)
- GitHub Check: DB (PostgreSQL pdo_pgsql 17)
- GitHub Check: DB (MariaDB pdo_mysql 11.4)
- GitHub Check: DB (MySQL pdo_mysql 8.3)
- GitHub Check: DB (MySQL pdo_mysql 5.7)
- GitHub Check: DB (MariaDB pdo_mysql 11)
- GitHub Check: DB (MariaDB pdo_mysql 10.6)
- GitHub Check: DB (MariaDB pdo_mysql 10.11)
- GitHub Check: DB (MariaDB pdo_mysql 11.0)
- GitHub Check: DB (MariaDB pdo_mysql 10.5)
- GitHub Check: DB (MySQL pdo_mysql 9)
- GitHub Check: PHPStan
- GitHub Check: Lint
- GitHub Check: Unit ( PHP 8.3 )
- GitHub Check: Unit ( PHP 8.3 )
- GitHub Check: Unit ( PHP 8.4 )
🔇 Additional comments (5)
src/CoreBundle/Generator/BillingIdGenerator.php (3)
42-51: Flag: PR title doesn't match the actual changes.The PR title mentions "Fix performance issues when loading api tokens," but the code changes are actually fixing autoincrement behavior for invoice/quote ID generation with prefixes/suffixes. The implementation correctly retrieves entity-specific prefix and suffix configuration.
53-55: LGTM!Correctly passes prefix and suffix to the underlying generator, enabling it to strip these values when calculating the next sequential ID from existing records.
59-64: LGTM!The final ID composition correctly concatenates the configured prefix, generated numeric ID, and configured suffix.
src/CoreBundle/Tests/Generator/BillingIdGenerator/AutoIncrementIdGeneratorTest.php (1)
25-30: LGTM!The test class properly uses Foundry for fixture generation and follows PHPUnit best practices with proper isolation and clear assertions.
src/CoreBundle/Generator/BillingIdGenerator/AutoIncrementIdGenerator.php (1)
73-73: TO_NUMBER is already properly registered and handles database compatibility.The custom Doctrine DQL function
ToNumberFunctionis registered inconfig/packages/doctrine.phpand implements database-aware SQL generation:
- PostgreSQL: Uses native
TO_NUMBER()function- MySQL/SQLite: Falls back to field dispatch (no conversion)
No further action needed.
src/CoreBundle/Generator/BillingIdGenerator/AutoIncrementIdGenerator.php
Show resolved
Hide resolved
Bundle ReportChanges will decrease total bundle size by 408.77kB (-5.12%) ⬇️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: solidinvoice-webpack-bundle-array-pushAssets Changed:
|
Fixes #2234