diff --git a/Changes.md b/Changes.md index 39beef3..9b59c73 100644 --- a/Changes.md +++ b/Changes.md @@ -2,6 +2,14 @@ --- +## 2026-08-02 – Nummernkreise geben die zuletzt vergebene Nummer beim Löschen wieder frei + +- Der E2E-Finalisierungstest verbrauchte bei jedem Lauf eine Rechnungs- und eine Belegnummer: Die Testrechnung wurde zwar wieder gelöscht, die Nummern blieben aber vergeben und rissen dauerhaft Lücken in den laufenden Nummernkreis. +- **`src/Models/Invoice.php`, `src/Models/Beleg.php`:** `delete()` gibt die Nummer des gelöschten Datensatzes jetzt wieder frei. Die zuletzt vergebene Nummer wird dafür aus dem Zählerstand rekonstruiert und nur bei exakter Übereinstimmung zurückgezählt – existiert bereits eine jüngere Rechnung bzw. ein jüngerer Beleg (oder stammt die Nummer aus einem früheren Monat/Jahr), bleibt der Zähler stehen. Damit kann keine Nummer doppelt vergeben werden. `Invoice::delete()` gibt zusätzlich die Belegnummer des mitgelöschten Rechnungsbelegs frei. +- **`scripts/e2e-cleanup.php`:** Räumt liegengebliebene Testdaten absteigend ab (jüngste zuerst), damit die Freigabe für die ganze Kette greift und nicht nur für den letzten Datensatz. +- **`tests/78_NummernkreisFreigabeTest.php`** (neu): Sichert Freigabe und Wiedervergabe für beide Nummernkreise ab – inklusive der Gegenprobe, dass das Löschen eines älteren Datensatzes den Zähler unverändert lässt. +- **`tests/e2e/specs/rechnung-export.spec.js`:** Neuer Test `Löschen einer finalisierten Rechnung gibt die Nummer wieder frei` weist über zwei aufeinanderfolgende Finalisierungen nach, dass der Testlauf den Nummernkreis nicht mehr weiterschiebt. Der Hinweis auf die „nicht umkehrbare Nebenwirkung" in `tests/e2e/README.md` entfällt damit. + ## 2026-08-02 – E2E-Tests: Browser-Ebene abgedeckt, zwei kaputte Seiten repariert - Die PHP-Suite prüft Models und Services, ruft aber nie einen Controller über HTTP auf. Routing, Views und das JavaScript in den Formularen waren damit vollständig ungetestet – genau die Ebene, auf der die Fehler der letzten Wochen auftraten. Steuerkram ist jetzt an den generischen [e2e-runner](../e2e-runner/) angebunden; auf dem Host wird dafür nichts installiert. diff --git a/docs/funktionen.md b/docs/funktionen.md index bbe78be..d5898ce 100644 --- a/docs/funktionen.md +++ b/docs/funktionen.md @@ -52,6 +52,7 @@ CRUD für Kundenstammdaten. Kunden werden beim Erstellen von Ausgangsrechnungen - DIN 5008-konformes Layout, PDF-Export via mPDF - Rechnungsnummernformat **PPPYYYYMM-nn** (konfigurierbarer Präfix + Jahr + Monat + laufende Nummer, monatlicher Reset) +- **Nummernfreigabe beim Löschen**: Wird eine Rechnung gelöscht (im Testmodus) und trug sie die zuletzt vergebene Nummer, zählt der Nummernkreis zurück – die Nummer wird als nächste wieder vergeben, statt eine Lücke zu hinterlassen. Für die Belegnummer des zugehörigen Belegs gilt dasselbe. War die Nummer nicht die letzte, bleibt der Zähler stehen; eine Nummer wird nie doppelt vergeben. - Status-Workflow: Entwurf → Finalisiert (gesperrt für Bearbeitung) → Gesendet → Teilzahlung / Bezahlt / Überzahlt / Storniert - **Zahlungsstatus automatisch**: Sobald dem Beleg einer Rechnung Zahlungsbuchungen zugeordnet werden, ergibt sich der Status aus der Summe der aktiven Einnahme-Buchungen – weniger als der Rechnungsbetrag → `Teilzahlung`, exakt → `Bezahlt`, mehr → `Überzahlt`. Stornierte Buchungen zählen nicht mit; ohne aktive Buchung fällt die Rechnung auf `Gesendet` (falls bereits versendet, Zeitpunkt in `sent_at`) bzw. `Finalisiert` zurück. Entwürfe und stornierte Rechnungen bleiben unberührt. Der Button „Rechnung als bezahlt markieren" bleibt als manuelle Übersteuerung erhalten (z. B. bei Skonto), wird aber von der nächsten Buchungsänderung wieder überschrieben. - Das Zahlungs-Modal der Detailansicht und die Buchungsansicht zeigen Rechnungsbetrag, gezahlten Betrag und offenen Rest; der Betragsvorschlag entspricht dem offenen Restbetrag. diff --git a/scripts/e2e-cleanup.php b/scripts/e2e-cleanup.php index c35c507..d3a2cf4 100644 --- a/scripts/e2e-cleanup.php +++ b/scripts/e2e-cleanup.php @@ -39,13 +39,16 @@ if (!Setting::isTestMode()) { exit(1); } +// Absteigend (jüngste zuerst): Nur die zuletzt vergebene Nummer wandert beim +// Löschen in den Nummernkreis zurück – rückwärts abgeräumt greift das für die +// ganze Kette statt nur für den letzten Datensatz. $belege = Database::fetchAll( - 'SELECT id, beleg_nr, filename FROM belege WHERE notes LIKE ? OR filename LIKE ?', + 'SELECT id, beleg_nr, filename FROM belege WHERE notes LIKE ? OR filename LIKE ? ORDER BY id DESC', [MARKER, MARKER] ); $rechnungen = Database::fetchAll( - 'SELECT id, rechnungs_nr, betreff, status FROM invoices WHERE betreff LIKE ?', + 'SELECT id, rechnungs_nr, betreff, status FROM invoices WHERE betreff LIKE ? ORDER BY id DESC', [MARKER] ); diff --git a/src/Models/Beleg.php b/src/Models/Beleg.php index c0362d1..2cc2057 100644 --- a/src/Models/Beleg.php +++ b/src/Models/Beleg.php @@ -267,9 +267,40 @@ class Beleg } Database::execute('DELETE FROM belege WHERE id=?', [$id]); + self::releaseBelegNr((string)($beleg['beleg_nr'] ?? '')); return true; } + /** + * Gibt die Belegnummer eines gelöschten Belegs wieder frei, sofern sie die + * zuletzt vergebene war – sonst risse jeder gelöschte Beleg eine Lücke in + * den Nummernkreis. + * + * Die Nummer wird aus dem aktuellen Zählerstand rekonstruiert und nur bei + * exakter Übereinstimmung zurückgezählt. Existiert bereits ein jüngerer + * Beleg (oder stammt die Nummer aus einem früheren Jahr), passt die + * Rekonstruktion nicht und der Zähler bleibt stehen; eine Nummer kann + * dadurch nie doppelt vergeben werden. + */ + public static function releaseBelegNr(string $nr): void + { + if ($nr === '') return; + + $settings = Database::fetchOne('SELECT beleg_nr_prefix, beleg_nr_next FROM company_settings WHERE id=1'); + if (!$settings) return; + + $next = (int)($settings['beleg_nr_next'] ?? 1); + if ($next <= 1) return; + + $letzte = sprintf('%s-%s-%04d', $settings['beleg_nr_prefix'] ?? 'BLG', date('Y'), $next - 1); + if ($nr !== $letzte) return; + + Database::execute( + 'UPDATE company_settings SET beleg_nr_next=? WHERE id=1 AND beleg_nr_next=?', + [$next - 1, $next] + ); + } + public static function generateBelegNr(): string { $settings = Database::fetchOne('SELECT beleg_nr_prefix, beleg_nr_next FROM company_settings WHERE id=1'); diff --git a/src/Models/Invoice.php b/src/Models/Invoice.php index b801ebf..d33ce91 100644 --- a/src/Models/Invoice.php +++ b/src/Models/Invoice.php @@ -397,6 +397,7 @@ class Invoice if (!$invoice) return; $belegId = !empty($invoice['beleg_id']) ? (int)$invoice['beleg_id'] : null; + $beleg = $belegId !== null ? Beleg::find($belegId) : false; Database::execute('DELETE FROM invoice_lines WHERE invoice_id = ?', [$id]); Database::execute('DELETE FROM invoices WHERE id = ?', [$id]); @@ -405,6 +406,48 @@ class Invoice Database::execute('DELETE FROM bookings WHERE beleg_id = ?', [$belegId]); Database::execute('DELETE FROM belege WHERE id = ?', [$belegId]); } + + // Beide Nummernkreise zurückrollen, solange die gelöschten Datensätze + // die zuletzt vergebenen Nummern trugen – sonst bliebe eine Lücke. + self::releaseRechnungsNr((string)($invoice['rechnungs_nr'] ?? '')); + if ($beleg) { + Beleg::releaseBelegNr((string)($beleg['beleg_nr'] ?? '')); + } + } + + /** + * Gibt die Rechnungsnummer einer gelöschten Rechnung wieder frei, sofern sie + * die zuletzt vergebene war. + * + * Die Nummer wird aus dem aktuellen Zählerstand rekonstruiert; nur bei exakter + * Übereinstimmung wird zurückgezählt. Gibt es bereits eine jüngere Rechnung, + * passt die Rekonstruktion nicht und der Zähler bleibt stehen – eine Nummer + * kann so nie doppelt vergeben werden. + */ + private static function releaseRechnungsNr(string $nr): void + { + if ($nr === '') return; + + $settings = Database::fetchOne( + 'SELECT invoice_nr_prefix, invoice_nr_next, invoice_nr_year, invoice_nr_month FROM company_settings WHERE id=1' + ); + if (!$settings) return; + + $next = (int)($settings['invoice_nr_next'] ?? 1); + if ($next <= 1) return; + + $letzte = self::formatRechnungsNr( + (string)($settings['invoice_nr_prefix'] ?? 'AR'), + (int)($settings['invoice_nr_year'] ?? 0), + (int)($settings['invoice_nr_month'] ?? 0), + $next - 1 + ); + if ($nr !== $letzte) return; + + Database::execute( + 'UPDATE company_settings SET invoice_nr_next=? WHERE id=1 AND invoice_nr_next=?', + [$next - 1, $next] + ); } public static function generateRechnungsNr(): string @@ -423,8 +466,7 @@ class Invoice $next = 1; } - $digits = max(2, strlen((string)$next)); - $nr = sprintf('%s%04d%02d-%0' . $digits . 'd', $prefix, $year, $month, $next); + $nr = self::formatRechnungsNr($prefix, $year, $month, $next); Database::execute( 'UPDATE company_settings SET invoice_nr_next=?, invoice_nr_year=?, invoice_nr_month=? WHERE id=1', @@ -433,4 +475,11 @@ class Invoice return $nr; } + + /** Einheitliches Format des Nummernkreises: Präfix + Jahr + Monat + laufende Nummer. */ + private static function formatRechnungsNr(string $prefix, int $year, int $month, int $lfd): string + { + $digits = max(2, strlen((string)$lfd)); + return sprintf('%s%04d%02d-%0' . $digits . 'd', $prefix, $year, $month, $lfd); + } } diff --git a/tests/78_NummernkreisFreigabeTest.php b/tests/78_NummernkreisFreigabeTest.php new file mode 100644 index 0000000..b1574ad --- /dev/null +++ b/tests/78_NummernkreisFreigabeTest.php @@ -0,0 +1,167 @@ +rechnungsnummerWirdFreigegeben(); + $this->aeltereRechnungsnummerBleibtVerbraucht(); + $this->belegnummerWirdFreigegeben(); + } + + // ── Rechnungen ────────────────────────────────────────────────────────── + + private function rechnungsnummerWirdFreigegeben(): void + { + $vorher = $this->invoiceNrNext(); + + $invoiceId = $this->createRechnung('Nummernkreis-Freigabe'); + $nr = \Steuerkram\Models\Invoice::finalize($invoiceId); + $this->assertNotEmpty($nr, 'Finalisieren vergibt eine Rechnungsnummer'); + $this->assertEquals($vorher + 1, $this->invoiceNrNext(), 'Zähler steht nach dem Finalisieren eins weiter'); + + \Steuerkram\Models\Invoice::delete($invoiceId); + $this->assertEquals( + $vorher, + $this->invoiceNrNext(), + 'Nach dem Löschen steht der Zähler wieder auf dem Ausgangswert' + ); + + // Die freigegebene Nummer muss erneut vergeben werden – sonst entstünde + // trotz zurückgesetztem Zähler eine andere Nummer. + $zweiteId = $this->createRechnung('Nummernkreis-Wiedervergabe'); + $this->assertEquals($nr, \Steuerkram\Models\Invoice::finalize($zweiteId), 'Die freigegebene Nummer wird erneut vergeben'); + + \Steuerkram\Models\Invoice::delete($zweiteId); + } + + private function aeltereRechnungsnummerBleibtVerbraucht(): void + { + $ersteId = $this->createRechnung('Nummernkreis-Alt'); + \Steuerkram\Models\Invoice::finalize($ersteId); + + $zweiteId = $this->createRechnung('Nummernkreis-Neu'); + $zweiteNr = \Steuerkram\Models\Invoice::finalize($zweiteId); + $standMitBeiden = $this->invoiceNrNext(); + + // Die ältere der beiden Rechnungen löschen: ihre Nummer darf nicht + // freigegeben werden, sie liegt vor der bereits vergebenen $zweiteNr. + \Steuerkram\Models\Invoice::delete($ersteId); + $this->assertEquals( + $standMitBeiden, + $this->invoiceNrNext(), + 'Das Löschen einer älteren Rechnung lässt den Zähler unverändert' + ); + + // Jetzt die jüngste löschen – deren Nummer wird freigegeben. + \Steuerkram\Models\Invoice::delete($zweiteId); + $this->assertEquals( + $standMitBeiden - 1, + $this->invoiceNrNext(), + 'Das Löschen der jüngsten Rechnung gibt deren Nummer frei' + ); + + $dritteId = $this->createRechnung('Nummernkreis-Nachrueck'); + $this->assertEquals( + $zweiteNr, + \Steuerkram\Models\Invoice::finalize($dritteId), + 'Die freigegebene Nummer der jüngsten Rechnung rückt nach' + ); + \Steuerkram\Models\Invoice::delete($dritteId); + } + + // ── Belege ────────────────────────────────────────────────────────────── + + private function belegnummerWirdFreigegeben(): void + { + $vorher = $this->belegNrNext(); + + $ersterId = $this->createBeleg('Nummernkreis-Beleg-1'); + $zweiterId = $this->createBeleg('Nummernkreis-Beleg-2'); + $this->assertEquals($vorher + 2, $this->belegNrNext(), 'Zwei Belege verbrauchen zwei Nummern'); + + // Älterer Beleg: keine Freigabe. + $this->assert(\Steuerkram\Models\Beleg::delete($ersterId), 'Älterer Beleg ist löschbar'); + $this->assertEquals( + $vorher + 2, + $this->belegNrNext(), + 'Das Löschen eines älteren Belegs lässt den Zähler unverändert' + ); + + // Jüngster Beleg: Nummer wird frei. + $this->assert(\Steuerkram\Models\Beleg::delete($zweiterId), 'Jüngster Beleg ist löschbar'); + $this->assertEquals( + $vorher + 1, + $this->belegNrNext(), + 'Das Löschen des jüngsten Belegs gibt dessen Nummer frei' + ); + } + + // ── Hilfsfunktionen ───────────────────────────────────────────────────── + + private function invoiceNrNext(): int + { + $row = \Steuerkram\Database::fetchOne('SELECT invoice_nr_next FROM company_settings WHERE id=1'); + return (int)($row['invoice_nr_next'] ?? 0); + } + + private function belegNrNext(): int + { + $row = \Steuerkram\Database::fetchOne('SELECT beleg_nr_next FROM company_settings WHERE id=1'); + return (int)($row['beleg_nr_next'] ?? 0); + } + + private function createRechnung(string $marker): int + { + $customerId = \Steuerkram\Models\Customer::create([ + 'nachname' => 'Nummernkreis', + 'firma' => $marker . ' GmbH', + 'strasse' => 'Zählerweg 1', + 'plz' => '12345', + 'ort' => 'Nummernstadt', + 'land' => 'DE', + 'payment_days' => 14, + ]); + + return \Steuerkram\Models\Invoice::create([ + 'customer_id' => $customerId, + 'betreff' => $marker, + 'rechnungs_datum' => date('Y-m-d'), + 'lines' => [ + ['bezeichnung' => 'Leistung', 'menge' => 1, 'einheit' => 'Std.', 'einzelpreis' => 100.00, 'ust_satz' => 19.0], + ], + ]); + } + + /** Legt einen Beleg ohne Datei an (storage_path zeigt bewusst ins Leere). */ + private function createBeleg(string $marker): int + { + $belegNr = \Steuerkram\Models\Beleg::generateBelegNr(); + + return \Steuerkram\Models\Beleg::create([ + 'beleg_nr' => $belegNr, + 'filename' => $marker . '.pdf', + 'storage_path' => date('Y') . '/' . $marker . '/original.pdf', + 'mime_type' => 'application/pdf', + 'file_hash' => hash('sha256', $belegNr . $marker), + 'belegtyp' => 'Eingangsrechnung', + 'beleg_datum' => date('Y-m-d'), + 'notes' => $marker, + ]); + } +} diff --git a/tests/e2e/README.md b/tests/e2e/README.md index 6596f9f..d4397a5 100644 --- a/tests/e2e/README.md +++ b/tests/e2e/README.md @@ -81,16 +81,23 @@ docker compose exec web php scripts/e2e-cleanup.php # nur anzeigen docker compose exec web php scripts/e2e-cleanup.php --force # löschen ``` -### Nicht umkehrbare Nebenwirkung +### Nummernkreise -`rechnung-export.spec.js` finalisiert eine Rechnung und **verbraucht dabei eine -Nummer aus dem laufenden Nummernkreis**. Die Rechnung selbst wird wieder gelöscht, -die Nummer bleibt vergeben (GoBD — eine Rücknahme wäre auch nicht wünschenswert). -Wer das vermeiden will, lässt diesen Test weg: +`rechnung-export.spec.js` finalisiert eine Rechnung und vergibt dabei eine +Rechnungs- und eine Belegnummer. Beide gehen beim Aufräumen wieder zurück: +`Invoice::delete()` und `Beleg::delete()` zählen den jeweiligen Nummernkreis +zurück, wenn der gelöschte Datensatz die **zuletzt vergebene** Nummer trug. +Nach einem vollständigen Testlauf stehen die Zähler also wieder auf ihrem +Ausgangswert; der Test `Löschen einer finalisierten Rechnung gibt die Nummer +wieder frei` weist das nach. -```bash -... npx playwright test --grep-invert "Finalisieren" -``` +War die Nummer nicht die letzte (z.B. weil parallel eine echte Rechnung +finalisiert wurde), bleibt der Zähler stehen — eine Nummer wird nie zweimal +vergeben. In dem Fall entsteht wie bisher eine Lücke im Nummernkreis. + +Bricht ein Lauf mittendrin ab, bleiben Testdatensätze liegen; +`scripts/e2e-cleanup.php --force` räumt sie nachträglich ab und arbeitet dabei +absteigend, damit möglichst viele Nummern zurückwandern. ## Aufbau diff --git a/tests/e2e/specs/rechnung-export.spec.js b/tests/e2e/specs/rechnung-export.spec.js index 211d8ff..2066c29 100644 --- a/tests/e2e/specs/rechnung-export.spec.js +++ b/tests/e2e/specs/rechnung-export.spec.js @@ -6,9 +6,10 @@ // (BT-48), die im XML fehlte. Beide Male hing der Fehler am Zusammenspiel aus // Controller, ZugferdService und den tatsächlich gespeicherten Rechnungsdaten. // -// Hinweis: Der Finalisierungs-Test verbraucht eine Rechnungsnummer aus dem -// laufenden Nummernkreis. Das lässt sich nicht zurücknehmen (GoBD) — die -// Rechnung selbst wird im Testmodus wieder gelöscht. Siehe README.md. +// Hinweis: Der Finalisierungs-Test vergibt eine Rechnungsnummer, gibt sie beim +// Löschen der Testrechnung aber wieder frei (Invoice::delete). Der Nummernkreis +// steht nach dem Testlauf also wieder da, wo er vorher stand — der letzte Test +// dieser Datei weist genau das nach. Siehe README.md. const { test, expect } = require('@playwright/test') const { uniqueTag, gotoOk, acceptDialogs, cleanupInvoice, parseMoney } = require('../helpers/app') @@ -46,6 +47,29 @@ async function entwurfAnlegen(page, { tag, einzelpreis = '100.00', ustSatz = '19 return id } +/** Finalisiert den Entwurf über den Button in der Detailansicht. */ +async function finalisieren(page, invoiceId) { + acceptDialogs(page) + await page.click('button[type="submit"]:has-text("Finalisieren")') + await page.waitForLoadState('networkidle') + await expect(page.locator('body'), 'Finalisieren hat den Status nicht geändert') + .not.toContainText('Finalisieren & Nummerieren') +} + +/** + * Liest die vergebene Rechnungsnummer aus dem Dateinamen des XML-Downloads. + * Sie steht nirgends sonst maschinell eindeutig auf der Seite. + */ +async function rechnungsnummerAusXml(page, invoiceId) { + const response = await page.request.get(`/invoices/${invoiceId}/xml`) + expect(response.status()).toBe(200) + const dateiname = response.headers()['content-disposition'] || '' + const nummer = dateiname.match(/Rechnung-(.+)\.xml/)?.[1] + expect(nummer, `Rechnungsnummer nicht aus Dateiname ermittelbar: ${dateiname}`).toBeTruthy() + expect(nummer).not.toBe('Entwurf') + return nummer +} + test.describe('Rechnung: Anlegen, Finalisieren, Export', () => { let invoiceId = 0 @@ -85,13 +109,9 @@ test.describe('Rechnung: Anlegen, Finalisieren, Export', () => { test('Finalisieren vergibt eine Nummer und liefert PDF und XML aus', async ({ page }) => { const tag = uniqueTag() invoiceId = await entwurfAnlegen(page, { tag }) - acceptDialogs(page) - await page.click('button[type="submit"]:has-text("Finalisieren")') - await page.waitForLoadState('networkidle') + await finalisieren(page, invoiceId) - // Status darf nicht mehr "Entwurf" sein und es muss eine Nummer geben. - await expect(page.locator('body')).not.toContainText('Finalisieren & Nummerieren') const seitentext = await page.locator('body').textContent() expect(seitentext).toContain(tag) @@ -109,13 +129,33 @@ test.describe('Rechnung: Anlegen, Finalisieren, Export', () => { expect(xml).toContain('CrossIndustryInvoice') // Die vergebene Rechnungsnummer muss im XML stehen (BT-1). - const dateiname = xmlResponse.headers()['content-disposition'] || '' - const nummer = dateiname.match(/Rechnung-(.+)\.xml/)?.[1] - expect(nummer, `Rechnungsnummer nicht aus Dateiname ermittelbar: ${dateiname}`).toBeTruthy() - expect(nummer).not.toBe('Entwurf') + const nummer = await rechnungsnummerAusXml(page, invoiceId) expect(xml).toContain(nummer) }) + test('Löschen einer finalisierten Rechnung gibt die Nummer wieder frei', async ({ page }) => { + // Der Testlauf darf keine Nummer aus dem laufenden Kreis verbrauchen: + // Invoice::delete() zählt den Nummernkreis zurück, wenn die gelöschte + // Rechnung die zuletzt nummerierte war. Nachweis über die Nummer der + // nächsten Rechnung — sie muss dieselbe sein. + const ersteId = await entwurfAnlegen(page, { tag: uniqueTag() }) + await finalisieren(page, ersteId) + const ersteNummer = await rechnungsnummerAusXml(page, ersteId) + + await cleanupInvoice(page, ersteId) + await expect( + page.locator(`form[action="/invoices/${ersteId}/delete"]`), + 'Die finalisierte Testrechnung wurde nicht gelöscht' + ).toHaveCount(0) + + invoiceId = await entwurfAnlegen(page, { tag: uniqueTag() }) + await finalisieren(page, invoiceId) + expect( + await rechnungsnummerAusXml(page, invoiceId), + 'Der Nummernkreis wurde beim Löschen nicht zurückgesetzt' + ).toBe(ersteNummer) + }) + test('PDF eines Entwurfs wird nicht ausgeliefert', async ({ page }) => { const tag = uniqueTag() invoiceId = await entwurfAnlegen(page, { tag })