diff --git a/app/Http/Controllers/InvoiceDeliveryController.php b/app/Http/Controllers/InvoiceDeliveryController.php index 040df0b4..1e334a7c 100644 --- a/app/Http/Controllers/InvoiceDeliveryController.php +++ b/app/Http/Controllers/InvoiceDeliveryController.php @@ -68,9 +68,11 @@ public function store(Request $request, Invoice $invoice): RedirectResponse ); if ($delivery->status === 'queued') { - $invoice->forceFill([ - 'delivery_message_draft' => null, - ])->save(); + $updates = ['delivery_message_draft' => null]; + if ($invoice->status === 'draft') { + $updates['status'] = 'sent'; + } + $invoice->forceFill($updates)->save(); } $statusMessage = $delivery->status === 'queued' diff --git a/docs/milestones/19_RC_HARDENING_OPS.md b/docs/milestones/19_RC_HARDENING_OPS.md index 4c9122af..5967b9b8 100644 --- a/docs/milestones/19_RC_HARDENING_OPS.md +++ b/docs/milestones/19_RC_HARDENING_OPS.md @@ -32,8 +32,8 @@ Supporting ops doc: [`docs/ops/DOCS_DX.md`](../ops/DOCS_DX.md) ## Phase Rollup -### [ ] Phase 1 — Notification Coverage Audit -Document every outbound mail type — trigger, recipient, delivery-log behavior — so the full mail surface is explicitly accounted for before RC. +### [x] Phase 1 — Notification Coverage Audit +Document every outbound mail type — trigger, recipient, delivery-log behavior — so the full mail surface is explicitly accounted for before RC. Closed 2026-05-09 — see [`x19.1_NOTIFICATION_COVERAGE_AUDIT.md`](../strategies/x19.1_NOTIFICATION_COVERAGE_AUDIT.md). ### [ ] Phase 2 — Auth/Password Policy Hardening Implement 419-to-login redirect and site-wide session-expiry logout. diff --git a/docs/specs/NOTIFICATIONS.md b/docs/specs/NOTIFICATIONS.md index 54881ff7..053ab751 100644 --- a/docs/specs/NOTIFICATIONS.md +++ b/docs/specs/NOTIFICATIONS.md @@ -17,7 +17,7 @@ ## 3. Base Communication Flows 1. **Send Invoice (manual owner action)** 1. Available when the invoice has a client email and its public link is enabled. - 2. Sending the invoice queues outbound delivery, records the attempt in the delivery log, and issues the invoice out of `draft`. + 2. Sending the invoice queues outbound delivery and records the attempt in the delivery log. If the invoice is currently in `draft`, the send action also transitions its status to `sent`. Sending an invoice that is already `sent`, `partial`, `pending`, `paid`, or `void` does not regress its status. 2. **Payment Acknowledgment + Receipt** 1. **Detected Payment Acknowledgment** @@ -56,14 +56,16 @@ 2. The owner reminder should communicate that the invoice is overdue, include the outstanding totals, and suggest next steps. 3. The client reminder should communicate the overdue status, include the outstanding balance and invoice link, and include a short “contact the sender if you already paid” caveat. -3. **Significant Overpayment Alert (Client)** +3. **Significant Overpayment Alert (Owner + Client)** 1. Triggered when the invoice reflects a significant overpayment (15% threshold for RC). 2. The client alert should explain that overpayments are treated as gratuities by default and tell the client to contact the sender if the overpayment was accidental. + 3. The owner alert should report the overpayment percentage and prompt a disposition decision — keep as tip, credit the client, or record a manual adjustment/refund — with a link back into the app. -4. **Significant Underpayment Alert (Client)** +4. **Significant Underpayment Alert (Owner + Client)** 1. Triggered when the invoice still carries a significant remaining balance after payment activity (15% threshold for RC). - 2. The client alert should neutrally communicate that a balance remains, include the outstanding USD/BTC amounts, and link to the public invoice so the client can settle; where appropriate, it may encourage completing the remaining balance in one payment for convenience. - 3. Fragmented or repeated partial payments should not create a separate repeated-warning alert family; they should continue to use the final underpayment behavior instead. + 2. The client alert should neutrally communicate that a balance remains, include the outstanding USD amount, and link to the public invoice so the client can settle; where appropriate, it may encourage completing the remaining balance in one payment for convenience. + 3. The owner alert should report the outstanding balance and link back to the invoice for follow-up or manual adjustment. + 4. Fragmented or repeated partial payments should not create a separate repeated-warning alert family; they should continue to use the final underpayment behavior instead. ## 5. Outbound Mail and History 1. Outbound invoice communication should use a shared queued delivery path and shared delivery history so send outcomes remain auditable. @@ -82,8 +84,8 @@ 1. `sending` is the claimed provider-boundary state used to prevent duplicate job execution from producing a second outbound send while a delivery is already in progress or awaiting operator review after an ambiguous worker failure. 12. The delivery history should use concise, human-friendly labels for communication classes and outcomes. 1. Manual invoice sends should display as `Invoice email`, not a raw storage key. - 2. Paired owner/client notification rows should keep the audience explicit in the label, such as `Past-due reminder (client)` and `Underpayment alert (owner)`. - 3. While the legacy repeated-partial warning rows still exist in stored history, they should display honestly as `Partial payment warning (client|owner)` rather than being hidden behind renamed copy. + 2. Paired issuer/client notification rows should keep the audience explicit in the label, such as `Past-due reminder (client)` and `Underpayment alert (issuer)`. + 3. While the legacy repeated-partial warning rows still exist in stored history, they should display honestly as `Partial payment warning (client|issuer)` rather than being hidden behind renamed copy. 4. Payment-triggered follow-up should keep the acknowledgment-versus-receipt split visible in history once those rows ship, using labels such as `Payment acknowledgment (client)`, `Payment acknowledgment (owner)`, and `Receipt (client)` for the later higher-certainty follow-up. 5. Outcome labels should display as `Queued`, `Sending`, `Sent`, `Skipped`, and `Failed`. 13. Outbound mail copy should stay concise and actionable. @@ -96,5 +98,23 @@ 2. That preview should be lightly rate-limited or cooldown-protected so repeated clicks do not spam the owner mailbox. 5. RC does not include arbitrary custom logo uploads for outbound mail; at most it may show or hide the default CryptoZing logo in the shared mail chrome. -## Coverage & Status (MS17 deliverable) -A coverage matrix must be added to this spec during MS17 to document the live state of all outbound notice classes. Columns: Audience, Trigger, Mailable class, Status (`live` / `stubbed` / `planned`), Feature test(s), Delivery log type. Must cover all active notice classes including paid notices, past-due reminders, overpayment and underpayment alerts, payment acknowledgments, and receipt. Note any gaps and route deferred items to `docs/BACKLOG.md`. +## Coverage & Status + +One row per outbound notice class. `Status`: `live` = in production code, behaving per spec; `stubbed` = class exists but does not behave per spec (legacy or unwired); `planned` = named in spec, not yet implemented. + +| Audience | Trigger | Mailable class | Status | Feature test(s) | Delivery log type | +|---|---|---|---|---|---| +| Client | Manual owner action (issue invoice) | `InvoiceReadyMail` | live | [`InvoiceDeliveryTest`](../../tests/Feature/InvoiceDeliveryTest.php) | `send` | +| Client | Detected on-chain payment (low-info ack) | `InvoicePaymentAcknowledgmentClientMail` | live | [`InvoiceDeliveryTest`](../../tests/Feature/InvoiceDeliveryTest.php), [`WatchPaymentsCommandTest`](../../tests/Feature/Wallet/WatchPaymentsCommandTest.php) | `payment_acknowledgment_client` | +| Owner | Detected on-chain payment (low-info ack) | `InvoicePaymentAcknowledgmentIssuerMail` | live | [`InvoiceDeliveryTest`](../../tests/Feature/InvoiceDeliveryTest.php), [`WatchPaymentsCommandTest`](../../tests/Feature/Wallet/WatchPaymentsCommandTest.php) | `payment_acknowledgment_issuer` | +| Client | Owner-reviewed manual receipt send (RC1 deliberate-manual) | `InvoicePaidReceiptMail` | live | [`InvoiceDeliveryTest`](../../tests/Feature/InvoiceDeliveryTest.php) | `receipt` | +| Owner | Invoice transitions to `paid` | `InvoiceIssuerPaidNoticeMail` | live | [`InvoiceNotificationTest`](../../tests/Feature/InvoiceNotificationTest.php), [`InvoiceDeliveryTest`](../../tests/Feature/InvoiceDeliveryTest.php), [`WatchPaymentsCommandTest`](../../tests/Feature/Wallet/WatchPaymentsCommandTest.php) | `issuer_paid_notice` | +| Owner | Past-due schedule slot fires (slots 1/2/3 at days 1/7/14) | `InvoicePastDueIssuerMail` | live | [`InvoiceNotificationTest`](../../tests/Feature/InvoiceNotificationTest.php) | `past_due_issuer` | +| Client | Past-due schedule slot fires (slots 1/2/3 at days 1/7/14) | `InvoicePastDueClientMail` | live | [`InvoiceNotificationTest`](../../tests/Feature/InvoiceNotificationTest.php) | `past_due_client` | +| Client | Overpayment ≥15% threshold | `InvoiceOverpaymentClientMail` | live | [`InvoiceNotificationTest`](../../tests/Feature/InvoiceNotificationTest.php) | `client_overpay_alert` | +| Owner | Overpayment ≥15% threshold | `InvoiceOverpaymentIssuerMail` | live | [`InvoiceNotificationTest`](../../tests/Feature/InvoiceNotificationTest.php) | `issuer_overpay_alert` | +| Client | Underpayment ≥15% remaining | `InvoiceUnderpaymentClientMail` | live | [`InvoiceNotificationTest`](../../tests/Feature/InvoiceNotificationTest.php) | `client_underpay_alert` | +| Owner | Underpayment ≥15% remaining | `InvoiceUnderpaymentIssuerMail` | live | [`InvoiceNotificationTest`](../../tests/Feature/InvoiceNotificationTest.php) | `issuer_underpay_alert` | +| Client | (deprecated — superseded by underpay alert per §4.4.3; legacy rows render in history per §5.12.3) | `InvoicePartialWarningClientMail` | stubbed | [`WatchPaymentsCommandTest`](../../tests/Feature/Wallet/WatchPaymentsCommandTest.php) (asserts zero new queued) | `client_partial_warning` | +| Owner | (deprecated — superseded by underpay alert per §4.4.3; legacy rows render in history per §5.12.3) | `InvoicePartialWarningIssuerMail` | stubbed | [`WatchPaymentsCommandTest`](../../tests/Feature/Wallet/WatchPaymentsCommandTest.php) (asserts zero new queued) | `issuer_partial_warning` | +| Owner (self) | Manual "send me a test email" preview action (§5.14.4) | `NotificationBrandingPreviewMail` | live | [`MailBrandingTest`](../../tests/Feature/MailBrandingTest.php) | — (no delivery-log row, per §5.14.4.1) | diff --git a/docs/strategies/19.1_NOTIFICATION_COVERAGE_AUDIT.md b/docs/strategies/19.1_NOTIFICATION_COVERAGE_AUDIT.md deleted file mode 100644 index a37bab3c..00000000 --- a/docs/strategies/19.1_NOTIFICATION_COVERAGE_AUDIT.md +++ /dev/null @@ -1,75 +0,0 @@ -# MS19 Phase 1 Strategy — Notification Coverage Audit - -Status: Not started. -Parent milestone doc: [`docs/milestones/19_RC_HARDENING_OPS.md`](../milestones/19_RC_HARDENING_OPS.md) - -**Goal:** Finish the coverage matrix that [`docs/specs/NOTIFICATIONS.md`](../specs/NOTIFICATIONS.md) flagged as an MS17 deliverable but never received. Confirm every active outbound mail class has a documented row covering audience, trigger, code reference, status, feature test, and delivery-log type. Tweak the spec narrative inline where code has drifted from documented behavior. - -## Decisions made - -- **Inventory location:** Extend `NOTIFICATIONS.md`. The matrix slot already exists at the bottom of the spec under `## Coverage & Status (MS17 deliverable)`. -- **Coverage format:** Use the column set the spec already specifies — Audience, Trigger, Mailable class, Status (`live` / `stubbed` / `planned`), Feature test(s), Delivery log type. -- **Gap-closure scope:** Fixes land in this phase by default. If a finding is small-consequence relative to its work cost, route it to [`docs/BACKLOG.md`](../BACKLOG.md) — but only when MS19's phase goal and exit criteria still hold without the deferred work. - -## Carried-in known finding (resolved) - -`Tests\Feature\InvoiceNotificationTest` was failing across every recent PR back to MS17 — past-due delivery assertions expecting `queued` observed `skipped`/`sent`. Root cause: `.env.example` shipped `MAIL_OUTBOUND_ENABLED=false` and the PR Tests workflow copied it verbatim into `.env`, so `InvoiceDeliveryService::skipReason()` marked every queued row as skipped. Resolved via [#68](https://github.com/n8bar/CryptoZing/issues/68) / [#69](https://github.com/n8bar/CryptoZing/pull/69) by pinning the env var in `phpunit.xml`. First Issue under the M19 GitHub Issues trial (see milestone doc Decisions recorded). - -## 1. Enumerate the live mail surface - -1. [ ] Re-list every Mail class under `app/Mail/` directly from disk; do not trust the snapshot in the Reference section without re-verifying. -2. [ ] List every Notification class under `app/Notifications/` (if any). -3. [ ] Identify any in-app outbound paths that bypass Mail/Notification classes (raw mailer calls in Jobs, Commands, or Console scheduled tasks). -4. [ ] Cross-check the resulting set against the notice classes named in `NOTIFICATIONS.md` Sections 3–5 — flag anything in code without a spec entry, or anything in the spec without code. - -## 2. Build the coverage matrix in `NOTIFICATIONS.md` - -1. [ ] Replace the placeholder under `## Coverage & Status (MS17 deliverable)` with a Markdown table using the spec's column set: Audience | Trigger | Mailable class | Status | Feature test(s) | Delivery log type. -2. [ ] Populate one row per notice class identified in §1. -3. [ ] For each row's **Status** field, record `live` (in production code, exercising correctly), `stubbed` (class exists but doesn't behave per spec), or `planned` (named in spec, not implemented). -4. [ ] For each row's **Feature test(s)** field, link to the corresponding test under `tests/Feature/**` if one exists; record `none` if not. -5. [ ] For each row's **Delivery log type** field, record the log-type identifier the class uses when creating delivery-log entries. -6. [ ] Drop the "(MS17 deliverable)" suffix from the section heading once the matrix is populated. - -## 3. Reconcile drift and tweak the narrative - -1. [ ] For each matrix row, compare the actual class behavior to the documented behavior in `NOTIFICATIONS.md` Sections 3–5. -2. [ ] Where the spec is right and the code drifted, capture a finding (audience, class, what the spec says vs. what the code does) for §4. -3. [ ] Where the code is right and the spec is stale, tweak the spec narrative inline — small edits only. -4. [ ] If a tweak feels larger than "small" (rewrites a paragraph, changes a documented invariant, contradicts a §3–§5 commitment), stop and surface for confirmation before applying. - -## 4. Resolve findings - -1. [ ] For each in-phase finding from §3, ship the fix (code change or test) within Phase 1 and check off the finding here. -2. [ ] For each backlog-bound finding (small consequence, high effort), add an entry to `docs/BACKLOG.md` with enough context to pick it up later. List the deferred finding here with a `[deferred → backlog]` note. -3. [ ] Confirm the Phase 1 Exit Criteria still hold after any deferrals. - -## Exit Criteria - -- [ ] `NOTIFICATIONS.md` Coverage & Status section contains a populated matrix with one row per active notice class. -- [ ] Every Mail/Notification class in code is represented in the matrix. -- [ ] Every notice class named in spec Sections 3–5 has a matching matrix row. -- [ ] Drift findings are either fixed in this phase or recorded in `BACKLOG.md` with context. -- [ ] `(MS17 deliverable)` placeholder text is removed from the spec heading. -- [x] CI flakiness in `Tests\Feature\InvoiceNotificationTest` (past-due delivery status: expected `queued`, observed `skipped`/`sent`) investigated and resolved. Tests pass green on a Phase-1-close PR. (Resolved via [#68](https://github.com/n8bar/CryptoZing/issues/68).) - -## Reference - -Mail classes present in `app/Mail/` at skeleton drafting (2026-05-08). Re-enumerate in §1.1; this list is a starting point, not authoritative. - -- `InvoiceIssuerPaidNoticeMail.php` -- `InvoiceOverpaymentClientMail.php` -- `InvoiceOverpaymentIssuerMail.php` -- `InvoicePaidReceiptMail.php` -- `InvoicePartialWarningClientMail.php` -- `InvoicePartialWarningIssuerMail.php` -- `InvoicePastDueClientMail.php` -- `InvoicePastDueIssuerMail.php` -- `InvoicePaymentAcknowledgmentClientMail.php` -- `InvoicePaymentAcknowledgmentIssuerMail.php` -- `InvoiceReadyMail.php` -- `InvoiceUnderpaymentClientMail.php` -- `InvoiceUnderpaymentIssuerMail.php` -- `NotificationBrandingPreviewMail.php` - -`NOTIFICATIONS.md` self-flagged the gap at lines 99–100 (the `## Coverage & Status (MS17 deliverable)` heading exists; the matrix below it does not). diff --git a/docs/strategies/x19.1_NOTIFICATION_COVERAGE_AUDIT.md b/docs/strategies/x19.1_NOTIFICATION_COVERAGE_AUDIT.md new file mode 100644 index 00000000..f22be040 --- /dev/null +++ b/docs/strategies/x19.1_NOTIFICATION_COVERAGE_AUDIT.md @@ -0,0 +1,114 @@ +# MS19 Phase 1 Strategy — Notification Coverage Audit + +Status: Complete. +Parent milestone doc: [`docs/milestones/19_RC_HARDENING_OPS.md`](../milestones/19_RC_HARDENING_OPS.md) + +**Goal:** Finish the coverage matrix that [`docs/specs/NOTIFICATIONS.md`](../specs/NOTIFICATIONS.md) flagged as an MS17 deliverable but never received. Confirm every active outbound mail class has a documented row covering audience, trigger, code reference, status, feature test, and delivery-log type. Tweak the spec narrative inline where code has drifted from documented behavior. + +## Decisions made + +- **Inventory location:** Extend `NOTIFICATIONS.md`. The matrix slot already exists at the bottom of the spec under `## Coverage & Status (MS17 deliverable)`. +- **Coverage format:** Use the column set the spec already specifies — Audience, Trigger, Mailable class, Status (`live` / `stubbed` / `planned`), Feature test(s), Delivery log type. +- **Gap-closure scope:** Fixes land in this phase by default. If a finding is small-consequence relative to its work cost, route it to [`docs/BACKLOG.md`](../BACKLOG.md) — but only when MS19's phase goal and exit criteria still hold without the deferred work. + +## Carried-in known finding (resolved) + +`Tests\Feature\InvoiceNotificationTest` was failing across every recent PR back to MS17 — past-due delivery assertions expecting `queued` observed `skipped`/`sent`. Root cause: `.env.example` shipped `MAIL_OUTBOUND_ENABLED=false` and the PR Tests workflow copied it verbatim into `.env`, so `InvoiceDeliveryService::skipReason()` marked every queued row as skipped. Resolved via [#68](https://github.com/n8bar/CryptoZing/issues/68) / [#69](https://github.com/n8bar/CryptoZing/pull/69) by pinning the env var in `phpunit.xml`. First Issue under the M19 GitHub Issues trial (see milestone doc Decisions recorded). + +## 1. Enumerate the live mail surface + +1. [x] Re-list every Mail class under `app/Mail/` directly from disk; do not trust the snapshot in the Reference section without re-verifying. +2. [x] List every Notification class under `app/Notifications/` (if any). +3. [x] Identify any in-app outbound paths that bypass Mail/Notification classes (raw mailer calls in Jobs, Commands, or Console scheduled tasks). +4. [x] Cross-check the resulting set against the notice classes named in `NOTIFICATIONS.md` Sections 3–5 — flag anything in code without a spec entry, or anything in the spec without code. + +**§1 results (2026-05-09):** + +- **Mail classes (14):** `app/Mail/` matches the Reference snapshot — no drift since drafting. +- **Notification classes (0):** No `app/Notifications/` directory. +- **Outbound paths (2 callsites, both via Mail classes — no raw-mailer bypass):** + - `App\Jobs\DeliverInvoiceMail::handle` — canonical delivery-log-driven job; renders the right Mailable per `delivery->type` (13 mappings + `InvoiceReadyMail` default for manual sends) and dispatches via `Mail::to(...)->send(...)`. + - `App\Http\Controllers\NotificationSettingsController::send` — owner-only branding preview; sends `NotificationBrandingPreviewMail` directly without creating a delivery-log row (per spec §5.14.4.1). +- **Drift flags for §3:** + - **Spec narrative gap — overpay/underpay paired classes (resolved):** Code has `InvoiceOverpaymentIssuerMail` and `InvoiceUnderpaymentIssuerMail` paired with their client siblings; spec §4.3 and §4.4 originally titled these alerts as "(Client)" only. Pulled forward from §3 and applied: §4.3 and §4.4 retitled as "(Owner + Client)" with an owner-alert sub-bullet derived from the actual issuer mail bodies (overpay disposition prompt; underpay outstanding-balance follow-up). §5.4 paired-pattern and §5.12.2 "Underpayment alert (owner)" language are now consistent with §4. + - **Partial-warning status — stubbed by design:** `InvoicePartialWarningClientMail` and `InvoicePartialWarningIssuerMail` are wired into `DeliverInvoiceMail`'s type map, but no service code queues new `*_partial_warning` deliveries; `InvoiceAlertService::skipInvalidQueuedDeliveries()` only skips legacy queued rows. Tests `test_repeated_partial_payment_detection_no_longer_creates_a_separate_partial_warning_family` and `..._does_not_enqueue_legacy_partial_warnings_on_repeat_runs` assert zero new partial-warning rows. Matches spec §4.4.3 (deprecating the family) and §5.12.3 (legacy display only). To be recorded as `stubbed` in the §2 matrix. + +## 2. Build the coverage matrix in `NOTIFICATIONS.md` + +1. [x] Replace the placeholder under `## Coverage & Status (MS17 deliverable)` with a Markdown table using the spec's column set: Audience | Trigger | Mailable class | Status | Feature test(s) | Delivery log type. +2. [x] Populate one row per notice class identified in §1. +3. [x] For each row's **Status** field, record `live` (in production code, exercising correctly), `stubbed` (class exists but doesn't behave per spec), or `planned` (named in spec, not implemented). +4. [x] For each row's **Feature test(s)** field, link to the corresponding test under `tests/Feature/**` if one exists; record `none` if not. +5. [x] For each row's **Delivery log type** field, record the log-type identifier the class uses when creating delivery-log entries. +6. [x] Drop the "(MS17 deliverable)" suffix from the section heading once the matrix is populated. + +**§2 results (2026-05-09):** 14 rows populated — 12 `live`, 2 `stubbed` (partial-warning legacy classes). All Mailables have feature-test coverage. Delivery log types match `DeliverInvoiceMail`'s type→Mailable map plus `send` (manual default) and `—` for the branding preview (which deliberately bypasses the delivery log). + +## 3. Reconcile drift and tweak the narrative + +1. [x] For each matrix row, compare the actual class behavior to the documented behavior in `NOTIFICATIONS.md` Sections 3–5. +2. [x] Where the spec is right and the code drifted, capture a finding (audience, class, what the spec says vs. what the code does) for §4. +3. [x] Where the code is right and the spec is stale, tweak the spec narrative inline — small edits only. (Pulled forward earlier for the overpay/underpay paired-class titling — see §1 drift flags.) +4. [x] If a tweak feels larger than "small" (rewrites a paragraph, changes a documented invariant, contradicts a §3–§5 commitment), stop and surface for confirmation before applying. + +**§3 results (2026-05-09):** 11 rows aligned with spec; 3 drift findings raised for §4 (one needing a user decision on direction, two small-scoped). 14th-row check (overpay/underpay narrative) was applied earlier in this phase. + +### Findings raised for §4 + +**F1 — Manual send does not transition invoice out of `draft` (decision needed).** +- Spec §3.1.2 says: "Sending the invoice queues outbound delivery, records the attempt in the delivery log, **and issues the invoice out of `draft`**." +- Code: `InvoiceDeliveryController::store` queues the `send` delivery row but does not update `Invoice::status`. `Invoice::refreshPaymentState()` explicitly skips `draft`/`void` invoices, so a draft that gets sent stays at `status='draft'` until something else moves it. +- This is bigger than a "small inline tweak" per §3.4 — needs a deliberate decision: either the spec is over-specifying (drop the draft-transition clause) or the code is missing a status update on successful queue (small but real code change). + +**F2 — Spec uses "(owner)" in display-label examples; shipped labels use "(issuer)".** +- Spec §5.12.2 example: `Underpayment alert (owner)`. Spec §5.12.3 example: `Partial payment warning (client|owner)`. +- Code (`InvoiceDelivery::typeLabel()`): consistently uses `(issuer)` after the MS17 owner→issuer terminology sweep — `Underpayment alert (issuer)`, `Partial payment warning (issuer)`, etc. +- Resolution: small spec tweak — replace "(owner)" with "(issuer)" in §5.12.2 and §5.12.3 examples so the spec matches the shipped labels. + +**F3 — Client underpayment alert view omits BTC outstanding amount (decision needed, small either way).** +- Spec §4.4.2: client alert should "include the outstanding USD/BTC amounts." +- Code (`resources/views/mail/invoice-underpayment-client.blade.php`): shows the outstanding USD amount only; no BTC line. +- The legacy `invoice-partial-warning-client.blade.php` view does show both USD and BTC, so the project pattern supports either interpretation. +- Two reasonable resolutions: (a) add a BTC line to the underpay-client view for spec compliance, or (b) drop "/BTC" from §4.4.2 since the linked invoice page already surfaces full BTC details. + +## 4. Resolve findings + +1. [x] For each in-phase finding from §3, ship the fix (code change or test) within Phase 1 and check off the finding here. +2. [x] For each backlog-bound finding (small consequence, high effort), add an entry to `docs/BACKLOG.md` with enough context to pick it up later. List the deferred finding here with a `[deferred → backlog]` note. +3. [x] Confirm the Phase 1 Exit Criteria still hold after any deferrals. + +**§4 results (2026-05-09):** All three §3 findings resolved in-phase; no backlog deferrals. + +- **F1 — Manual send draft transition (Option C, applied):** Spec §3.1.2 reworded to make the transition rule explicit and conditional with a no-regression guarantee. `InvoiceDeliveryController::store` updated to transition `draft` → `sent` after a successful queue. Two new tests in `InvoiceDeliveryTest` (`test_sending_draft_invoice_transitions_status_to_sent`, `test_sending_does_not_regress_status_for_non_draft_invoice`). +- **F2 — Owner → Issuer in spec display labels:** §5.12.2 and §5.12.3 examples replaced with `(issuer)` to match shipped `InvoiceDelivery::typeLabel()` output. Catches the spec up to the MS17 terminology sweep. +- **F3 — Client underpay alert USD/BTC (b, applied):** §4.4.2 narrowed to "outstanding USD amount." Keeps the spec loose; the linked invoice page surfaces full BTC details, and the email body stays simple/USD-focused for the typical recipient. Code is free to add a BTC line or current-rate hint as a future enhancement without a re-spec round. + +## Exit Criteria + +- [x] `NOTIFICATIONS.md` Coverage & Status section contains a populated matrix with one row per active notice class. +- [x] Every Mail/Notification class in code is represented in the matrix. +- [x] Every notice class named in spec Sections 3–5 has a matching matrix row. +- [x] Drift findings are either fixed in this phase or recorded in `BACKLOG.md` with context. +- [x] `(MS17 deliverable)` placeholder text is removed from the spec heading. +- [x] CI flakiness in `Tests\Feature\InvoiceNotificationTest` (past-due delivery status: expected `queued`, observed `skipped`/`sent`) investigated and resolved. Tests pass green on a Phase-1-close PR. (Resolved via [#68](https://github.com/n8bar/CryptoZing/issues/68).) + +## Reference + +Mail classes present in `app/Mail/` at skeleton drafting (2026-05-08). Re-enumerate in §1.1; this list is a starting point, not authoritative. + +- `InvoiceIssuerPaidNoticeMail.php` +- `InvoiceOverpaymentClientMail.php` +- `InvoiceOverpaymentIssuerMail.php` +- `InvoicePaidReceiptMail.php` +- `InvoicePartialWarningClientMail.php` +- `InvoicePartialWarningIssuerMail.php` +- `InvoicePastDueClientMail.php` +- `InvoicePastDueIssuerMail.php` +- `InvoicePaymentAcknowledgmentClientMail.php` +- `InvoicePaymentAcknowledgmentIssuerMail.php` +- `InvoiceReadyMail.php` +- `InvoiceUnderpaymentClientMail.php` +- `InvoiceUnderpaymentIssuerMail.php` +- `NotificationBrandingPreviewMail.php` + +`NOTIFICATIONS.md` self-flagged the gap at lines 99–100 (the `## Coverage & Status (MS17 deliverable)` heading exists; the matrix below it does not). diff --git a/tests/Feature/InvoiceDeliveryTest.php b/tests/Feature/InvoiceDeliveryTest.php index f6aa3514..f7040f52 100644 --- a/tests/Feature/InvoiceDeliveryTest.php +++ b/tests/Feature/InvoiceDeliveryTest.php @@ -65,6 +65,66 @@ public function test_owner_can_queue_invoice_email(): void }); } + public function test_sending_draft_invoice_transitions_status_to_sent(): void + { + Queue::fake(); + $owner = User::factory()->create(); + $client = Client::create([ + 'user_id' => $owner->id, + 'name' => 'Acme Co', + 'email' => 'billing@example.com', + ]); + + $invoice = Invoice::create([ + 'user_id' => $owner->id, + 'client_id' => $client->id, + 'number' => 'INV-1001-DRAFT-SEND', + 'amount_usd' => 200, + 'btc_rate' => 40_000, + 'amount_btc' => 0.005, + 'payment_address' => 'tb1qq0exampledraftsend', + 'status' => 'draft', + 'invoice_date' => now()->toDateString(), + ]); + $invoice->enablePublicShare(); + + $this->actingAs($owner) + ->post(route('invoices.deliver', $invoice)) + ->assertRedirect(); + + $this->assertSame('sent', $invoice->fresh()->status); + } + + public function test_sending_does_not_regress_status_for_non_draft_invoice(): void + { + Queue::fake(); + $owner = User::factory()->create(); + $client = Client::create([ + 'user_id' => $owner->id, + 'name' => 'Acme Co', + 'email' => 'billing@example.com', + ]); + + $invoice = Invoice::create([ + 'user_id' => $owner->id, + 'client_id' => $client->id, + 'number' => 'INV-1001-PAID-SEND', + 'amount_usd' => 200, + 'btc_rate' => 40_000, + 'amount_btc' => 0.005, + 'payment_address' => 'tb1qq0examplepaidsend', + 'status' => 'paid', + 'invoice_date' => now()->toDateString(), + ]); + $invoice->enablePublicShare(); + + $this->actingAs($owner) + ->post(route('invoices.deliver', $invoice)) + ->assertRedirect(); + + $this->assertSame('paid', $invoice->fresh()->status); + } + public function test_owner_can_save_delivery_message_draft(): void { $owner = User::factory()->create();