Feature/126 implement peppol#471
Conversation
Restore full Peppol integration on this feature branch: - Peppol models, migrations, enums, events, jobs, listeners, traits - EInvoiceBe and Storecove provider implementations - Format handlers (UBL, BIS, CII, Factur-X, ZUGFeRD, FatturaPA, etc.) - Artisan commands: PollPeppolStatus, RetryFailedTransmissions, TestPeppolIntegration - Client peppol-fields migrations for relations table - Full unit test suite for Peppol integration Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
nielsdrost7
force-pushed
the
feature/126-implement-peppol
branch
from
6c38c5d to
b3c2b7a
Compare
|
@coderabbitai full review |
✅ Action performedFull review finished. |
|
Hey @nielsdrost7 both this and #473 seems like huge PRs do to in 2 commits with apparently little oversight. v1 has some overly generous AI coding as well, but it does not seem to be as bad. This feels like it will turn into a maintenance nightmare. |
|
@b12f thanks for tagging me. Don't worry about #473 for a while, I had it working some time ago. You're correct about this PR, but i'll keep an eye on it. I squashed some commits, therefore it looks big, but trust me, I developed it diligently I can be found on Discord: https://discord.gg/PPzD2hTrXt |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
📝 WalkthroughWalkthroughThis PR adds Peppol invoicing support across database schema, models, enums, HTTP/provider clients, document format handlers, services, actions, queue jobs, console commands, audit events, tests, documentation, and resets the PHPStan ignored-errors baseline. ChangesPeppol platform
Sequence Diagram(s)sequenceDiagram
participant Action as SendInvoiceToPeppolAction
participant Service as PeppolService
participant Factory as FormatHandlerFactory
participant Client as DocumentsClient
participant Provider as EInvoiceBe API
Action->>Service: sendInvoiceToPeppol(invoice, additionalData)
Service->>Factory: createForInvoice(invoice)
Factory-->>Service: handler
Service->>Client: submitDocument(documentData)
Client->>Provider: POST api/documents
Provider-->>Client: document response
Client-->>Service: response
Service-->>Action: success, document_id, status
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (28)
Modules/Invoices/Tests/Unit/Peppol/FormatHandlers/FormatHandlersTest.php-34-34 (1)
34-34: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd native parameter types and remove placeholder unused parameters in data-provider tests.
Several test methods use untyped arguments and placeholder
$format = null, which violates the project typing rule and triggers avoidable static-analysis noise.Proposed refactor
- public function it_returns_correct_format($handlerClass, $expectedFormat): void + public function it_returns_correct_format(string $handlerClass, PeppolDocumentFormat $expectedFormat): void @@ - public function it_returns_correct_mime_type($handlerClass, $format = null): void + public function it_returns_correct_mime_type(string $handlerClass, PeppolDocumentFormat $expectedFormat): void { $handler = new $handlerClass(); $mimeType = $handler->getMimeType(); + $this->assertInstanceOf(PeppolDocumentFormat::class, $expectedFormat); @@ - public function it_returns_correct_file_extension($handlerClass, $format = null): void + public function it_returns_correct_file_extension(string $handlerClass, PeppolDocumentFormat $expectedFormat): voidAs per coding guidelines,
**/*.php: Use native PHP type hints throughout the codebase.Also applies to: 44-44, 55-55, 66-66, 80-80, 94-94, 115-115, 130-130, 145-145
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Tests/Unit/Peppol/FormatHandlers/FormatHandlersTest.php` at line 34, Add native PHP type hints to all parameters in the test method signatures and remove any unused placeholder parameters like `$format = null`. The method at line 34 (it_returns_correct_format) and the related test methods at lines 44, 55, 66, 80, 94, 115, 130, and 145 all need to have their parameter types explicitly declared (e.g., string, int, array, etc.) according to PHP type-hinting standards and remove any null-initialized placeholder parameters that are not used within the method body.Sources: Coding guidelines, Linters/SAST tools
Modules/Invoices/Peppol/FILES_CREATED.md-223-226 (1)
223-226:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDependency versions in docs conflict with the project target.
This section documents Laravel 12.x / PHP 8.2+, which conflicts with the repository target and can cause incorrect setup guidance.
As per coding guidelines,
**/*.{php,json}: Use Laravel 11 (PHP 8.1+) — not Laravel 12 or PHP 8.2.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Peppol/FILES_CREATED.md` around lines 223 - 226, The Production section in the FILES_CREATED.md documentation lists incorrect dependency versions that conflict with the project's actual target. Update the version specifications in this section to match the project's coding guidelines: change "Laravel 12.x" to "Laravel 11.x" and change "PHP 8.2+" to "PHP 8.1+". Keep the Filament 4.x reference as is, as it does not conflict with the project target.Source: Coding guidelines
Modules/Invoices/Tests/Unit/Peppol/Enums/PeppolDocumentFormatTest.php-116-117 (1)
116-117:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove intentionally disabled tests and make invalid-value checks executable.
At Line 151 and Line 159, both invalid-value test methods are effectively disabled by
markTestIncomplete(...), and they also omit#[Test]. Combined with#[Group('failing')]usage (Line 116 and Line 168), this leaves key negative-path assertions non-enforced in normal test execution.As per coding guidelines, "All tests must use PHPUnit exclusively with class-based approach using #[Test] attributes; never use Pest syntax (it(), test(), describe(), uses(), or expect() chains)".
Suggested fix
- #[Group('failing')] public function it_identifies_mandatory_formats_correctly( PeppolDocumentFormat $format, string $countryCode, bool $expectedMandatory ): void { $isMandatory = $format->isMandatoryFor($countryCode); $this->assertEquals($expectedMandatory, $isMandatory); } + #[Test] public function test_it_throws_on_invalid_enum_value(): void { - $this->markTestIncomplete('weird test'); - $this->expectException(ValueError::class); PeppolDocumentFormat::from('invalid_value'); } + #[Test] public function test_it_throws_on_invalid_enum_value_name(): void { - $this->markTestIncomplete('weird test'); - $this->expectException(ValueError::class); PeppolDocumentFormat::from('not_a_real_enum'); } - #[Group('failing')] public function it_provides_description_for_formats(): void { $description = PeppolDocumentFormat::PEPPOL_BIS_30->description();Also applies to: 151-165, 168-169
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Tests/Unit/Peppol/Enums/PeppolDocumentFormatTest.php` around lines 116 - 117, Remove the #[Group('failing')] attributes from test methods like it_identifies_mandatory_formats_correctly and other test methods to allow them to execute in normal test runs. Replace the markTestIncomplete(...) calls in the invalid-value test methods with proper test implementations, and add the missing #[Test] attributes to all test methods that currently omit them. Ensure all test methods follow the required PHPUnit class-based approach with #[Test] attributes for proper test discovery and execution, removing any barriers that prevent key negative-path assertions from being enforced during normal test execution.Source: Coding guidelines
Modules/Invoices/Tests/Unit/Peppol/Clients/DocumentsClientTest.php-196-209 (1)
196-209:⚠️ Potential issue | 🟠 Major | ⚡ Quick winError-path assertions conflict with the configured client stack.
These tests assert returned unsuccessful responses, but this class constructs
DocumentsClientwithHttpClientExceptionHandler, which (per its own unit tests) throws on 4xx/5xx. The current assertions validate the wrong contract.Use
expectException(RequestException::class)+ response assertions in catch blocks (or remove the exception decorator from this test fixture if you want raw-response behavior).Also applies to: 230-244, 246-260, 262-276
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Tests/Unit/Peppol/Clients/DocumentsClientTest.php` around lines 196 - 209, The test method it_handles_validation_errors asserts that a 422 response is returned and checks response properties, but the DocumentsClient is configured with HttpClientExceptionHandler which throws a RequestException on 4xx/5xx status codes instead of returning the response object. Refactor this test to use expectException(RequestException::class) to catch the thrown exception, then move any response assertions into a catch block if validation of the error details is needed. Apply the same fix pattern to the other similar test methods (the ones that assert on unsuccessful responses and check status codes or error messages) to align with the actual behavior of the HttpClientExceptionHandler.Modules/Invoices/Peppol/FormatHandlers/FormatHandlerFactory.php-98-103 (1)
98-103:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard missing-customer invoices before dereferencing
$customer.Line 102 dereferences
$customer->peppol_formatwithout a null check. If an invoice has no customer, this will crash the factory path.💡 Proposed fix
public static function createForInvoice(Invoice $invoice): InvoiceFormatHandlerInterface { $customer = $invoice->customer; - $countryCode = $customer->country_code ?? null; + if ($customer === null) { + throw new RuntimeException("Cannot resolve Peppol format: invoice {$invoice->id} has no customer."); + } + + $countryCode = $customer->country_code ?? null;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Peppol/FormatHandlers/FormatHandlerFactory.php` around lines 98 - 103, The code in the FormatHandlerFactory.php file retrieves a customer from the invoice and immediately dereferences its properties without verifying the customer exists. Add a null check to guard against the case where an invoice has no customer by verifying that $customer is not null before attempting to access $customer->peppol_format and other customer properties in the subsequent conditional logic.Modules/Invoices/Peppol/Enums/PeppolDocumentFormat.php-87-97 (1)
87-97:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
recommendedForCountry('NL')returns the wrong format.Line 95 maps
NLtoUBL_24, but the PR objective states NL must resolve toUBL_21. This breaks the intended country-format contract and can mis-route handler selection.💡 Proposed fix
- 'NL', 'BE', 'GB', 'SE', 'FI', 'XX', '' => self::UBL_24, - default => self::UBL_24, + 'NL', 'BE', 'GB', 'SE', 'FI', 'XX', '' => self::UBL_21, + default => self::UBL_21,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Peppol/Enums/PeppolDocumentFormat.php` around lines 87 - 97, The recommendedForCountry method incorrectly maps Netherlands (NL) to UBL_24 when it should map to UBL_21. In the match statement, remove 'NL' from the group on line 95 that maps to self::UBL_24, and create a new case for 'NL' that maps to self::UBL_21 instead. This ensures the country-format contract is honored and correct handler selection occurs.Modules/Invoices/Database/Migrations/2025_10_02_000002_create_peppol_integration_config_table.php-25-25 (1)
25-25:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBoth key/value tables allow duplicate keys per parent due to non-unique indexes. This creates ambiguous state and non-deterministic reads.
Modules/Invoices/Database/Migrations/2025_10_02_000002_create_peppol_integration_config_table.php#L25-L25: replace the composite index with a composite unique key on (integration_id,config_key).Modules/Invoices/Database/Migrations/2025_10_02_000004_create_peppol_transmission_responses_table.php#L25-L25: replace the composite index with a composite unique key on (transmission_id,response_key).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Database/Migrations/2025_10_02_000002_create_peppol_integration_config_table.php` at line 25, Both key/value configuration tables allow duplicate keys per parent due to non-unique composite indexes, creating ambiguous state. In Modules/Invoices/Database/Migrations/2025_10_02_000002_create_peppol_integration_config_table.php at line 25, replace the $table->index() call with $table->unique() on the same columns (integration_id, config_key) to enforce uniqueness. Similarly, in Modules/Invoices/Database/Migrations/2025_10_02_000004_create_peppol_transmission_responses_table.php at line 25, replace the $table->index() call with $table->unique() on (transmission_id, response_key) to ensure only one response_key value can exist per transmission_id.Modules/Invoices/Peppol/FormatHandlers/EhfHandler.php-109-109 (1)
109-109: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winUse native parameter types on helper methods.
Untyped helper params (
$endpointScheme,$item) should be typed to satisfy repository standards and tighten contracts.As per coding guidelines, "
**/*.php: Use native PHP type hints throughout the codebase".Also applies to: 173-173, 492-492
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Peppol/FormatHandlers/EhfHandler.php` at line 109, Add native PHP type hints to untyped parameters in helper methods throughout the file to satisfy coding standards. In the buildSupplierParty method, add an appropriate type hint to the $endpointScheme parameter (likely string). Apply the same type-hinting approach to other helper methods in the file that have untyped parameters (such as methods with $item parameters) to ensure all method signatures include complete type information as per the repository's PHP coding guidelines.Source: Coding guidelines
Modules/Invoices/Peppol/FormatHandlers/CiiHandler.php-137-137 (1)
137-137: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd native parameter type hints to helper methods.
Several helper signatures still use untyped parameters (
$customer,$company,$items), which violates the project rule and weakens static safety.Example direction
- protected function buildHeaderTradeAgreement(Invoice $invoice, $customer): array + protected function buildHeaderTradeAgreement(Invoice $invoice, object $customer): array - protected function buildSellerParty($company): array + protected function buildSellerParty(object $company): array - protected function buildBuyerParty($customer): array + protected function buildBuyerParty(object $customer): array - protected function buildHeaderTradeSettlement(Invoice $invoice, $customer, $company): array + protected function buildHeaderTradeSettlement(Invoice $invoice, object $customer, object $company): array - protected function buildLineItems($items, string $currencyCode): array + protected function buildLineItems(iterable $items, string $currencyCode): arrayAs per coding guidelines, "
**/*.php: Use native PHP type hints throughout the codebase".Also applies to: 153-153, 190-190, 233-233, 317-317
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Peppol/FormatHandlers/CiiHandler.php` at line 137, Add native PHP type hints to all untyped parameters in the helper methods throughout the CiiHandler.php file. The methods affected are at lines 137, 153, 190, 233, and 317. For each method, identify untyped parameters like $customer, $company, and $items, determine their expected types based on the method's logic and class context, and add appropriate native type hints (e.g., Customer, Company, array, etc.) to match the project's coding guidelines requiring type hints throughout the codebase.Source: Coding guidelines
Modules/Invoices/Peppol/FormatHandlers/CiiHandler.php-42-59 (1)
42-59:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard missing customer relation before field access in validation.
validate()reads$customer->name/$customer->country_codewithout verifying$invoice->customeris present, which can break the validation path.Suggested fix
public function validate(Invoice $invoice): array { $errors = []; $customer = $invoice->customer; + if (! $customer) { + $errors[] = 'Customer is required for CII format'; + return $errors; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Peppol/FormatHandlers/CiiHandler.php` around lines 42 - 59, The validate() method accesses $customer->name and $customer->country_code properties without first verifying that the $invoice->customer relation exists. Add a guard clause at the beginning of the validation block (right after assigning $customer = $invoice->customer) to check if $customer is null or empty, and if so, add an appropriate error message to the $errors array. This ensures the customer relation is present before attempting to access its fields.Modules/Invoices/Peppol/Providers/EInvoiceBe/EInvoiceBeProvider.php-44-47 (1)
44-47:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
testConnection()ignores its$configinput, so connection checks can be misleading.The method receives provider config but always uses pre-resolved clients (
Lines 44-47). This can test default credentials instead of the credentials being validated.Also applies to: 67-70
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Peppol/Providers/EInvoiceBe/EInvoiceBeProvider.php` around lines 44 - 47, The testConnection() method receives a $config parameter but always uses the pre-resolved client properties (documentsClient, participantsClient, trackingClient, healthClient) that were initialized in the constructor with default credentials, ignoring the passed configuration. To fix this, refactor testConnection() to instantiate new client instances from the provided $config parameter instead of using the instance properties, ensuring that the connection test actually validates the credentials being provided in the $config input rather than testing default credentials.Modules/Invoices/Peppol/Providers/Storecove/StorecoveProvider.php-33-121 (1)
33-121:⚠️ Potential issue | 🟠 MajorPrevent Storecove selection until implementation is complete.
All core provider methods are placeholders that return "not yet implemented". Storecove is currently listed as a supported provider in the config and can be instantiated via ProviderFactory, meaning users can select it and trigger production failures across send/status/cancel flows (used in
SendInvoiceToPeppolJob,PeppolStatusPoller, andPeppolManagementService).Recommend either: (1) Remove "storecove" from supported providers list until implementation is ready, or (2) Add an explicit unsupported exception at factory instantiation level to prevent runtime failures.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Peppol/Providers/Storecove/StorecoveProvider.php` around lines 33 - 121, The StorecoveProvider class contains only placeholder implementations for all methods (testConnection, validatePeppolId, sendInvoice, getTransmissionStatus, cancelDocument) that return "not yet implemented" messages, yet the provider is still selectable in production. To prevent users from selecting an incomplete provider and triggering failures in SendInvoiceToPeppolJob, PeppolStatusPoller, and PeppolManagementService, either remove "storecove" from the list of supported providers in the provider configuration or add an explicit exception throw in the ProviderFactory at the point where StorecoveProvider would be instantiated to block its use until implementation is complete.Modules/Invoices/Peppol/Providers/EInvoiceBe/EInvoiceBeProvider.php-35-43 (1)
35-43:⚠️ Potential issue | 🟠 MajorNarrow constructor type to
?PeppolIntegrationto prevent runtime type failures.Line 36 accepts
?object, but line 42 passes it toBaseProvider::__construct(?PeppolIntegration ...). Any non-PeppolIntegrationobject will fail at runtime.🐛 Suggested fix
+use Modules\Invoices\Models\PeppolIntegration; ... public function __construct( - ?object $integration = null, + ?PeppolIntegration $integration = null, ?DocumentsClient $documentsClient = null, ?ParticipantsClient $participantsClient = null, ?TrackingClient $trackingClient = null, ?HealthClient $healthClient = null🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Peppol/Providers/EInvoiceBe/EInvoiceBeProvider.php` around lines 35 - 43, The EInvoiceBeProvider::__construct method declares the $integration parameter with an overly broad type of ?object, but then passes it to parent::__construct() which expects ?PeppolIntegration. This type mismatch allows incorrect object types to be passed, causing runtime failures. Change the type hint of the $integration parameter in EInvoiceBeProvider::__construct from ?object to ?PeppolIntegration to properly enforce type safety and match the parent class requirement.Modules/Invoices/Peppol/Services/PeppolService.php-23-24 (1)
23-24: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winMake this service extend
Modules\Core\Services\BaseService.Line 23 currently declares a standalone service class, but services in this codebase must inherit the shared base service.
Suggested change
namespace Modules\Invoices\Peppol\Services; use Illuminate\Http\Client\RequestException; use InvalidArgumentException; +use Modules\Core\Services\BaseService; ... -class PeppolService +class PeppolService extends BaseService {As per coding guidelines, "All services must extend Modules\Core\Services\BaseService and use array-based parameters without DTO layer".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Peppol/Services/PeppolService.php` around lines 23 - 24, The PeppolService class is currently declared as a standalone service without extending the required base service class. Modify the class declaration to extend Modules\Core\Services\BaseService so that PeppolService inherits the shared base service functionality as per the codebase's coding guidelines requiring all services to extend BaseService.Source: Coding guidelines
Modules/Invoices/Peppol/Services/PeppolManagementService.php-29-30 (1)
29-30:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftAlign this service with the module base-service contract.
Line 29 defines
PeppolManagementServicewithout extendingModules\Core\Services\BaseService, which breaks the Services-layer contract in this repository.As per coding guidelines "
**/Services/*.php: All services must extend Modules\Core\Services\BaseService and use array-based parameters without DTO layer".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Peppol/Services/PeppolManagementService.php` around lines 29 - 30, The PeppolManagementService class definition does not extend the required Modules\Core\Services\BaseService base class, violating the repository's service-layer contract. Modify the PeppolManagementService class declaration to extend Modules\Core\Services\BaseService so it adheres to the coding guidelines that mandate all service classes must inherit from this base service.Source: Coding guidelines
Modules/Invoices/Models/PeppolIntegration.php-119-124 (1)
119-124: 🛠️ Refactor suggestion | 🟠 MajorAdd native type hints to
getConfigValue.The method signature at line 119 omits native type hints for the
$defaultparameter and return type, violating the project guideline to use native PHP type hints throughout the codebase.Proposed fix
- public function getConfigValue(string $key, $default = null) + public function getConfigValue(string $key, mixed $default = null): mixed🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Models/PeppolIntegration.php` around lines 119 - 124, Add native PHP type hints to the getConfigValue method to comply with project guidelines. Add a return type hint to the method signature (use mixed as the return type since it can return either the config_value or the default parameter which could be any type). Add a type hint to the $default parameter (use mixed as the type since it defaults to null but can accept any value). This ensures the method signature is fully type-hinted as required by the project standard.Source: Coding guidelines
Modules/Invoices/Peppol/Services/PeppolManagementService.php-61-63 (1)
61-63:⚠️ Potential issue | 🟠 MajorDispatch Peppol domain events only after database transaction commits.
Events at lines 61 and 184 are dispatched before
DB::commit()(lines 63 and 189). If the commit fails, event listeners may execute against unpersisted state, compromising data integrity.Move event dispatches to occur only after successful commit using
DB::afterCommit():Proposed fix pattern
- event(new PeppolIntegrationCreated($integration)); - - DB::commit(); + DB::commit(); + DB::afterCommit(fn () => event(new PeppolIntegrationCreated($integration)));- event(new PeppolIdValidationCompleted($customer, $validationStatus->value, [ - 'history_id' => $history->id, - 'present' => $result['present'], - ])); - - DB::commit(); + DB::commit(); + DB::afterCommit(fn () => event(new PeppolIdValidationCompleted($customer, $validationStatus->value, [ + 'history_id' => $history->id, + 'present' => $result['present'], + ])));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Peppol/Services/PeppolManagementService.php` around lines 61 - 63, The PeppolIntegrationCreated event dispatch occurs before DB::commit(), which means if the transaction fails, event listeners will execute against uncommitted data, compromising integrity. Wrap the event dispatch call in DB::afterCommit() to ensure it executes only after the transaction successfully commits. Apply this same fix to the second event dispatch mentioned in the comment (around line 184) to ensure consistent transactional behavior across all Peppol domain events in this service.Modules/Invoices/Models/PeppolTransmission.php-114-121 (1)
114-121:⚠️ Potential issue | 🟠 MajorAdd JSON error handling and implement BelongsToCompany trait to ensure data integrity and meet model guidelines.
Line 119 stores the result of
json_encode()without error handling, which silently persistsfalseon encoding failure, corrupting response data. Additionally, this model is inModules/Invoices/Models/and must use theBelongsToCompanytrait per coding guidelines to auto-inject company context on create and apply global scope—however, the database schema currently lacks acompany_idcolumn.JSON encoding fix
- ['response_value' => is_array($value) ? json_encode($value) : $value] + [ + 'response_value' => is_array($value) + ? json_encode($value, JSON_THROW_ON_ERROR) + : $value, + ]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Models/PeppolTransmission.php` around lines 114 - 121, In the setProviderResponse method of the PeppolTransmission model, wrap the json_encode call with error handling to detect and handle encoding failures instead of silently storing false values—check the return value and either throw an exception or log the error before persisting. Additionally, add the BelongsToCompany trait to the PeppolTransmission class to ensure company context is automatically injected and global scoping is applied, and ensure the corresponding database migration or schema includes a company_id column to support this relationship.Modules/Invoices/Jobs/Peppol/SendInvoiceToPeppolJob.php-122-126 (1)
122-126:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid logging raw exception traces in this worker path.
At Line 125, full stack traces can expose sensitive internals in centralized logs. Keep structured error metadata and reserve traces for guarded debug-only channels.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Jobs/Peppol/SendInvoiceToPeppolJob.php` around lines 122 - 126, The logPeppolError method call in SendInvoiceToPeppolJob is logging the full exception stack trace via getTraceAsString(), which can expose sensitive internals in centralized logs. Remove the 'trace' key and its corresponding $e->getTraceAsString() value from the error metadata array passed to logPeppolError, keeping only the invoice_id and error message for structured error tracking.Modules/Invoices/Jobs/Peppol/SendInvoiceToPeppolJob.php-96-99 (1)
96-99:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTreat validation failures as non-retriable job failures.
At Line 97 and Line 121-133,
InvalidArgumentExceptionis rethrown like transient runtime failures. That causes repeated queue retries for permanently invalid invoices.Proposed fix
public function handle(): void { try { ... - } catch (Exception $e) { + } catch (InvalidArgumentException $e) { + $this->logPeppolWarning('Peppol sending aborted due to invalid invoice data', [ + 'invoice_id' => $this->invoice->id, + 'error' => $e->getMessage(), + ]); + $this->fail($e); + return; + } catch (Exception $e) { $this->logPeppolError('Peppol sending job failed', [ 'invoice_id' => $this->invoice->id, 'error' => $e->getMessage(), - 'trace' => $e->getTraceAsString(), ]);Also applies to: 121-133
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Jobs/Peppol/SendInvoiceToPeppolJob.php` around lines 96 - 99, The validateInvoice() method call and subsequent validation logic are allowing InvalidArgumentException to propagate as retriable errors when they represent permanent failures. Catch InvalidArgumentException thrown from validateInvoice() at the call site and handle it as a permanent job failure by failing the job immediately without queue retry capability. Apply the same treatment to the validation error handling that occurs in the 121-133 line range to ensure all validation exceptions are marked as non-retriable and prevent the queue system from automatically retrying jobs with invalid invoices.Modules/Invoices/Jobs/Peppol/RetryFailedTransmissions.php-44-47 (1)
44-47:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftAtomically claim each due transmission before re-dispatching send jobs.
At Line 44-47 and Line 87-92, due rows are read then dispatched without a claim transition. Concurrent workers can dispatch the same transmission more than once.
Proposed fix
foreach ($transmissions as $transmission) { try { + $claimed = PeppolTransmission::query() + ->whereKey($transmission->id) + ->where('status', PeppolTransmissionStatus::RETRYING) + ->where('next_retry_at', '<=', now()) + ->update([ + 'status' => PeppolTransmissionStatus::PROCESSING, + 'next_retry_at' => null, + ]); + + if ($claimed === 0) { + continue; // already claimed by another worker + } + $this->retryTransmission($transmission); } catch (Exception $e) {Also applies to: 87-92
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Jobs/Peppol/RetryFailedTransmissions.php` around lines 44 - 47, The code reads transmissions with status RETRYING and due for retry, then dispatches jobs without first atomically claiming each transmission. This allows concurrent workers to process the same transmission record multiple times. Implement an atomic claim mechanism by updating each transmission's status to a claimed state (or using a database-level lock/update pattern) within a transaction before dispatching the send job in the RetryFailedTransmissions job handler. This ensures only one worker can claim and process each transmission, eliminating duplicate dispatches across concurrent workers.Modules/Invoices/Jobs/Peppol/SendInvoiceToPeppolJob.php-437-457 (1)
437-457:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse one retry scheduler path to prevent duplicate resend attempts.
At Line 455-456, this job self-dispatches delayed retries, while
Modules/Invoices/Jobs/Peppol/RetryFailedTransmissions.phpalso dispatches due retries. Running both paths can duplicate sends for the same transmission.Proposed fix (if scanner job is the chosen scheduler)
- // Re-dispatch the job - static::dispatch($this->invoice, $this->integration, false, $transmission->id) - ->delay($nextRetryAt); + // Retry dispatch is handled by RetryFailedTransmissions scanner.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Jobs/Peppol/SendInvoiceToPeppolJob.php` around lines 437 - 457, The scheduleRetry method in SendInvoiceToPeppolJob is self-dispatching delayed job retries while RetryFailedTransmissions.php also handles retries, creating duplicate send attempts. Remove the static::dispatch call and ->delay() logic from the scheduleRetry method (the last two lines of the diff) so that only RetryFailedTransmissions manages dispatching retries. Keep the transmission scheduling logic (the call to $transmission->scheduleRetry($nextRetryAt)) so the transmission state is updated and RetryFailedTransmissions can pick it up.Modules/Invoices/Listeners/Peppol/LogPeppolEventToAudit.php-36-41 (1)
36-41:⚠️ Potential issue | 🟠 MajorRegister Peppol models in morphMap or use morph-compatible audit_type values.
LogPeppolEventToAuditstoresaudit_typevalues (peppol_transmission,peppol_integration,peppol_validation,peppol_event) that are not registered in the morph map. TheAuditLog::audit()method usesmorphTo(), which expects discriminators to be either registered morph aliases or fully qualified class names. WhengetFormattedActivityAttribute()accesses$this->audit, the relation resolution will fail for these values, breaking audit log access at runtime.Add
PeppolTransmissionandPeppolIntegrationto the morph map inAppServiceProvider:Relation::morphMap([ 'invoice' => Invoice::class, + 'peppol_transmission' => PeppolTransmission::class, + 'peppol_integration' => PeppolIntegration::class, ]);Then update
getAuditType()inLogPeppolEventToAuditto return the corresponding morph alias:protected function getAuditType(PeppolEvent $event): string { $eventName = $event->getEventName(); if (str_contains($eventName, 'transmission')) { - return 'peppol_transmission'; + return 'peppol_transmission'; // matches morph alias } if (str_contains($eventName, 'integration')) { - return 'peppol_integration'; + return 'peppol_integration'; // matches morph alias } if (str_contains($eventName, 'validation')) { - return 'peppol_validation'; + return 'peppol_transmission'; // should this be transmission or create separate entry? } - return 'peppol_event'; + return 'peppol_transmission'; // fallback—clarify intent }Also add cases to
getFormattedActivityAttribute()inAuditLogto handle Peppol model types.Also applies to: 83-98
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Listeners/Peppol/LogPeppolEventToAudit.php` around lines 36 - 41, The audit_type values for Peppol events (peppol_transmission, peppol_integration, peppol_validation, peppol_event) are not registered in the morph map, causing runtime failures when the morphTo() relation tries to resolve them. Register PeppolTransmission and PeppolIntegration models in the morph map within AppServiceProvider. Then update the getAuditType() method in LogPeppolEventToAudit to return the corresponding morph alias strings (e.g., 'peppol_transmission') instead of custom identifiers. Finally, add cases to the getFormattedActivityAttribute() method in AuditLog to handle the Peppol model types and properly format their activity display.Modules/Invoices/Http/Traits/LogsApiRequests.php-124-130 (1)
124-130:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHeader redaction is case-sensitive and can leak credentials.
Line 124 onward only redacts exact keys (
Authorization,X-API-Key,X-Auth-Token). Lowercase/mixed-case variants (e.g.,authorization) will be logged unredacted.Suggested fix
- if (isset($sanitized['headers'])) { - $sensitiveHeaders = ['Authorization', 'X-API-Key', 'X-Auth-Token']; - foreach ($sensitiveHeaders as $header) { - if (isset($sanitized['headers'][$header])) { - $sanitized['headers'][$header] = '***REDACTED***'; - } - } - } + if (isset($sanitized['headers']) && is_array($sanitized['headers'])) { + $sensitiveHeaders = ['authorization', 'x-api-key', 'x-auth-token']; + foreach ($sanitized['headers'] as $header => $value) { + if (in_array(mb_strtolower((string) $header), $sensitiveHeaders, true)) { + $sanitized['headers'][$header] = '***REDACTED***'; + } + } + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Http/Traits/LogsApiRequests.php` around lines 124 - 130, The header redaction logic in LogsApiRequests.php uses case-sensitive exact key matching, which means HTTP headers with different case variants (e.g., 'authorization' instead of 'Authorization') will not be redacted and credentials will leak. Change the redaction approach to iterate through the actual headers in the $sanitized['headers'] array and compare each header key against the sensitive headers list using case-insensitive comparison (such as strtolower or strcasecmp), ensuring all variants of Authorization, X-API-Key, and X-Auth-Token are properly redacted regardless of casing.Modules/Invoices/Http/Clients/ApiClient.php-38-40 (1)
38-40:⚠️ Potential issue | 🟠 MajorRemove unconditional
->throw()to allow status-code-based error handling in downstream callers.The
request()method unconditionally calls->throw()on line 40, which throws aRequestExceptionfor all 4xx/5xx responses before they can reach callers. This breaks the 404 handling inEInvoiceBeProvider::validatePeppolId()(lines 125–129), where code checks$response->status() === 404to determine if a participant is not found—this branch is now unreachable because the exception is thrown instead.Suggested fix
- return $client - ->{$method->value}($uri, $options['payload'] ?? []) - ->throw(); + $response = $client + ->{$method->value}($uri, $options['payload'] ?? []); + + if (($options['throw'] ?? false) === true) { + $response->throw(); + } + + return $response;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Http/Clients/ApiClient.php` around lines 38 - 40, Remove the unconditional ->throw() call from the return statement in the request() method of ApiClient. The ->throw() on the return chain prevents callers like EInvoiceBeProvider::validatePeppolId() from receiving the response object to check status codes (such as checking for 404). Instead of throwing immediately, return the response object directly so downstream callers can handle specific HTTP status codes through conditional logic on $response->status().Modules/Invoices/Peppol/FormatHandlers/FacturXHandler.php-45-55 (1)
45-55:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
generateXml()currently returns JSON instead of XML payload.This breaks the format contract when Factur-X is selected: downstream code expecting XML/PDF-A-3 content will receive JSON text.
🔧 Suggested minimal safe fix (fail fast until XML generation is implemented)
public function generateXml(Invoice $invoice, array $options = []): string { - $data = $this->transform($invoice, $options); - - // Placeholder - would generate proper CII XML embedded in PDF/A-3 - // For Factur-X, this would: - // 1. Generate the CII XML - // 2. Generate a PDF from the invoice - // 3. Embed the XML into the PDF as PDF/A-3 attachment - return json_encode($data, JSON_PRETTY_PRINT); + throw new \RuntimeException('Factur-X XML generation is not implemented yet.'); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Peppol/FormatHandlers/FacturXHandler.php` around lines 45 - 55, The generateXml() method in the FacturXHandler class currently returns JSON-encoded data instead of XML/PDF-A-3 content, which breaks the format contract when Factur-X is selected. Replace the json_encode($data, JSON_PRETTY_PRINT) return statement with code that throws an exception to fail fast and explicitly indicate that Factur-X XML generation is not yet implemented, rather than silently returning incorrect JSON format that would cause downstream failures.Modules/Invoices/Peppol/FormatHandlers/FacturXHandler.php-361-364 (1)
361-364:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHardcoded 20% tax fallback can generate incorrect invoice totals.
Defaulting missing
tax_rateto20.0will overstate tax for exempt/zero-tax lines and non-20% jurisdictions, which can corrupt transmitted fiscal data.🔧 Suggested fix
protected function getTaxRate(mixed $item): float { - return $item->tax_rate ?? 20.0; // Default French VAT rate + return isset($item->tax_rate) ? (float) $item->tax_rate : 0.0; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Peppol/FormatHandlers/FacturXHandler.php` around lines 361 - 364, The getTaxRate method in the FacturXHandler class uses a hardcoded 20.0 percent default for missing tax_rate values, which incorrectly assumes all items should default to French VAT and can corrupt fiscal data. Replace the hardcoded default with either a 0.0 default (for tax-exempt items), a configurable default retrieved from settings, or remove the default entirely and throw an exception to ensure tax_rate is always explicitly provided before processing the invoice.Modules/Invoices/Peppol/FormatHandlers/UblHandler.php-65-71 (1)
65-71:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftAll
generateXml()methods return JSON — stored artifacts will be invalid XML.All six format handlers implement
generateXml()as a placeholder that returnsjson_encode($data, JSON_PRETTY_PRINT)instead of actual XML. PerSendInvoiceToPeppolJob::prepareArtifacts()(context snippet), this output is stored atstored_xml_path. The mismatch will cause downstream XML validators or Peppol network receivers to reject the documents.
Modules/Invoices/Peppol/FormatHandlers/UblHandler.php#L65-L71: Replace JSON placeholder with proper UBL XML generation (e.g., usingsabre/xmlor a dedicated UBL library).Modules/Invoices/Peppol/FormatHandlers/FacturaeHandler.php#L58-L64: Implement Facturae 3.2 XML generation.Modules/Invoices/Peppol/FormatHandlers/FatturaPaHandler.php#L57-L63: Implement FatturaPA 1.2 XML generation.Modules/Invoices/Peppol/FormatHandlers/OioublHandler.php#L90-L96: Implement OIOUBL XML generation.Modules/Invoices/Peppol/FormatHandlers/PeppolBisHandler.php#L150-L157: Implement PEPPOL BIS 3.0 XML generation.Modules/Invoices/Peppol/FormatHandlers/ZugferdHandler.php#L58-L64: Implement ZUGFeRD CII XML generation (and PDF/A-3 embedding for production).If this is intentionally deferred, consider throwing
NotImplementedExceptionor gating transmission behind a feature flag to prevent invalid artifacts from being stored/sent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Peppol/FormatHandlers/UblHandler.php` around lines 65 - 71, All six format handlers have placeholder generateXml() methods that return json_encode() instead of proper XML, causing invalid artifacts to be stored. In Modules/Invoices/Peppol/FormatHandlers/UblHandler.php (lines 65-71), replace the json_encode placeholder with proper UBL XML generation using a library like sabre/xml. Apply the same pattern to the remaining five sibling handlers: FacturaeHandler.php (lines 58-64) for Facturae 3.2 XML, FatturaPaHandler.php (lines 57-63) for FatturaPA 1.2 XML, OioublHandler.php (lines 90-96) for OIOUBL XML, PeppolBisHandler.php (lines 150-157) for PEPPOL BIS 3.0 XML, and ZugferdHandler.php (lines 58-64) for ZUGFeRD CII XML generation. If implementation is deferred, throw NotImplementedException in the generateXml() method at all six locations to prevent invalid artifacts from being stored or transmitted to the Peppol network.
🧹 Nitpick comments (9)
Modules/Invoices/Peppol/FormatHandlers/BaseFormatHandler.php (1)
150-156: ⚡ Quick winRemove the unused variadic parameter from
getCurrencyCode().Line 155 defines
...$argsbut never uses it. This keeps PHPMD noise and widens the method signature without value.💡 Proposed refactor
- protected function getCurrencyCode(Invoice $invoice, ...$args): string + protected function getCurrencyCode(Invoice $invoice): string🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Peppol/FormatHandlers/BaseFormatHandler.php` around lines 150 - 156, The getCurrencyCode() method in BaseFormatHandler.php accepts a variadic parameter ...$args but never uses it in the method body, creating unnecessary complexity and PHPMD warnings. Simply remove the ...$args parameter from the method signature on line 155, keeping only the Invoice parameter, to clean up the code and reduce noise.Source: Linters/SAST tools
Modules/Invoices/Enums/PeppolConnectionStatus.php (1)
21-27: ⚡ Quick winLocalize enum labels via
trans()instead of hardcoded strings.
label()currently hardcodes English values. Move these to translation keys so enum labels are i18n-ready.♻️ Suggested change
public function label(): string { return match ($this) { - self::UNTESTED => 'Untested', - self::SUCCESS => 'Success', - self::FAILED => 'Failed', + self::UNTESTED => trans('invoices::peppol.connection_status.untested'), + self::SUCCESS => trans('invoices::peppol.connection_status.success'), + self::FAILED => trans('invoices::peppol.connection_status.failed'), }; }As per coding guidelines, "Use
trans()function for internationalization, never use__()".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Enums/PeppolConnectionStatus.php` around lines 21 - 27, The label() method in the PeppolConnectionStatus enum is returning hardcoded English strings for the match cases (UNTESTED, SUCCESS, FAILED). Replace each of these hardcoded string values with trans() function calls using appropriate translation keys (following your project's i18n naming conventions, typically something like 'invoices.peppol_connection_status.untested'). Ensure you use the trans() function as specified in the coding guidelines, not __().Source: Coding guidelines
Modules/Invoices/Peppol/Clients/EInvoiceBe/DocumentsClient.php (1)
183-191: The code is functionally correct—Laravel's HTTP client automatically treats the second parameter as query parameters for GET requests, so filtering works as intended. However, using the 'payload' key for GET request filters is semantically confusing since 'payload' conventionally refers to request bodies (POST/PUT/PATCH). Rename to 'query' for clarity:Suggested change
- $options = array_merge($this->getRequestOptions(), [ - 'payload' => $filters, - ]); + $options = array_merge($this->getRequestOptions(), [ + 'query' => $filters, + ]);This improves code readability without changing functionality.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Peppol/Clients/EInvoiceBe/DocumentsClient.php` around lines 183 - 191, In the method that calls $this->client->request() with RequestMethod::GET and builds the URL 'api/documents', change the 'payload' key in the options array to 'query'. This semantic change clarifies that these are query parameters for a GET request rather than a request body payload, making the code's intent more obvious to future readers. No functional change is needed—just rename the key from 'payload' to 'query' in the array_merge call.Modules/Invoices/Peppol/FormatHandlers/OioublHandler.php (3)
40-77: ⚡ Quick winUnused
$customervariable.
$customeris assigned on line 42 but never used intransform().♻️ Suggested fix
public function transform(Invoice $invoice, array $options = []): array { - $customer = $invoice->customer; $currencyCode = $this->getCurrencyCode($invoice); $endpointScheme = $this->getEndpointScheme($invoice);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Peppol/FormatHandlers/OioublHandler.php` around lines 40 - 77, The variable `$customer` is assigned in the `transform()` method of the `OioublHandler` class but is never used anywhere in the method body. Remove the line that assigns `$customer = $invoice->customer;` to eliminate the unused variable.Source: Linters/SAST tools
334-356: ⚡ Quick winUnused
$taxAmountvariable.
$taxAmountis computed on line 336 but never used inbuildMonetaryTotal().♻️ Suggested fix
protected function buildMonetaryTotal(Invoice $invoice, string $currencyCode): array { - $taxAmount = $invoice->invoice_total - $invoice->invoice_subtotal; - return [🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Peppol/FormatHandlers/OioublHandler.php` around lines 334 - 356, The buildMonetaryTotal() method calculates a $taxAmount variable but never uses it anywhere in the function body. Remove the line that assigns $taxAmount to eliminate this unused variable and clean up the code.Source: Linters/SAST tools
371-410: ⚡ Quick winUnused
$taxAmountin invoice line closure.
$taxAmountis computed on line 375 but never used in the returned line structure.♻️ Suggested fix
return $invoice->invoiceItems->map(function ($item, $index) use ($currencyCode) { $taxRate = $this->getTaxRate($item); - $taxAmount = $item->subtotal * ($taxRate / 100); return [🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Peppol/FormatHandlers/OioublHandler.php` around lines 371 - 410, The variable $taxAmount is calculated in the closure within the buildInvoiceLines method but is never referenced in the returned array structure. Remove the unused $taxAmount variable assignment (the line that calculates item->subtotal * (taxRate / 100)) from the closure, as it serves no purpose and constitutes dead code.Source: Linters/SAST tools
Modules/Invoices/Peppol/FormatHandlers/FatturaPaHandler.php (1)
38-47: ⚡ Quick winUnused
$customervariable.
$customeris assigned on line 40 but never referenced intransform(). Remove the dead assignment.♻️ Suggested fix
public function transform(Invoice $invoice, array $options = []): array { - $customer = $invoice->customer; $currencyCode = $this->getCurrencyCode($invoice); return [🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Peppol/FormatHandlers/FatturaPaHandler.php` around lines 38 - 47, The transform() method in the FatturaPaHandler class assigns the variable $customer from $invoice->customer but never uses it anywhere in the method body. Remove the unused variable assignment line that assigns $customer to eliminate the dead code.Source: Linters/SAST tools
Modules/Invoices/Peppol/FormatHandlers/UblHandler.php (1)
32-60: ⚡ Quick winUnused
$customerand$endpointSchemevariables intransform().Both are assigned but only used indirectly through helper methods that re-fetch them.
♻️ Suggested fix
public function transform(Invoice $invoice, array $options = []): array { - $customer = $invoice->customer; $currencyCode = $this->getCurrencyCode($invoice); - $endpointScheme = $this->getEndpointScheme($invoice); return [🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Peppol/FormatHandlers/UblHandler.php` around lines 32 - 60, The transform() method in UblHandler assigns two variables, $customer and $endpointScheme, that are never directly used within the method body. Remove both variable assignments since the helper methods (buildSupplierParty(), buildCustomerParty(), buildInvoiceLines(), and buildMonetaryTotals()) re-fetch these values themselves rather than accepting them as parameters.Source: Linters/SAST tools
Modules/Invoices/Peppol/FormatHandlers/FacturaeHandler.php (1)
362-388: 💤 Low valueUnused
$indexparameter in closure.The
$indexparameter is received but never used. If line numbering is needed for Facturae (similar to FatturaPA'sNumeroLinea), add it; otherwise remove the parameter.♻️ Suggested fix
- protected function buildItems(Invoice $invoice, string $currencyCode): array - { - $items = $invoice->invoiceItems->map(function ($item, $index) { + protected function buildItems(Invoice $invoice, string $currencyCode): array + { + $items = $invoice->invoiceItems->map(function ($item) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Peppol/FormatHandlers/FacturaeHandler.php` around lines 362 - 388, The closure function in the map() call within the invoice items processing block receives an `$index` parameter that is not used. Either remove the unused `$index` parameter from the closure signature if line numbering is not required for the Facturae format, or if line numbering is needed (similar to FatturaPA's NumeroLinea), use the index value and include it in the returned InvoiceLine structure to properly number each invoice line in the formatted output.
| if ($invoice->invoice_status === 'draft') { | ||
| throw new InvalidArgumentException('Cannot send draft invoices to Peppol'); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect Invoice model status fields/casts/accessors and draft references.
fd -i 'Invoice.php' Modules/Invoices/Models -t f
rg -n -C3 '\binvoice_status\b|\binvoice_status_id\b|casts|Attribute::make|draft' Modules/Invoices/Models/Invoice.php Modules/InvoicesRepository: InvoicePlane/InvoicePlane-v2
Length of output: 46310
Replace string literal with enum constant for invoice status check.
Line 108 compares $invoice->invoice_status to the string 'draft', but the field is cast to the InvoiceStatus enum in the Invoice model. This comparison will always fail, silently allowing draft invoices to pass validation. Use InvoiceStatus::DRAFT instead:
if ($invoice->invoice_status === InvoiceStatus::DRAFT) {
throw new InvalidArgumentException('Cannot send draft invoices to Peppol');
}See InvoiceService.php line 183 for the correct pattern.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Modules/Invoices/Actions/SendInvoiceToPeppolAction.php` around lines 108 -
109, In the SendInvoiceToPeppolAction class, the condition that checks if the
invoice status is draft compares the invoice_status field to the string literal
'draft', but since invoice_status is cast to the InvoiceStatus enum in the
Invoice model, this comparison will always fail. Replace the string literal
'draft' with the enum constant InvoiceStatus::DRAFT in the if condition to
properly validate that draft invoices cannot be sent to Peppol.
| /** | ||
| * Polls providers for status updates of recently sent Peppol transmissions and updates local records. | ||
| * | ||
| * Retrieves up to 100 transmissions that are marked SENT, have an external ID, are not yet acknowledged, | ||
| * and were sent more than five minutes ago. For each transmission it checks the provider status, updates | ||
| * the transmission state accordingly, and logs per-transmission errors without aborting the batch. | ||
| * | ||
| * Logs the start and completion of the polling job and includes the number of transmissions checked. | ||
| * / | ||
| * public function handle(): void | ||
| * { | ||
| * $this->logPeppolInfo('Starting Peppol status polling job'); | ||
| * | ||
| * // Get all transmissions awaiting acknowledgement | ||
| * $transmissions = PeppolTransmission::query()->where('status', PeppolTransmissionStatus::SENT) | ||
| * ->whereNotNull('external_id') | ||
| * ->whereNull('acknowledged_at') | ||
| * ->where('sent_at', '<', now()->subMinutes(5)) // Allow 5 min grace period | ||
| * ->limit(100) // Process in batches | ||
| * ->get(); | ||
| * | ||
| * foreach ($transmissions as $transmission) { | ||
| * try { | ||
| * $this->checkStatus($transmission); | ||
| * } catch (\Exception $e) { | ||
| * $this->logPeppolError('Failed to check transmission status', [ | ||
| * 'transmission_id' => $transmission->id, | ||
| * 'error' => $e->getMessage(), | ||
| * ]); | ||
| * } | ||
| * } | ||
| * | ||
| * $this->logPeppolInfo('Completed Peppol status polling', [ | ||
| * 'checked' => $transmissions->count(), | ||
| * ]); | ||
| * } | ||
| * |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1) Confirm no executable handle() currently exists.
rg -n -C2 '^\s*public function handle\(' Modules/Invoices/Jobs/Peppol/PeppolStatusPoller.php || true
# 2) Confirm malformed docblock marker.
rg -n -C2 '\* /' Modules/Invoices/Jobs/Peppol/PeppolStatusPoller.php
# 3) Confirm referenced status enum usage/import state.
rg -n -C2 'PeppolTransmissionStatus' Modules/Invoices/Jobs/Peppol/PeppolStatusPoller.phpRepository: InvoicePlane/InvoicePlane-v2
Length of output: 961
🏁 Script executed:
head -30 Modules/Invoices/Jobs/Peppol/PeppolStatusPoller.phpRepository: InvoicePlane/InvoicePlane-v2
Length of output: 957
handle() is commented out due to malformed PHPDoc, making the queued job non-functional.
Line 39 contains * / instead of */, which closes the docblock prematurely and comments out the entire handle() method (lines 40–66). The class implements ShouldQueue, so the queue worker expects an executable handle() method. Additionally, the commented code references PeppolTransmissionStatus but the import is missing from the top of the file.
Suggested fix
use Modules\Invoices\Events\Peppol\PeppolAcknowledgementReceived;
+use Modules\Invoices\Enums\PeppolTransmissionStatus;
use Modules\Invoices\Models\PeppolTransmission;
use Modules\Invoices\Peppol\Providers\ProviderFactory;
use Modules\Invoices\Traits\LogsPeppolActivity;
@@
/**
* Polls providers for status updates of recently sent Peppol transmissions and updates local records.
@@
* Logs the start and completion of the polling job and includes the number of transmissions checked.
- * /
- * public function handle(): void
- * {
- * $this->logPeppolInfo('Starting Peppol status polling job');
- *
- * // Get all transmissions awaiting acknowledgement
- * $transmissions = PeppolTransmission::query()->where('status', PeppolTransmissionStatus::SENT)
- * ->whereNotNull('external_id')
- * ->whereNull('acknowledged_at')
- * ->where('sent_at', '<', now()->subMinutes(5)) // Allow 5 min grace period
- * ->limit(100) // Process in batches
- * ->get();
- *
- * foreach ($transmissions as $transmission) {
- * try {
- * $this->checkStatus($transmission);
- * } catch (\Exception $e) {
- * $this->logPeppolError('Failed to check transmission status', [
- * 'transmission_id' => $transmission->id,
- * 'error' => $e->getMessage(),
- * ]);
- * }
- * }
- *
- * $this->logPeppolInfo('Completed Peppol status polling', [
- * 'checked' => $transmissions->count(),
- * ]);
- * }
- *
- * /**
+ */
+ public function handle(): void
+ {
+ $this->logPeppolInfo('Starting Peppol status polling job');
+
+ $transmissions = PeppolTransmission::query()
+ ->where('status', PeppolTransmissionStatus::SENT)
+ ->whereNotNull('external_id')
+ ->whereNull('acknowledged_at')
+ ->where('sent_at', '<', now()->subMinutes(5))
+ ->limit(100)
+ ->get();
+
+ foreach ($transmissions as $transmission) {
+ try {
+ $this->checkStatus($transmission);
+ } catch (\Exception $e) {
+ $this->logPeppolError('Failed to check transmission status', [
+ 'transmission_id' => $transmission->id,
+ 'error' => $e->getMessage(),
+ ]);
+ }
+ }
+
+ $this->logPeppolInfo('Completed Peppol status polling', [
+ 'checked' => $transmissions->count(),
+ ]);
+ }
+
+ /**
* Polls the external provider for a transmission's delivery status and updates the local record accordingly.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Polls providers for status updates of recently sent Peppol transmissions and updates local records. | |
| * | |
| * Retrieves up to 100 transmissions that are marked SENT, have an external ID, are not yet acknowledged, | |
| * and were sent more than five minutes ago. For each transmission it checks the provider status, updates | |
| * the transmission state accordingly, and logs per-transmission errors without aborting the batch. | |
| * | |
| * Logs the start and completion of the polling job and includes the number of transmissions checked. | |
| * / | |
| * public function handle(): void | |
| * { | |
| * $this->logPeppolInfo('Starting Peppol status polling job'); | |
| * | |
| * // Get all transmissions awaiting acknowledgement | |
| * $transmissions = PeppolTransmission::query()->where('status', PeppolTransmissionStatus::SENT) | |
| * ->whereNotNull('external_id') | |
| * ->whereNull('acknowledged_at') | |
| * ->where('sent_at', '<', now()->subMinutes(5)) // Allow 5 min grace period | |
| * ->limit(100) // Process in batches | |
| * ->get(); | |
| * | |
| * foreach ($transmissions as $transmission) { | |
| * try { | |
| * $this->checkStatus($transmission); | |
| * } catch (\Exception $e) { | |
| * $this->logPeppolError('Failed to check transmission status', [ | |
| * 'transmission_id' => $transmission->id, | |
| * 'error' => $e->getMessage(), | |
| * ]); | |
| * } | |
| * } | |
| * | |
| * $this->logPeppolInfo('Completed Peppol status polling', [ | |
| * 'checked' => $transmissions->count(), | |
| * ]); | |
| * } | |
| * | |
| /** | |
| * Polls providers for status updates of recently sent Peppol transmissions and updates local records. | |
| * | |
| * Retrieves up to 100 transmissions that are marked SENT, have an external ID, are not yet acknowledged, | |
| * and were sent more than five minutes ago. For each transmission it checks the provider status, updates | |
| * the transmission state accordingly, and logs per-transmission errors without aborting the batch. | |
| * | |
| * Logs the start and completion of the polling job and includes the number of transmissions checked. | |
| */ | |
| public function handle(): void | |
| { | |
| $this->logPeppolInfo('Starting Peppol status polling job'); | |
| $transmissions = PeppolTransmission::query() | |
| ->where('status', PeppolTransmissionStatus::SENT) | |
| ->whereNotNull('external_id') | |
| ->whereNull('acknowledged_at') | |
| ->where('sent_at', '<', now()->subMinutes(5)) | |
| ->limit(100) | |
| ->get(); | |
| foreach ($transmissions as $transmission) { | |
| try { | |
| $this->checkStatus($transmission); | |
| } catch (\Exception $e) { | |
| $this->logPeppolError('Failed to check transmission status', [ | |
| 'transmission_id' => $transmission->id, | |
| 'error' => $e->getMessage(), | |
| ]); | |
| } | |
| } | |
| $this->logPeppolInfo('Completed Peppol status polling', [ | |
| 'checked' => $transmissions->count(), | |
| ]); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Modules/Invoices/Jobs/Peppol/PeppolStatusPoller.php` around lines 31 - 67,
Fix the malformed PHPDoc closing tag in the handle() method by changing the `*
/` to `*/` on line 39 to properly close the docblock and uncomment the method
body. Additionally, add the missing import statement for
PeppolTransmissionStatus at the top of the PeppolStatusPoller.php file so that
the class reference used in the handle() method is available. This will make the
queued job functional again since the class implements ShouldQueue and the queue
worker expects an executable handle() method.
| { | ||
| $this->client = $client; | ||
| $this->apiKey = $apiKey; | ||
| $this->baseUrl = mb_rtrim($baseUrl, '/'); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify whether mb_ltrim/mb_rtrim are defined anywhere in the repository.
rg -n -C2 'function\s+mb_(ltrim|rtrim)\s*\('
# Show all call sites to confirm impact surface.
rg -n -C2 '\bmb_(ltrim|rtrim)\s*\('Repository: InvoicePlane/InvoicePlane-v2
Length of output: 54
🏁 Script executed:
# Check if file exists and read the relevant lines
find . -name "BasePeppolClient.php" -type f | head -5Repository: InvoicePlane/InvoicePlane-v2
Length of output: 127
🌐 Web query:
mb_ltrim mb_rtrim PHP functions standard library
💡 Result:
The mb_ltrim and mb_rtrim functions are part of the PHP standard library, specifically within the Multibyte String (mbstring) extension [1]. They were introduced in PHP 8.4.0 [2][3]. These functions provide multibyte-safe versions of the traditional ltrim and rtrim functions, allowing for the correct trimming of whitespace or specified characters from the beginning or end of strings that contain multibyte characters (such as UTF-8) [4][5][6]. Key details include: - Functionality: Like their standard counterparts, mb_ltrim strips characters from the beginning of a string, while mb_rtrim strips characters from the end [4][5]. - Parameters: Both functions accept the input string, an optional string of characters to strip, and an optional encoding parameter [4][5][6]. If no encoding is specified, the internal character encoding is used [4][5]. - Default Behavior: When the second parameter is omitted, these functions automatically strip a comprehensive set of Unicode whitespace characters, which is an improvement over the standard trim functions that only handle a more limited set of ASCII whitespace [7][6]. Before the introduction of these functions in PHP 8.4, developers typically had to use workarounds like regular expressions with the /u (Unicode) modifier to achieve similar behavior [8].
Citations:
- 1: https://www.php.net/manual/en/ref.mbstring.php
- 2: https://www.php.net/manual/en/function.mb-rtrim.php
- 3: https://lindevs.com/trim-multibyte-string-using-mb_trim-in-php-8-4
- 4: https://www.php.net/mb-ltrim
- 5: https://www.php.net/function.mb-rtrim.php
- 6: https://php.watch/versions/8.4/mb_trim-mb_ltrim-mb_rtrim
- 7: https://www.php.net/mb_trim
- 8: https://stackoverflow.com/questions/10066647/multibyte-trim-in-php
🏁 Script executed:
# Read the file to see lines 56 and 109
cat -n ./Modules/Invoices/Peppol/Clients/BasePeppolClient.php | sed -n '50,115p'Repository: InvoicePlane/InvoicePlane-v2
Length of output: 2529
🏁 Script executed:
# Check project's PHP version requirement in composer.json
cat composer.json | grep -A5 '"php"'Repository: InvoicePlane/InvoicePlane-v2
Length of output: 277
Replace mb_rtrim / mb_ltrim with standard rtrim / ltrim functions.
mb_rtrim() and mb_ltrim() are PHP 8.4+ functions, but the project supports PHP 8.2+. Using them causes a fatal error on PHP 8.2 and 8.3. Use the standard rtrim() and ltrim() functions instead.
Suggested change
- $this->baseUrl = mb_rtrim($baseUrl, '/');
+ $this->baseUrl = rtrim($baseUrl, '/');- return $this->baseUrl . '/' . mb_ltrim($path, '/');
+ return $this->baseUrl . '/' . ltrim($path, '/');Also applies to: 109-109
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Modules/Invoices/Peppol/Clients/BasePeppolClient.php` at line 56, The code
uses mb_rtrim and mb_ltrim functions which are only available in PHP 8.4+, but
the project supports PHP 8.2+, causing a fatal error on earlier versions.
Replace all occurrences of mb_rtrim with the standard rtrim function and
mb_ltrim with the standard ltrim function throughout the BasePeppolClient class.
This change appears at multiple locations in the file where these multibyte
versions are currently being used for string trimming operations.
| public function generateXml(Invoice $invoice, array $options = []): string | ||
| { | ||
| // Implement XML generation logic | ||
| return ''; | ||
| } |
There was a problem hiding this comment.
XML generation is unimplemented across handlers, blocking reliable transmission output. Both methods violate the generateXml() contract with placeholder payloads.
Modules/Invoices/Peppol/FormatHandlers/CiiHandler.php#L74-L78: implement real CII XML generation (or fail explicitly until implemented), never return empty string.Modules/Invoices/Peppol/FormatHandlers/EhfHandler.php#L89-L95: implement real EHF XML generation (or fail explicitly until implemented), never return JSON from an XML API.
📍 Affects 2 files
Modules/Invoices/Peppol/FormatHandlers/CiiHandler.php#L74-L78(this comment)Modules/Invoices/Peppol/FormatHandlers/EhfHandler.php#L89-L95
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Modules/Invoices/Peppol/FormatHandlers/CiiHandler.php` around lines 74 - 78,
The generateXml() methods in both handler classes are returning placeholder
values instead of proper XML content. In
Modules/Invoices/Peppol/FormatHandlers/CiiHandler.php at lines 74-78, replace
the empty string return with actual CII XML generation logic based on the
Invoice parameter, or throw an explicit exception if the feature is not yet
implemented. In Modules/Invoices/Peppol/FormatHandlers/EhfHandler.php at lines
89-95, replace the JSON return value with actual EHF XML generation logic that
produces valid XML content, or throw an explicit exception if not yet
implemented. Both methods must honor their generateXml() contract by returning
valid XML strings or failing explicitly, never returning empty strings or JSON
from an XML API method.
| public function transform(Invoice $invoice, string $format): array | ||
| { | ||
| return [ | ||
| 'invoice_type_code' => $this->getInvoiceTypeCode($invoice), | ||
| 'invoice_number' => $invoice->number, | ||
| 'issue_date' => $invoice->invoice_date->format('Y-m-d'), | ||
| 'due_date' => $invoice->due_date?->format('Y-m-d'), | ||
| 'currency_code' => config('invoices.peppol.currency_code', 'EUR'), | ||
|
|
||
| 'supplier' => $this->transformSupplier($invoice), | ||
| 'customer' => $this->transformCustomer($invoice), | ||
| 'invoice_lines' => $this->transformInvoiceLines($invoice), | ||
| 'tax_totals' => $this->transformTaxTotals($invoice), | ||
| 'monetary_totals' => $this->transformMonetaryTotals($invoice), | ||
| 'payment_terms' => $this->transformPaymentTerms($invoice), | ||
|
|
||
| // Metadata | ||
| 'format' => $format, | ||
| 'invoice_id' => $invoice->id, | ||
| ]; | ||
| } |
There was a problem hiding this comment.
Critical: transformSupplier() method is missing — runtime crash.
Line 32 calls $this->transformSupplier($invoice), but no such method exists in this class. The implementation appears to be accidentally embedded in a docblock (lines 75-87) rather than as executable code. This will throw Error: Call to undefined method when transform() is invoked.
🐛 Fix: Extract the method from the docblock
+ /**
+ * Build an array representing the supplier (company) information for Peppol output.
+ *
+ * `@param` Invoice $invoice the invoice used to source supplier data
+ *
+ * `@return` array Supplier structure with address fields mapped for Peppol.
+ */
+ protected function transformSupplier(Invoice $invoice): array
+ {
+ return [
+ 'name' => config('invoices.peppol.supplier.name', $invoice->company->name ?? ''),
+ 'vat_number' => config('invoices.peppol.supplier.vat'),
+ 'address' => [
+ 'street' => config('invoices.peppol.supplier.street'),
+ 'city' => config('invoices.peppol.supplier.city'),
+ 'postal_code' => config('invoices.peppol.supplier.postal'),
+ 'country_code' => config('invoices.peppol.supplier.country'),
+ ],
+ ];
+ }
+
/**
- * Build an array representing the supplier (company) information for Peppol output.
- *
- * `@param` Invoice $invoice the invoice used to source supplier data; company name will fall back to $invoice->company->name when not configured
- *
- * `@return` array{
- * name: string,
- * vat_number: null|string,
- * address: array{
- * street: null|string,
- * city: null|string,
- * postal_code: null|string,
- * country_code: null|string
- * }
- * } Supplier structure with address fields mapped for Peppol.
- * protected function transformSupplier(Invoice $invoice): array
- * {
- * return [
- * 'name' => config('invoices.peppol.supplier.name', $invoice->company->name ?? ''),
- * 'vat_number' => config('invoices.peppol.supplier.vat'),
- * 'address' => [
- * 'street' => config('invoices.peppol.supplier.street'),
- * 'city' => config('invoices.peppol.supplier.city'),
- * 'postal_code' => config('invoices.peppol.supplier.postal'),
- * 'country_code' => config('invoices.peppol.supplier.country'),
- * ],
- * ];
- * }
- *
- * `@param` Invoice $invoice the invoice containing the customer and address data to transform
+ * Transform customer data for Peppol output.
...📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public function transform(Invoice $invoice, string $format): array | |
| { | |
| return [ | |
| 'invoice_type_code' => $this->getInvoiceTypeCode($invoice), | |
| 'invoice_number' => $invoice->number, | |
| 'issue_date' => $invoice->invoice_date->format('Y-m-d'), | |
| 'due_date' => $invoice->due_date?->format('Y-m-d'), | |
| 'currency_code' => config('invoices.peppol.currency_code', 'EUR'), | |
| 'supplier' => $this->transformSupplier($invoice), | |
| 'customer' => $this->transformCustomer($invoice), | |
| 'invoice_lines' => $this->transformInvoiceLines($invoice), | |
| 'tax_totals' => $this->transformTaxTotals($invoice), | |
| 'monetary_totals' => $this->transformMonetaryTotals($invoice), | |
| 'payment_terms' => $this->transformPaymentTerms($invoice), | |
| // Metadata | |
| 'format' => $format, | |
| 'invoice_id' => $invoice->id, | |
| ]; | |
| } | |
| /** | |
| * Build an array representing the supplier (company) information for Peppol output. | |
| * | |
| * `@param` Invoice $invoice the invoice used to source supplier data | |
| * | |
| * `@return` array Supplier structure with address fields mapped for Peppol. | |
| */ | |
| protected function transformSupplier(Invoice $invoice): array | |
| { | |
| return [ | |
| 'name' => config('invoices.peppol.supplier.name', $invoice->company->name ?? ''), | |
| 'vat_number' => config('invoices.peppol.supplier.vat'), | |
| 'address' => [ | |
| 'street' => config('invoices.peppol.supplier.street'), | |
| 'city' => config('invoices.peppol.supplier.city'), | |
| 'postal_code' => config('invoices.peppol.supplier.postal'), | |
| 'country_code' => config('invoices.peppol.supplier.country'), | |
| ], | |
| ]; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Modules/Invoices/Peppol/Services/PeppolTransformerService.php` around lines
23 - 43, The transform() method calls transformSupplier() on line 32, but this
method does not exist as executable code in the PeppolTransformerService class —
it is currently embedded in a docblock at lines 75-87. Extract the
transformSupplier() implementation from the docblock and create it as a proper
method within the class with the correct visibility (likely private or
protected) and proper PHP syntax to ensure the transform() method can
successfully invoke it at runtime without throwing an undefined method error.
| 'invoice_number' => $invoice->number, | ||
| 'issue_date' => $invoice->invoice_date->format('Y-m-d'), | ||
| 'due_date' => $invoice->due_date?->format('Y-m-d'), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check Invoice model properties
ast-grep --pattern 'class Invoice {
$$$
}'
# Also check for property definitions
rg -n 'invoice_number|invoiced_at|number|invoice_date' --type php -g '**/Models/Invoice.php'Repository: InvoicePlane/InvoicePlane-v2
Length of output: 864
🏁 Script executed:
cat -n Modules/Invoices/Peppol/Services/PeppolTransformerService.php | sed -n '20,35p'Repository: InvoicePlane/InvoicePlane-v2
Length of output: 975
🏁 Script executed:
rg -n 'due_date' --type php -g '**/Models/Invoice.php'Repository: InvoicePlane/InvoicePlane-v2
Length of output: 54
🏁 Script executed:
cat -n Modules/Invoices/Models/Invoice.php | head -100Repository: InvoicePlane/InvoicePlane-v2
Length of output: 4523
Fix property name mismatches in PeppolTransformerService.
The service uses incorrect property names that do not exist in the Invoice model:
- Line 28:
$invoice->invoice_dateshould be$invoice->invoiced_at - Line 29:
$invoice->due_dateshould be$invoice->due_at
These will cause null/undefined access errors at runtime. The model defines these properties as invoiced_at (line 40) and due_at (line 42) respectively.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Modules/Invoices/Peppol/Services/PeppolTransformerService.php` around lines
27 - 29, The PeppolTransformerService is using incorrect property names that
don't match the Invoice model. In the array being constructed, replace the
property name `invoice_date` with `invoiced_at` and replace `due_date` with
`due_at` to correctly reference the Invoice model's properties. Both properties
use the nullable safe operator for formatting, so ensure the format calls are
preserved after updating the property names.
2 participants
Given the work already in PR #471, the remaining Sprint 5 deliverables are:
Summary by CodeRabbit
Release Notes
New Features
Addresses #126