Summary
The branch ships two parallel, disconnected send architectures: the older SendInvoiceToPeppolAction/PeppolManagementService path (well-tested, 23 tests, but not wired to SendInvoiceToPeppolJob) and the newer ProviderFactory/multi-provider path (actually wired to the job and the new admin UI, but almost untested and broken — see the payload-mismatch and hollow-tests issues). Resolve the duplication rather than continuing to test/maintain both.
Related dead code to remove/resolve as part of the same cleanup
PeppolTransformerService — dead code that would fatal if ever called; nothing tests it.
- Duplicated PDF-rendering block between
QontoProvider and SuperPdpProvider — extract into a BaseProvider helper.
QontoClient::authenticate(array $credentials) (a separate, currently-unused local method — BaseProvider::authenticate() is what's actually wired up) has an operator-precedence bug: return ! empty($credentials['access_token'] || $credentials['api_key']); evaluates the || before empty(). Dead code today, but worth fixing or removing while in this area.
Fix shape
Pick one architecture (the ProviderFactory/providers path is the one actually reachable from the new admin UI and job, so it's the natural survivor), migrate or delete the old SendInvoiceToPeppolAction/PeppolService/PeppolTransformerService path, and port forward whatever real test value its 23 tests have onto the surviving path.
Source
Found by a senior-laravel-developer-code-reviewer review of feature/126-implement-peppol vs develop on 2026-09-12 (Suggestions section).
Summary
The branch ships two parallel, disconnected send architectures: the older
SendInvoiceToPeppolAction/PeppolManagementServicepath (well-tested, 23 tests, but not wired toSendInvoiceToPeppolJob) and the newerProviderFactory/multi-provider path (actually wired to the job and the new admin UI, but almost untested and broken — see the payload-mismatch and hollow-tests issues). Resolve the duplication rather than continuing to test/maintain both.Related dead code to remove/resolve as part of the same cleanup
PeppolTransformerService— dead code that would fatal if ever called; nothing tests it.QontoProviderandSuperPdpProvider— extract into aBaseProviderhelper.QontoClient::authenticate(array $credentials)(a separate, currently-unused local method —BaseProvider::authenticate()is what's actually wired up) has an operator-precedence bug:return ! empty($credentials['access_token'] || $credentials['api_key']);evaluates the||beforeempty(). Dead code today, but worth fixing or removing while in this area.Fix shape
Pick one architecture (the
ProviderFactory/providers path is the one actually reachable from the new admin UI and job, so it's the natural survivor), migrate or delete the oldSendInvoiceToPeppolAction/PeppolService/PeppolTransformerServicepath, and port forward whatever real test value its 23 tests have onto the surviving path.Source
Found by a
senior-laravel-developer-code-reviewerreview offeature/126-implement-peppolvsdevelopon 2026-09-12 (Suggestions section).