Skip to content

Conversation

@pierredup
Copy link
Member

Fixes #2234

@pierredup pierredup added this to the 2.3.13 milestone Jan 9, 2026
@pierredup pierredup self-assigned this Jan 9, 2026
@pierredup pierredup added the bug label Jan 9, 2026
@codecov
Copy link

codecov bot commented Jan 9, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 47.83%. Comparing base (9c98384) to head (33989a7).
⚠️ Report is 1 commits behind head on 2.3.x.

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              
Flag Coverage Δ
unittests 47.83% <100.00%> (+0.08%) ⬆️

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.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 9, 2026

Walkthrough

The 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

Cohort / File(s) Summary
ID Generation Configuration
src/CoreBundle/Generator/BillingIdGenerator.php
Retrieves id_generation/id_prefix and id_generation/id_suffix from entity config and passes them to the selected IdGenerator via options array.
Auto-Increment Logic
src/CoreBundle/Generator/BillingIdGenerator/AutoIncrementIdGenerator.php
Implements dynamic SUBSTRING trimming to extract numeric portion of IDs by conditionally removing configured prefix and suffix before computing MAX value for incrementing.
Test Coverage
src/CoreBundle/Tests/Generator/BillingIdGenerator/AutoIncrementIdGeneratorTest.php
Adds three test methods (testItIncrementsTheIdWithPrefix, testItIncrementsTheIdWithSuffix, testItIncrementsTheIdWithPrefixAndSuffix) to verify ID incrementation with prefix/suffix combinations.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • Merge 2.3.x into 2.4.x #2202: Handles renamed/explicit id_prefix/id_suffix configuration and related trimming logic in ID generation and increment calculations.
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed The description simply states 'Fixes #2234' which is related to the autoincrement bug fix that the code changes address.
Linked Issues check ✅ Passed The code changes directly address issue #2234 by implementing prefix/suffix handling in BillingIdGenerator and fixing numeric extraction in AutoIncrementIdGenerator with comprehensive test coverage.
Out of Scope Changes check ✅ Passed All changes (BillingIdGenerator, AutoIncrementIdGenerator, and test coverage) are directly scoped to fixing the autoincrement ID generation with prefix/suffix as required by issue #2234.
Title check ✅ Passed The title 'Fix auto increment invoice id with prefix' directly addresses the main issue being resolved: enabling auto-increment functionality for invoice IDs when a prefix is configured.

✏️ 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

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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:

  1. 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' => '']));
    }
  2. 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

📥 Commits

Reviewing files that changed from the base of the PR and between 9c98384 and 33989a7.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (3)
  • src/CoreBundle/Generator/BillingIdGenerator.php
  • src/CoreBundle/Generator/BillingIdGenerator/AutoIncrementIdGenerator.php
  • src/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 with declare(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 using declare(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.php
  • src/CoreBundle/Generator/BillingIdGenerator/AutoIncrementIdGenerator.php
  • src/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
Prefer final classes 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
Order use statements 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
Prefer final classes 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.php
  • src/CoreBundle/Generator/BillingIdGenerator/AutoIncrementIdGenerator.php
  • src/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.php
  • src/CoreBundle/Generator/BillingIdGenerator/AutoIncrementIdGenerator.php
  • src/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 ToNumberFunction is registered in config/packages/doctrine.php and implements database-aware SQL generation:

  • PostgreSQL: Uses native TO_NUMBER() function
  • MySQL/SQLite: Falls back to field dispatch (no conversion)

No further action needed.

@pierredup pierredup changed the title Fix performance issues when loading api tokens Fix auto increment invoice id with prefix Jan 9, 2026
@pierredup pierredup merged commit 5c7c02f into 2.3.x Jan 9, 2026
27 of 28 checks passed
@pierredup pierredup deleted the fix-invoice-increment branch January 9, 2026 18:41
@codecov
Copy link

codecov bot commented Jan 9, 2026

Bundle Report

Changes will decrease total bundle size by 408.77kB (-5.12%) ⬇️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
solidinvoice-webpack-bundle-array-push 7.57MB -408.77kB (-5.12%) ⬇️

Affected Assets, Files, and Routes:

view changes for bundle: solidinvoice-webpack-bundle-array-push

Assets Changed:

Asset Name Size Change Total Size Change (%)
app.*.css -555 bytes 1.36MB -0.04%
email.*.css -152.22kB 504.16kB -23.19%
pdf.*.css -152.22kB 445.21kB -25.48%
648.*.js (New) 376.06kB 376.06kB 100.0% 🚀
709.*.css (New) 285.35kB 285.35kB 100.0% 🚀
core.*.css 11.28kB 12.31kB 1104.11% ⚠️
core.*.js 53 bytes 3.1kB 1.74%
355.*.js (Deleted) -446.4kB 0 bytes -100.0% 🗑️
847.*.css (Deleted) -330.13kB 0 bytes -100.0% 🗑️

This was referenced Jan 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants