From ab4b0311d490b64e9318e7311abfba4734f254e8 Mon Sep 17 00:00:00 2001 From: nexxo Date: Wed, 29 Jul 2026 02:32:08 +0200 Subject: [PATCH] Copy every invoice to the archive when it is issued, and notice when that fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The application knows nothing about NFS and should not. It writes to a directory; whether that is a local disk, a bind mount or a NAS over NFS 4.1 belongs to the mount. No protocol library, the target changes without code changing with it, and the whole thing is testable against a temp directory. The finding that shaped it: mkdir -p on a path whose mount is NOT there succeeds. It creates the empty directory beneath the mountpoint, writes the invoice onto the container's own disk and reports success — so an unmounted NAS would quietly collect invoices locally while every check said it worked. An archive that silently is not the archive is worse than none. The root is therefore never created, only used: a missing root means a missing mount, and that is a failure. Only the year subdirectory is created, inside a root already proven to exist. Written to a .part and renamed. A rename within one filesystem is atomic, so whatever watches that folder never sees a truncated PDF and takes it for a finished invoice. The size is read back afterwards rather than the write being trusted: over a network mount a short write can report success, and an archive of truncated files looks exactly like one that worked. Queued, not done during the request. A network mount can block for its whole timeout, and an invoice must not fail to be issued because a NAS is rebooting. Dispatched after the commit, or a worker could look for an invoice that has not landed yet. And copy-on-issue is not enough on its own: an invoice issued while the office connection was down never arrives, the queue eventually gives up, and nobody opens an archive to check whether last Tuesday is in it. An hourly sweep re-queues whatever is still missing, the failure is recorded on the invoice rather than only in a log, and the Finance page says how many are outstanding. Co-Authored-By: Claude Opus 5 --- .../Commands/ArchiveUnexportedInvoices.php | 51 ++++++ app/Jobs/ArchiveInvoice.php | 62 ++++++++ app/Livewire/Admin/Finance.php | 19 +++ app/Models/Invoice.php | 1 + app/Services/Billing/InvoiceArchive.php | 111 +++++++++++++ app/Services/Billing/IssueInvoice.php | 16 +- ...29_140000_add_export_state_to_invoices.php | 35 +++++ lang/de/finance.php | 6 + lang/en/finance.php | 6 + .../views/livewire/admin/finance.blade.php | 19 +++ routes/console.php | 11 ++ tests/Feature/Billing/InvoiceArchiveTest.php | 146 ++++++++++++++++++ 12 files changed, 482 insertions(+), 1 deletion(-) create mode 100644 app/Console/Commands/ArchiveUnexportedInvoices.php create mode 100644 app/Jobs/ArchiveInvoice.php create mode 100644 app/Services/Billing/InvoiceArchive.php create mode 100644 database/migrations/2026_07_29_140000_add_export_state_to_invoices.php create mode 100644 tests/Feature/Billing/InvoiceArchiveTest.php diff --git a/app/Console/Commands/ArchiveUnexportedInvoices.php b/app/Console/Commands/ArchiveUnexportedInvoices.php new file mode 100644 index 0000000..f5e56c4 --- /dev/null +++ b/app/Console/Commands/ArchiveUnexportedInvoices.php @@ -0,0 +1,51 @@ +line('No archive path configured — nothing to do.'); + + return self::SUCCESS; + } + + $pending = Invoice::query() + ->whereNull('exported_at') + ->orderBy('id') + ->limit((int) $this->option('limit')) + ->get(); + + foreach ($pending as $invoice) { + ArchiveInvoice::dispatch($invoice); + } + + $this->line("Queued {$pending->count()} invoice(s) for the archive."); + + return self::SUCCESS; + } +} diff --git a/app/Jobs/ArchiveInvoice.php b/app/Jobs/ArchiveInvoice.php new file mode 100644 index 0000000..94e66a4 --- /dev/null +++ b/app/Jobs/ArchiveInvoice.php @@ -0,0 +1,62 @@ +invoice->exported_at !== null) { + return; + } + + $archive->store($this->invoice); + + $this->invoice->forceFill([ + 'exported_at' => now(), + 'export_error' => null, + ])->save(); + } + + public function failed(?Throwable $e): void + { + // On the invoice, not only in the log. The console shows it there, and + // a log line nobody reads is not a report. + $this->invoice->forceFill([ + 'export_error' => mb_substr((string) $e?->getMessage(), 0, 250), + ])->save(); + } +} diff --git a/app/Livewire/Admin/Finance.php b/app/Livewire/Admin/Finance.php index 3d71dcc..355aa8a 100644 --- a/app/Livewire/Admin/Finance.php +++ b/app/Livewire/Admin/Finance.php @@ -2,7 +2,9 @@ namespace App\Livewire\Admin; +use App\Models\Invoice; use App\Models\InvoiceSeries; +use App\Services\Billing\InvoiceArchive; use App\Support\CompanyProfile; use App\Support\Settings; use Illuminate\Support\Facades\Storage; @@ -32,6 +34,14 @@ class Finance extends Component public float $taxRate = 20.0; + /** + * Where a copy of every invoice is written, or empty for none. + * + * A plain directory. Whether it is a local disk, a bind mount or a NAS over + * NFS 4.1 is the mount's business — nothing here knows or needs to. + */ + public string $archivePath = ''; + /** New logo upload, validated on save. */ public $logo = null; @@ -44,6 +54,7 @@ class Finance extends Component $this->company = CompanyProfile::all(); $this->logoPath = (string) ($this->company['logo_path'] ?? ''); $this->taxRate = CompanyProfile::taxRate(); + $this->archivePath = InvoiceArchive::path(); } public function saveCompany(): void @@ -72,6 +83,7 @@ class Finance extends Component // ever rendered from then on. 'logo' => 'nullable|image|mimes:png,webp|max:1024', 'taxRate' => 'required|numeric|min:0|max:100', + 'archivePath' => 'nullable|string|max:255', ]); if ($this->logo !== null) { @@ -92,6 +104,7 @@ class Finance extends Component CompanyProfile::put($data['company']); Settings::set('company.tax_rate', (float) $data['taxRate']); + Settings::set(InvoiceArchive::PATH_KEY, trim((string) ($data['archivePath'] ?? ''))); $this->dispatch('notify', message: __('finance.company_saved')); } @@ -119,6 +132,12 @@ class Finance extends Component // invoice without a name, an address or a VAT number is not a valid // invoice here, and issuing one is worse than refusing to. 'missing' => CompanyProfile::missingForInvoicing(), + // Written from here rather than checked on every render: a stat() + // on a network mount that has gone away blocks for the mount's + // timeout, and this is a page an operator opens when something is + // already wrong. + 'unexported' => Invoice::query()->whereNull('exported_at')->count(), + 'exportFailures' => Invoice::query()->whereNotNull('export_error')->count(), ]); } } diff --git a/app/Models/Invoice.php b/app/Models/Invoice.php index 98a6e27..1cf409a 100644 --- a/app/Models/Invoice.php +++ b/app/Models/Invoice.php @@ -32,6 +32,7 @@ class Invoice extends Model 'issued_on' => 'date', 'due_on' => 'date', 'sent_at' => 'datetime', + 'exported_at' => 'datetime', 'net_cents' => 'integer', 'tax_cents' => 'integer', 'gross_cents' => 'integer', diff --git a/app/Services/Billing/InvoiceArchive.php b/app/Services/Billing/InvoiceArchive.php new file mode 100644 index 0000000..a68b5e6 --- /dev/null +++ b/app/Services/Billing/InvoiceArchive.php @@ -0,0 +1,111 @@ +issued_on?->format('Y') ?? 'unbekannt'); + + if (! is_dir($directory) && ! @mkdir($directory, 0o775, true) && ! is_dir($directory)) { + throw new RuntimeException("Cannot create the archive directory: {$directory}"); + } + + if (! is_writable($directory)) { + // Said plainly. On a mount that has gone away this is the first + // thing that fails, and "permission denied" on a path nobody + // recognises is a worse message than the path itself. + throw new RuntimeException("The archive directory is not writable: {$directory}"); + } + + $target = $directory.'/'.$invoice->number.'.pdf'; + $pdf = $this->renderer->forInvoice($invoice); + + // Same directory, so the rename below stays within one filesystem and + // is therefore atomic. A temp file in /tmp would make it a copy. + $temporary = $target.'.part'; + + if (@file_put_contents($temporary, $pdf) === false) { + throw new RuntimeException("Could not write {$temporary}"); + } + + if (! @rename($temporary, $target)) { + @unlink($temporary); + + throw new RuntimeException("Could not move the finished file into place: {$target}"); + } + + // Read back the size rather than trusting the write. Over a network + // mount a short write can report success, and an archive of truncated + // files is worse than an empty one: it looks like it worked. + clearstatcache(true, $target); + $written = @filesize($target); + + if ($written === false || $written < 1000) { + throw new RuntimeException("The archived file is too small to be an invoice: {$target}"); + } + + return $target; + } +} diff --git a/app/Services/Billing/IssueInvoice.php b/app/Services/Billing/IssueInvoice.php index b1afe13..88d5e58 100644 --- a/app/Services/Billing/IssueInvoice.php +++ b/app/Services/Billing/IssueInvoice.php @@ -2,6 +2,7 @@ namespace App\Services\Billing; +use App\Jobs\ArchiveInvoice; use App\Models\Customer; use App\Models\Invoice; use App\Models\InvoiceSeries; @@ -81,7 +82,7 @@ final class IssueInvoice 'meta' => $this->meta($customer, $treatment), ]; - return DB::transaction(function () use ($series, $customer, $orders, $snapshot, $totals) { + $invoice = DB::transaction(function () use ($series, $customer, $orders, $snapshot, $totals) { [$number, $sequence, $year] = $this->numbers->next($series); // The number is only true once it is on the document, so both are @@ -104,6 +105,19 @@ final class IssueInvoice 'currency' => strtoupper((string) ($orders->first()->currency ?: 'EUR')), ]); }); + + // After the commit, never inside it: a worker can pick a job up before + // the transaction lands, and it would then look for an invoice that + // does not exist yet. + // + // Dispatched rather than done here because the archive is a network + // mount: it can block for as long as its timeout allows, and an invoice + // must not fail to be issued because a NAS in an office is rebooting. + if (InvoiceArchive::enabled()) { + ArchiveInvoice::dispatch($invoice); + } + + return $invoice; } /** diff --git a/database/migrations/2026_07_29_140000_add_export_state_to_invoices.php b/database/migrations/2026_07_29_140000_add_export_state_to_invoices.php new file mode 100644 index 0000000..db13089 --- /dev/null +++ b/database/migrations/2026_07_29_140000_add_export_state_to_invoices.php @@ -0,0 +1,35 @@ +timestamp('exported_at')->nullable()->after('sent_at'); + $table->string('export_error')->nullable()->after('exported_at'); + // The sweep asks for "issued but not exported" on every tick. + $table->index('exported_at'); + }); + } + + public function down(): void + { + Schema::table('invoices', function (Blueprint $table) { + $table->dropIndex(['exported_at']); + $table->dropColumn(['exported_at', 'export_error']); + }); + } +}; diff --git a/lang/de/finance.php b/lang/de/finance.php index 9525323..9950116 100644 --- a/lang/de/finance.php +++ b/lang/de/finance.php @@ -31,13 +31,19 @@ return [ 'bank_name' => 'Bank', 'iban' => 'IBAN', 'bic' => 'BIC', + 'archive_path' => 'Archivpfad', 'logo' => 'Logo', 'tax_rate' => 'Umsatzsteuer (%)', 'payment_days' => 'Zahlungsziel (Tage)', 'payment_terms' => 'Zahlungsbedingungen', ], + 'archive_failed' => ':n Rechnung(en) konnten nicht ins Archiv geschrieben werden.', + 'archive_pending' => ':n Rechnung(en) warten noch auf das Archiv.', + 'archive_clear' => 'Jede Rechnung ist im Archiv.', + 'hint' => [ + 'archive_path' => 'Verzeichnis, in das eine Kopie jeder Rechnung geschrieben wird — z. B. der Einhängepunkt Ihrer NAS. Leer lassen, um nichts zu kopieren.', 'vat_id' => 'Pflichtangabe auf jeder Rechnung.', 'tax_rate' => 'Wird in jede Rechnung eingefroren. Eine Änderung wirkt nur auf neue Rechnungen.', 'logo' => 'PNG oder WEBP, höchstens 1 MB. Wird in jede erzeugte Rechnung eingebettet.', diff --git a/lang/en/finance.php b/lang/en/finance.php index cf29cf5..ebc193a 100644 --- a/lang/en/finance.php +++ b/lang/en/finance.php @@ -31,13 +31,19 @@ return [ 'bank_name' => 'Bank', 'iban' => 'IBAN', 'bic' => 'BIC', + 'archive_path' => 'Archive path', 'logo' => 'Logo', 'tax_rate' => 'VAT (%)', 'payment_days' => 'Payment terms (days)', 'payment_terms' => 'Payment conditions', ], + 'archive_failed' => ':n invoice(s) could not be written to the archive.', + 'archive_pending' => ':n invoice(s) are still waiting for the archive.', + 'archive_clear' => 'Every invoice is on the archive.', + 'hint' => [ + 'archive_path' => 'Directory a copy of every invoice is written to — your NAS mount point, for instance. Leave empty to copy nothing.', 'vat_id' => 'Required on every invoice.', 'tax_rate' => 'Frozen into every invoice. A change affects new invoices only.', 'logo' => 'PNG or WEBP, 1 MB at most. Embedded into every invoice rendered.', diff --git a/resources/views/livewire/admin/finance.blade.php b/resources/views/livewire/admin/finance.blade.php index 9615b83..81e7b90 100644 --- a/resources/views/livewire/admin/finance.blade.php +++ b/resources/views/livewire/admin/finance.blade.php @@ -82,6 +82,25 @@ + {{-- The archive. A directory, nothing more: the mount behind it is the + operator's business, and keeping it that way means the target can + change without a line of code changing with it. --}} +
+ + @if ($archivePath !== '') +

