fix: expose Play grace-period payment failures - #507
Conversation
|
Approach before implementation:
|
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes Verified the current PR head contains no source, test, configuration, or contract changes.
- Empty implementation-start revision: Commit
6385f99records the start of ticket work but has an authoritative diff of zero changed files, so later implementation commits require a fresh review.
GPT Sol | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes Reviewed the substantive implementation added since the prior Pullfrog review at 6385f99.
- Grace-state propagation: Carried Google's current grace-period state through the billing adapter, notification handler, purchase verification handler, and domain mutation while retaining Pro access and exposing
payment_failed. - Entitlement precedence: Preserved Stripe ownership when it covers a later period and prevented a stale cancellation notification from replacing the current Play grace-period reason.
- Regression coverage: Expanded SDK-backed adapter, handler, status-projection, and domain tests across active, grace, hold, pause, cancellation, recovery, and cross-provider cases.
GPT Sol | 𝕏
The drift job regenerates architecture.json and architecture.html and fails on any diff. They were stale on this branch, so the job failed on the artifact rather than on the change. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01E565x2qj1gcMoQbSym1cvy
|
@pullfrog review |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes Reviewed the complete Play grace-period implementation at e5ece98, including billing-state projection, entitlement ownership, persistence, client-facing status semantics, regression coverage, and generated architecture artifacts.
- Authoritative grace state: Carries Google's current grace-period state through the internal billing record without changing a public DTO or persisted schema.
- Entitlement and reason handling: Keeps Play Pro access with
payment_failed, clears the reason after recovery, protects a later Stripe entitlement, and prevents a stale cancellation notification from replacing the current grace state. - Client compatibility: Preserves append-only API compatibility for installed clients, while the paired
orbit-ui-mobile#850path consumes the existingpayment_failedvalue after the API-first rollout. - Regression coverage: Exercises the real Google SDK deserialization adapter plus notification, purchase-verification, status-projection, non-entitled-state, cross-provider, and domain paths; focused application and domain tests pass locally.
GPT Sol | 𝕏
|
|
@pullfrog review |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes Reviewed the complete Play grace-period implementation at d9cd26c, including the paired mobile warning path and all current CI results.
- Authoritative Play state: Projects the Google SDK grace-period state through the internal billing record without changing a public DTO or persisted schema.
- Entitlement semantics: Retains Pro access with
payment_failed, clears the reason after recovery, preserves a later Stripe entitlement, and prevents a stale cancellation notification from replacing a current grace state. - Writer coverage: Applies the same state policy in both real-time developer notifications and direct purchase verification.
- Regression coverage: Exercises SDK deserialization, notification and verification handlers, status projection, domain mutation, non-entitled states, and provider precedence; all required checks are green.
- Client compatibility: The paired
orbit-ui-mobile#850implementation already maps the existingpayment_failedvalue to the entitled Play management warning, so the API-first rollout remains append-only for installed clients.
GPT Sol | 𝕏




Fixes thomasluizon/orbit-tickets#449
Play grace-period refreshes previously kept Pro access while clearing the payment-failure reason. The Google adapter now carries IsInGracePeriod through notifications and purchase verification. SetPlaySubscription maps that state to the existing PaymentFailed reason while retaining entitlement; verified active state clears it. The status response exposes source=play, hasProAccess=true and lapseReason=payment_failed during grace.
The verified state takes precedence over a stale cancellation notification while grace is current. Stripe entitlement ownership remains protected. The new flag is optional on the internal billing record; public DTOs, enum values, database schema and Stripe production paths are unchanged. The boolean domain input can only select PaymentFailed or null, so no arbitrary lapse reason can enter through this method. Existing request validators remain unchanged because no request contract changes.
State policy
Assumptions
External interface evidence
No live Play purchase or credentials were used. The new external read is the existing Google SDK SubscriptionPurchaseV2.SubscriptionState property. Its string type and subscriptionState JSON binding were confirmed in the exact source commit recorded by the installed Google.Apis.AndroidPublisher.v3 1.75.0.4246 nuspec: generated SDK source. Re-derive by reading the package's repository commit, then inspecting SubscriptionPurchaseV2 in that commit. The source also confirms the fixture's acknowledgementState, externalAccountIdentifiers, lineItems, expiryTime, productId and offerDetails/basePlanId fields. ExpiryTimeRaw is produced by the installed SDK's ExpiryTimeDateTimeOffset setter.
The test HTTP injection uses the installed Google.Apis 1.75.0 source commit's HttpClientFactory.CreateHandler. The SDK deserializes the mocked response and the production GooglePlayBillingService creates the handler input. Tests do not invent an entitled-plus-payment-failure PlaySubscriptionState fixture.
The Google state reference confirms the state strings. The notification reference confirms types 2, 3, 5, 6 and 10. The existing repository notification envelope is retained. This is unit coverage of adapter, handlers and status projection, not a claim of live notification delivery validation.
Test evidence
Before test or implementation changes:
dotnet test tests/Orbit.Application.Tests --filter FullyQualifiedName~HandlePlayNotificationCommandHandlerTestspassed all 25 tests with the defect present, including the unchanged Handle_ActiveState_GrantsPro.Before production changes, strengthened Handle_ActiveState_GrantsPro to derive active/grace states through the real SDK and adapter, then ran
dotnet test tests/Orbit.Application.Tests --filter FullyQualifiedName~Handle_ActiveState_GrantsPro. Observed 1 passing active case and 1 failing grace case: expected SubscriptionLapseReason.PaymentFailed, but found null. Preliminary fixture setup runs failed on missing packageName and SDK timestamp formatting; those setup issues were corrected before recording the intended regression failure.After the fix:
dotnet test tests/Orbit.Application.Tests --filter 'FullyQualifiedName~HandlePlayNotificationCommandHandlerTests|FullyQualifiedName~VerifyPlayPurchaseCommandHandlerTests'passed all 49 tests. The strengthened theory now also starts with a prior payment failure, checks recovery and stale notifications, and queries the actual client status projection. Additional cases cover hold, pause, verification refresh and Stripe ownership.dotnet test tests/Orbit.Domain.Tests --filter FullyQualifiedName~SetPlaySubscriptionpassed all 3 tests.dotnet build Orbit.slnxcompleted with 0 errors and 15 existing dependency/analyzer/obsolete-API warnings. No route, endpoint or module structure changed; generated OpenAPI output was unchanged.git diff --check, the dash gate over all nine changed files, and inspection for bare narration comments passed. Only the existing linked UTC pragma comments remain in the changed production files.After committing e60a32a,
dotnet testpassed all 6,068 tests with 0 failures and 0 skips: 32 analyzer, 565 domain, 3,276 application and 2,195 infrastructure. Existing Stripe tests passed unmodified. The working tree is clean.Deploy the API first through the normal release process. Per #449, the existing warning surface in orbit-ui-mobile PR #850 consumes the existing payment_failed value without a client contract change. No environment setting, secret, manual migration or backfill is required.