From 5bae510da3cb584a7122d22653d065439c07795b Mon Sep 17 00:00:00 2001 From: Nate Barlow Date: Sat, 9 May 2026 20:48:20 -0600 Subject: [PATCH 1/5] =?UTF-8?q?M19.1=20=C2=A71:=20enumerate=20live=20mail?= =?UTF-8?q?=20surface=20and=20flag=20drift?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Confirms 14 Mail classes (no app/Notifications/, no raw-mailer bypass) and lists the two outbound callsites — DeliverInvoiceMail (canonical delivery-log-driven path) and NotificationSettingsController (owner branding preview, not delivery-log driven). Two drift flags for §3 to address: - Spec §4.3 and §4.4 title overpay/underpay alerts as "(Client)" only, but code has paired client/issuer classes and §5.4 / §5.12.2 imply the pair. Small inline spec tweak indicated. - Partial-warning Mail classes are wired into the type map but no service queues them; matches spec §4.4.3 / §5.12.3 (deprecated alert family, legacy display only). Will surface as `stubbed` in the §2 matrix. --- .../19.1_NOTIFICATION_COVERAGE_AUDIT.md | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/docs/strategies/19.1_NOTIFICATION_COVERAGE_AUDIT.md b/docs/strategies/19.1_NOTIFICATION_COVERAGE_AUDIT.md index a37bab3..e1c3d59 100644 --- a/docs/strategies/19.1_NOTIFICATION_COVERAGE_AUDIT.md +++ b/docs/strategies/19.1_NOTIFICATION_COVERAGE_AUDIT.md @@ -17,10 +17,21 @@ Parent milestone doc: [`docs/milestones/19_RC_HARDENING_OPS.md`](../milestones/1 ## 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. +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:** Code has `InvoiceOverpaymentIssuerMail` and `InvoiceUnderpaymentIssuerMail` paired with their client siblings. Spec §4.3 and §4.4 title these alerts as "(Client)" only, even though §5.4 establishes the paired pattern and §5.12.2 names "Underpayment alert (owner)" explicitly. Recommend: small inline tweak to §4.3 and §4.4 to reflect the paired classes. + - **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` From 37f1a83398663125437a3e9e007d566a7b845733 Mon Sep 17 00:00:00 2001 From: Nate Barlow Date: Sat, 9 May 2026 21:23:11 -0600 Subject: [PATCH 2/5] NOTIFICATIONS spec: title overpay/underpay alerts as paired (Owner + Client) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code has had paired client/issuer Mailables for both alerts since they were introduced, and §5.4 / §5.12.2 of NOTIFICATIONS.md already imply the pair. §4.3 and §4.4 still titled them as "(Client)" only, leaving the spec narrative internally inconsistent. Retitled both as "(Owner + Client)" — matching §4.2 Past Due — and added one owner-alert sub-bullet to each, derived from the actual issuer mail bodies: - §4.3 owner: overpayment percentage + disposition prompt (tip / credit / manual adjustment / refund) with link back into app. - §4.4 owner: outstanding balance + link back for follow-up or manual adjustment. Existing client-alert language and the fragmented-partial-payment guidance are unchanged. Pulled this work forward from §3 of the M19.1 strategy doc; the §1 drift flag is annotated as resolved with a breadcrumb to here. --- docs/specs/NOTIFICATIONS.md | 8 +++++--- docs/strategies/19.1_NOTIFICATION_COVERAGE_AUDIT.md | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/specs/NOTIFICATIONS.md b/docs/specs/NOTIFICATIONS.md index 54881ff..2aac377 100644 --- a/docs/specs/NOTIFICATIONS.md +++ b/docs/specs/NOTIFICATIONS.md @@ -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. + 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. diff --git a/docs/strategies/19.1_NOTIFICATION_COVERAGE_AUDIT.md b/docs/strategies/19.1_NOTIFICATION_COVERAGE_AUDIT.md index e1c3d59..b813ca5 100644 --- a/docs/strategies/19.1_NOTIFICATION_COVERAGE_AUDIT.md +++ b/docs/strategies/19.1_NOTIFICATION_COVERAGE_AUDIT.md @@ -30,7 +30,7 @@ Parent milestone doc: [`docs/milestones/19_RC_HARDENING_OPS.md`](../milestones/1 - `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:** Code has `InvoiceOverpaymentIssuerMail` and `InvoiceUnderpaymentIssuerMail` paired with their client siblings. Spec §4.3 and §4.4 title these alerts as "(Client)" only, even though §5.4 establishes the paired pattern and §5.12.2 names "Underpayment alert (owner)" explicitly. Recommend: small inline tweak to §4.3 and §4.4 to reflect the paired classes. + - **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` From 2c66600f9e2c83e2ca18bc0a151b3be4bd1c9f31 Mon Sep 17 00:00:00 2001 From: Nate Barlow Date: Sat, 9 May 2026 21:26:13 -0600 Subject: [PATCH 3/5] =?UTF-8?q?M19.1=20=C2=A72:=20populate=20Coverage=20&?= =?UTF-8?q?=20Status=20matrix=20in=20NOTIFICATIONS=20spec?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 14-row coverage matrix replaces the MS17 placeholder. One row per outbound Mailable in `app/Mail/`. Status: 12 `live`, 2 `stubbed` (InvoicePartialWarning client/issuer — Mail classes wired into DeliverInvoiceMail's type map but no service queues new rows; matches spec §4.4.3 deprecation guidance and §5.12.3 legacy-display rule). Each row links to its feature test(s) by relative path; delivery log types reference the canonical map in DeliverInvoiceMail::handle plus `send` (manual default) and `—` for NotificationBrandingPreviewMail which bypasses the delivery log per spec §5.14.4.1. `(MS17 deliverable)` suffix dropped from the heading. Strategy doc §2 checkoffs all marked, with a brief §2 results note. --- docs/specs/NOTIFICATIONS.md | 22 +++++++++++++++++-- .../19.1_NOTIFICATION_COVERAGE_AUDIT.md | 14 +++++++----- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/docs/specs/NOTIFICATIONS.md b/docs/specs/NOTIFICATIONS.md index 2aac377..c64cc07 100644 --- a/docs/specs/NOTIFICATIONS.md +++ b/docs/specs/NOTIFICATIONS.md @@ -98,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 index b813ca5..6f7bfa6 100644 --- a/docs/strategies/19.1_NOTIFICATION_COVERAGE_AUDIT.md +++ b/docs/strategies/19.1_NOTIFICATION_COVERAGE_AUDIT.md @@ -35,12 +35,14 @@ Parent milestone doc: [`docs/milestones/19_RC_HARDENING_OPS.md`](../milestones/1 ## 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. +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 From d890d0fab471c3c5061b506d4ab89e430219b871 Mon Sep 17 00:00:00 2001 From: Nate Barlow Date: Sat, 9 May 2026 21:47:53 -0600 Subject: [PATCH 4/5] =?UTF-8?q?M19.1=20=C2=A73:=20per-row=20drift=20verifi?= =?UTF-8?q?cation=20=E2=80=94=2011=20aligned,=203=20findings=20raised?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Walked each of the 14 matrix rows. Read each Mailable's Blade view and its trigger callsite (InvoiceAlertService for alerts, DeliverInvoiceMail for sends, the two controllers for manual send/receipt and the branding preview, and the past-due console command). Aligned (11 rows): InvoiceReady (modulo F1 question), payment-ack pair, InvoicePaidReceipt, InvoiceIssuerPaidNotice, past-due pair, overpay pair (post-tweak), underpay-issuer, partial-warning pair (stubbed by design, matches §4.4.3 / §5.12.3), NotificationBrandingPreview. Three findings raised for §4: - F1: Spec §3.1.2 says manual send "issues the invoice out of `draft`" but no code path transitions Invoice::status on send. Either spec over-specifies or code is missing the transition. Decision needed. - F2: Spec §5.12.2 / §5.12.3 examples use "(owner)" but shipped labels use "(issuer)" after the MS17 sweep. Small spec tweak. - F3: Client underpay alert view shows USD only; spec §4.4.2 calls for USD/BTC. Either add BTC line to the view or drop "/BTC" from spec. Decision needed (small either way). Strategy doc §3 checkoffs marked; findings recorded inline. --- .../19.1_NOTIFICATION_COVERAGE_AUDIT.md | 28 ++++++++++++++++--- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/docs/strategies/19.1_NOTIFICATION_COVERAGE_AUDIT.md b/docs/strategies/19.1_NOTIFICATION_COVERAGE_AUDIT.md index 6f7bfa6..62f96e0 100644 --- a/docs/strategies/19.1_NOTIFICATION_COVERAGE_AUDIT.md +++ b/docs/strategies/19.1_NOTIFICATION_COVERAGE_AUDIT.md @@ -46,10 +46,30 @@ Parent milestone doc: [`docs/milestones/19_RC_HARDENING_OPS.md`](../milestones/1 ## 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. +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 From 3d075c620cd524b04c218e55e0ed59432e7af5b2 Mon Sep 17 00:00:00 2001 From: Nate Barlow Date: Sat, 9 May 2026 22:31:23 -0600 Subject: [PATCH 5/5] =?UTF-8?q?M19.1=20=C2=A74:=20resolve=20all=20three=20?= =?UTF-8?q?drift=20findings;=20close=20Phase=201?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three findings from §3 resolved in-phase. No backlog deferrals. All Phase 1 exit criteria satisfied. Strategy doc renamed with `x` prefix per the project's completed-doc convention; Phase 1 rollup checked off in the M19 milestone doc. F1 — Manual send draft transition (Option C): - Spec §3.1.2 reworded: explicit transition rule with no-regression guarantee for already-sent/partial/pending/paid/void invoices. - InvoiceDeliveryController::store: after a successful queue, draft invoices transition to `sent`; non-draft invoices keep their current status. - Two new feature tests in InvoiceDeliveryTest covering both the transition and the no-regression cases. F2 — Owner → Issuer in spec display labels: - Spec §5.12.2 example: `Underpayment alert (owner)` → `(issuer)`. - Spec §5.12.3 example: `Partial payment warning (client|owner)` → `(client|issuer)`. - Catches the spec up to the MS17 owner→issuer terminology sweep that already shipped in InvoiceDelivery::typeLabel(). F3 — Client underpay alert USD/BTC: - Spec §4.4.2 narrowed to "outstanding USD amount". Linked invoice page surfaces full BTC; email body stays simple/USD-focused. - No code change — current `invoice-underpayment-client.blade.php` is already aligned with the narrower spec. Test suite: 336 passing (was 334; +2 new tests for F1). --- .../Controllers/InvoiceDeliveryController.php | 8 ++- docs/milestones/19_RC_HARDENING_OPS.md | 4 +- docs/specs/NOTIFICATIONS.md | 8 +-- ...d => x19.1_NOTIFICATION_COVERAGE_AUDIT.md} | 24 +++++--- tests/Feature/InvoiceDeliveryTest.php | 60 +++++++++++++++++++ 5 files changed, 86 insertions(+), 18 deletions(-) rename docs/strategies/{19.1_NOTIFICATION_COVERAGE_AUDIT.md => x19.1_NOTIFICATION_COVERAGE_AUDIT.md} (86%) diff --git a/app/Http/Controllers/InvoiceDeliveryController.php b/app/Http/Controllers/InvoiceDeliveryController.php index 040df0b..1e334a7 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 4c9122a..5967b9b 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 c64cc07..053ab75 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** @@ -63,7 +63,7 @@ 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. + 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. @@ -84,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. diff --git a/docs/strategies/19.1_NOTIFICATION_COVERAGE_AUDIT.md b/docs/strategies/x19.1_NOTIFICATION_COVERAGE_AUDIT.md similarity index 86% rename from docs/strategies/19.1_NOTIFICATION_COVERAGE_AUDIT.md rename to docs/strategies/x19.1_NOTIFICATION_COVERAGE_AUDIT.md index 62f96e0..f22be04 100644 --- a/docs/strategies/19.1_NOTIFICATION_COVERAGE_AUDIT.md +++ b/docs/strategies/x19.1_NOTIFICATION_COVERAGE_AUDIT.md @@ -1,6 +1,6 @@ # MS19 Phase 1 Strategy — Notification Coverage Audit -Status: Not started. +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. @@ -73,17 +73,23 @@ Parent milestone doc: [`docs/milestones/19_RC_HARDENING_OPS.md`](../milestones/1 ## 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. +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 -- [ ] `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] `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 diff --git a/tests/Feature/InvoiceDeliveryTest.php b/tests/Feature/InvoiceDeliveryTest.php index f6aa351..f7040f5 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();