mirror of
https://github.com/arsvendg/Stirling-PDF.git
synced 2026-07-14 10:34:06 +02:00
fix(payg): cancelled subscription left team gated as subscribed (#6611)
## Problem
A team that **cancelled** its PAYG subscription kept full subscribed
access:
- **UI didn't reflect cancellation** — the Plan tab still rendered the
subscribed view, never the free/upgrade view.
- **Automation wasn't stopped** — automation / AI / API kept running
without ever falling back to the free-grant gate.
## Root cause
`TeamBillingService.compute` decided `subscribed` as:
```java
boolean subscribed =
subscriptionId != null
|| extOpt.map(PaygTeamExtensions::getStripeCustomerId).filter(s -> !s.isBlank()).isPresent();
```
On cancellation, the `customer.subscription.deleted` webhook calls
`payg_unlink_subscription`, which nulls `payg_subscription_id` but
**deliberately keeps `stripe_customer_id`** (so a future re-subscribe
can reuse the Stripe customer).
`payg_link_subscription` is the **only** writer of
`payg_team_extensions.stripe_customer_id`, and it writes it in the
*same* `UPDATE` as `payg_subscription_id` (on
`customer.subscription.created`). So the customer id is never set before
the subscription id — the "pre-webhook stand-in" the old comment claimed
**cannot happen**. The fallback only ever pinned a team that *ever*
subscribed to `subscribed` forever, because the Stripe customer outlives
the subscription.
Both symptoms are this one flag:
- `PaygWalletController` status → `SUBSCRIBED` vs `FREE`
- `EntitlementService` gate branch → monthly-cap vs free-grant
## Fix
Gate `subscribed` purely on `payg_subscription_id != null`. A cancelled
team now correctly drops to free (UI shows free; billable ops gate on
the one-time grant). This aligns the wallet/entitlement read with the
**meter path** (`JobChargeService.close`), which already gated on
`payg_subscription_id`.
Handles both Stripe cancel modes: "cancel at period end" keeps the sub
`active` (id stays set) until `.deleted` fires at period end → access
through the paid period; immediate cancel fires `.deleted` now → flips
to free now.
**No data migration / backfill** — already-cancelled teams have
`payg_subscription_id = NULL`, so they flip to free as soon as this
ships (within the 30s billing-cache TTL).
## Tests
Adds `TeamBillingServiceTest` — the `subscribed` computation previously
had **no** unit coverage (which is how this shipped). Covers: subscribed
iff subscription id present; **cancelled team (customer id remains,
subscription id null) ≠ subscribed** + free grant survives;
no-subscription/no-customer; no extension row.
`:saas:test` + `:saas:spotlessCheck` green; coverage gates met.
## Not included (optional hardening, can fast-follow)
- Cross-check the synced `stripe.subscriptions.status` to guard a
*missed* `.deleted` webhook leaving `payg_subscription_id` stale.
- Push cache-invalidation from the webhook (currently ≤30s TTL
staleness).
This commit is contained in:
@@ -16,8 +16,8 @@ import java.time.LocalDateTime;
|
||||
* + cap only.
|
||||
* </ul>
|
||||
*
|
||||
* @param subscribed team has an active PAYG subscription (or a Stripe customer awaiting its
|
||||
* subscription-created webhook)
|
||||
* @param subscribed team has a live PAYG subscription — i.e. {@code payg_subscription_id} is set.
|
||||
* Cleared by {@code payg_unlink_subscription} on cancellation, so a cancelled team reads false.
|
||||
* @param subscriptionId {@code payg_team_extensions.payg_subscription_id}; null when free
|
||||
* @param periodStart inclusive start of the monthly billing window — the Stripe subscription's
|
||||
* current period when subscribed, calendar month otherwise
|
||||
|
||||
@@ -111,14 +111,19 @@ public class TeamBillingService {
|
||||
Optional<WalletPolicy> walletPolicyOpt = walletPolicyRepository.findByTeamId(teamId);
|
||||
|
||||
String subscriptionId = extOpt.map(PaygTeamExtensions::getPaygSubscriptionId).orElse(null);
|
||||
// payg_subscription_id is the designed switch; stripe_customer_id presence is the
|
||||
// pre-webhook stand-in kept so a team whose checkout completed but whose
|
||||
// subscription-created webhook hasn't landed yet still renders as subscribed.
|
||||
boolean subscribed =
|
||||
subscriptionId != null
|
||||
|| extOpt.map(PaygTeamExtensions::getStripeCustomerId)
|
||||
.filter(s -> !s.isBlank())
|
||||
.isPresent();
|
||||
// payg_subscription_id is the single subscription switch. payg_link_subscription sets it
|
||||
// (alongside stripe_customer_id, in the same write) on customer.subscription.created;
|
||||
// payg_unlink_subscription nulls it on customer.subscription.deleted while deliberately
|
||||
// keeping stripe_customer_id so a future re-subscribe can reuse the Stripe customer. So a
|
||||
// cancelled team has a null subscription id and must read as free again.
|
||||
//
|
||||
// We deliberately do NOT fall back to stripe_customer_id presence. payg_link_subscription
|
||||
// is the only writer of that column and it writes it together with the subscription id, so
|
||||
// it can never be set "before the webhook lands" — there is no gap for it to bridge. A
|
||||
// customer-id fallback would instead keep every team that ever subscribed pinned to
|
||||
// subscribed forever (the customer outlives the subscription), which is the cancelled-team
|
||||
// bug this guards against.
|
||||
boolean subscribed = subscriptionId != null;
|
||||
|
||||
long freeGrant = resolveGrant(teamId);
|
||||
long freeRemaining =
|
||||
|
||||
+121
@@ -0,0 +1,121 @@
|
||||
package stirling.software.saas.payg.billing;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.mockito.Mockito.when;
|
||||
|
||||
import java.util.Optional;
|
||||
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.mockito.Mockito;
|
||||
|
||||
import stirling.software.saas.payg.policy.PaygTeamExtensions;
|
||||
import stirling.software.saas.payg.policy.PricingPolicy;
|
||||
import stirling.software.saas.payg.policy.PricingPolicyService;
|
||||
import stirling.software.saas.payg.repository.PaygTeamExtensionsRepository;
|
||||
import stirling.software.saas.payg.repository.WalletPolicyRepository;
|
||||
import stirling.software.saas.payg.stripe.StripeSubscriptionDao;
|
||||
|
||||
/**
|
||||
* Unit tests for {@link TeamBillingService#forTeam(Long)} — specifically the {@code subscribed}
|
||||
* determination, which is the single switch both the wallet UI ({@code status}) and the entitlement
|
||||
* gate read.
|
||||
*
|
||||
* <p>Regression focus: a team is subscribed iff {@code payg_subscription_id} is set. {@code
|
||||
* payg_unlink_subscription} nulls that column on {@code customer.subscription.deleted} but
|
||||
* deliberately keeps {@code stripe_customer_id} (for a future re-subscribe), so a cancelled team
|
||||
* must read as free again. An earlier fallback treated <em>customer-id presence</em> as subscribed,
|
||||
* which kept every team that ever subscribed pinned to subscribed forever — the cancelled-team bug
|
||||
* these tests lock down.
|
||||
*/
|
||||
class TeamBillingServiceTest {
|
||||
|
||||
private static final long TEAM_ID = 100L;
|
||||
|
||||
private PaygTeamExtensionsRepository extensionsRepository;
|
||||
private WalletPolicyRepository walletPolicyRepository;
|
||||
private PricingPolicyService pricingPolicyService;
|
||||
private StripeSubscriptionDao subscriptionDao;
|
||||
private TeamBillingService service;
|
||||
|
||||
@BeforeEach
|
||||
void setUp() {
|
||||
extensionsRepository = Mockito.mock(PaygTeamExtensionsRepository.class);
|
||||
walletPolicyRepository = Mockito.mock(WalletPolicyRepository.class);
|
||||
pricingPolicyService = Mockito.mock(PricingPolicyService.class);
|
||||
subscriptionDao = Mockito.mock(StripeSubscriptionDao.class);
|
||||
service =
|
||||
new TeamBillingService(
|
||||
extensionsRepository,
|
||||
walletPolicyRepository,
|
||||
pricingPolicyService,
|
||||
subscriptionDao);
|
||||
|
||||
// Default grant so the free-tier fields are populated; individual tests don't depend on it
|
||||
// beyond the cancelled-team case below, which asserts the grant survives.
|
||||
PricingPolicy policy = Mockito.mock(PricingPolicy.class);
|
||||
when(policy.getFreeTierUnits()).thenReturn(500L);
|
||||
when(pricingPolicyService.getEffectivePolicy(TEAM_ID)).thenReturn(policy);
|
||||
}
|
||||
|
||||
private PaygTeamExtensions ext(String subscriptionId, String customerId, long freeRemaining) {
|
||||
PaygTeamExtensions ext = new PaygTeamExtensions();
|
||||
ext.setTeamId(TEAM_ID);
|
||||
ext.setPaygSubscriptionId(subscriptionId);
|
||||
ext.setStripeCustomerId(customerId);
|
||||
ext.setFreeUnitsRemaining(freeRemaining);
|
||||
return ext;
|
||||
}
|
||||
|
||||
@Test
|
||||
void subscribed_whenSubscriptionIdPresent() {
|
||||
when(extensionsRepository.findById(TEAM_ID))
|
||||
.thenReturn(Optional.of(ext("sub_123", "cus_123", 0L)));
|
||||
|
||||
TeamBillingContext ctx = service.forTeam(TEAM_ID);
|
||||
|
||||
assertThat(ctx.subscribed()).isTrue();
|
||||
assertThat(ctx.subscriptionId()).isEqualTo("sub_123");
|
||||
}
|
||||
|
||||
/**
|
||||
* The cancelled-subscription regression: after {@code payg_unlink_subscription} the
|
||||
* subscription id is null but the Stripe customer id remains. The team must read as NOT
|
||||
* subscribed (drops to the free-grant gate), and must still surface its remaining free grant.
|
||||
*/
|
||||
@Test
|
||||
void notSubscribed_afterCancellation_whenOnlyCustomerIdRemains() {
|
||||
when(extensionsRepository.findById(TEAM_ID))
|
||||
.thenReturn(Optional.of(ext(null, "cus_123", 120L)));
|
||||
|
||||
TeamBillingContext ctx = service.forTeam(TEAM_ID);
|
||||
|
||||
assertThat(ctx.subscribed()).isFalse();
|
||||
assertThat(ctx.subscriptionId()).isNull();
|
||||
// The free grant survives cancellation and is what now gates the team.
|
||||
assertThat(ctx.freeGrantUnits()).isEqualTo(500L);
|
||||
assertThat(ctx.freeRemainingUnits()).isEqualTo(120L);
|
||||
// Not subscribed → no monthly paid-doc cap.
|
||||
assertThat(ctx.monthlyCapDocUnits()).isNull();
|
||||
}
|
||||
|
||||
@Test
|
||||
void notSubscribed_whenNoSubscriptionAndNoCustomer() {
|
||||
when(extensionsRepository.findById(TEAM_ID)).thenReturn(Optional.of(ext(null, null, 500L)));
|
||||
|
||||
TeamBillingContext ctx = service.forTeam(TEAM_ID);
|
||||
|
||||
assertThat(ctx.subscribed()).isFalse();
|
||||
assertThat(ctx.subscriptionId()).isNull();
|
||||
}
|
||||
|
||||
@Test
|
||||
void notSubscribed_whenNoExtensionRow() {
|
||||
when(extensionsRepository.findById(TEAM_ID)).thenReturn(Optional.empty());
|
||||
|
||||
TeamBillingContext ctx = service.forTeam(TEAM_ID);
|
||||
|
||||
assertThat(ctx.subscribed()).isFalse();
|
||||
assertThat(ctx.subscriptionId()).isNull();
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user