+ @if ($exportFailures > 0) + {{ __('finance.archive_failed', ['n' => $exportFailures]) }} + @elseif ($unexported > 0) + {{ __('finance.archive_pending', ['n' => $unexported]) }} + @else + {{ __('finance.archive_clear') }} + @endif +

+ @endif +
+
{{ __('finance.save') }}
diff --git a/routes/console.php b/routes/console.php index 1ee7ae2..09d5968 100644 --- a/routes/console.php +++ b/routes/console.php @@ -45,3 +45,14 @@ Schedule::call(fn () => \App\Models\StripePendingEvent::query() ->delete()) ->daily() ->name('stripe-pending-prune'); + +// Copies of invoices that never reached the archive — a NAS that was rebooting +// when the invoice was issued, a mount that had gone away. Copy-on-issue is +// right and is not enough: nobody opens an archive to check whether last +// Tuesday is in it, so something has to go and look. +// +// Hourly rather than by the minute: the failure it repairs lasts as long as an +// outage lasts, and a retry storm against a mount that is down helps nobody. +Schedule::command('clupilot:archive-invoices') + ->hourly() + ->withoutOverlapping(); diff --git a/tests/Feature/Billing/InvoiceArchiveTest.php b/tests/Feature/Billing/InvoiceArchiveTest.php new file mode 100644 index 0000000..5ae5437 --- /dev/null +++ b/tests/Feature/Billing/InvoiceArchiveTest.php @@ -0,0 +1,146 @@ + 'CluPilot Cloud e.U.', 'address' => 'Dreherstraße 66/1/8', + 'postcode' => '1110', 'city' => 'Wien', 'vat_id' => 'ATU00000000', + ]); + + $this->archive = sys_get_temp_dir().'/clupilot-archive-'.bin2hex(random_bytes(6)); + mkdir($this->archive, 0o775, true); + Settings::set(InvoiceArchive::PATH_KEY, $this->archive); +}); + +afterEach(function () { + if (isset($this->archive) && is_dir($this->archive)) { + exec('rm -rf '.escapeshellarg($this->archive)); + } +}); + +function anInvoice(): Invoice +{ + $customer = Customer::factory()->create(['name' => 'Muster GmbH']); + + return app(IssueInvoice::class)->forOrders($customer, collect([ + Order::factory()->create(['customer_id' => $customer->id, 'amount_cents' => 1900, 'currency' => 'EUR', 'status' => 'paid']), + ])); +} + +it('queues a copy the moment an invoice is issued', function () { + Queue::fake(); + + anInvoice(); + + Queue::assertPushed(ArchiveInvoice::class); +}); + +it('queues nothing when no archive is configured', function () { + Queue::fake(); + Settings::set(InvoiceArchive::PATH_KEY, ''); + + anInvoice(); + + Queue::assertNotPushed(ArchiveInvoice::class); +}); + +it('writes the invoice under its number, grouped by year', function () { + // Seven years of invoices in one directory is a directory nobody opens + // twice. + $invoice = anInvoice(); + + (new ArchiveInvoice($invoice))->handle(app(InvoiceArchive::class)); + + $expected = $this->archive.'/'.now()->format('Y').'/'.$invoice->number.'.pdf'; + + expect(is_file($expected))->toBeTrue() + ->and(substr(file_get_contents($expected), 0, 5))->toBe('%PDF-') + ->and($invoice->refresh()->exported_at)->not->toBeNull(); +}); + +it('leaves no half-written file behind under the final name', function () { + // Written to a .part and renamed. A rename inside one filesystem is atomic, + // so whatever watches that folder never sees a truncated PDF and mistakes + // it for a finished invoice. + $invoice = anInvoice(); + + (new ArchiveInvoice($invoice))->handle(app(InvoiceArchive::class)); + + $year = $this->archive.'/'.now()->format('Y'); + + expect(glob($year.'/*.part'))->toBe([]); +}); + +it('does not write the same invoice twice', function () { + // A retry after a timeout that actually succeeded must not do it again. + $invoice = anInvoice(); + + (new ArchiveInvoice($invoice))->handle(app(InvoiceArchive::class)); + $first = $invoice->refresh()->exported_at; + + (new ArchiveInvoice($invoice))->handle(app(InvoiceArchive::class)); + + expect($invoice->refresh()->exported_at->toIso8601String())->toBe($first->toIso8601String()); +}); + +it('refuses to create the archive root, so an unmounted NAS is a failure and not a hole', function () { + // The finding that made this test necessary. mkdir -p on a path whose mount + // is not there SUCCEEDS: it creates the empty directory underneath the + // mountpoint, writes the invoice onto the container's own disk, and reports + // success. An archive that silently is not the archive is worse than none, + // because every check says it worked. + // Faked, or the dispatch-on-issue archives it synchronously into the + // working directory from beforeEach() before this test has said anything. + Queue::fake(); + + $invoice = anInvoice(); + Settings::set(InvoiceArchive::PATH_KEY, sys_get_temp_dir().'/clupilot-not-mounted-'.bin2hex(random_bytes(4))); + + expect(fn () => app(InvoiceArchive::class)->store($invoice)) + ->toThrow(RuntimeException::class); + + expect($invoice->refresh()->exported_at)->toBeNull() + // And nothing was left on disk pretending to be an archive. + ->and(is_dir(InvoiceArchive::path()))->toBeFalse(); +}); + +it('records why it gave up, on the invoice rather than only in a log', function () { + // Nobody opens an archive to check whether last Tuesday is in it, so the + // failure has to be somewhere a person will pass. + $invoice = anInvoice(); + + (new ArchiveInvoice($invoice))->failed(new RuntimeException('mount is gone')); + + expect($invoice->refresh()->export_error)->toContain('mount is gone'); +}); + +it('sweeps up whatever the queue gave up on', function () { + Queue::fake(); + + $invoice = anInvoice(); + Queue::assertPushed(ArchiveInvoice::class); + + // The office connection was down; nothing reached the archive. + expect($invoice->exported_at)->toBeNull(); + + $this->artisan('clupilot:archive-invoices')->assertExitCode(0); + + Queue::assertPushed(ArchiveInvoice::class, 2); +});