Skip to content

[PM-38574] feat: Set trialInitiationPath for sales-assisted trials - #8272

Open
cyprain-okeke wants to merge 5 commits into
mainfrom
billing/pm-38574/set-trial-initiation-path-for-sales-assisted-trials
Open

[PM-38574] feat: Set trialInitiationPath for sales-assisted trials#8272
cyprain-okeke wants to merge 5 commits into
mainfrom
billing/pm-38574/set-trial-initiation-path-for-sales-assisted-trials

Conversation

@cyprain-okeke

@cyprain-okeke cyprain-okeke commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-38574

📔 Objective

Sales-assisted trial organizations (PM-38388/PM-38389) currently fall through to marketing-initiated in Stripe subscription metadata, so the warnings API cannot distinguish invoice-billed prospects from self-serve trials.

  • Map the client's new InitiationPath value ("Sales assisted trial from admin portal") to trialInitiationPath = "sales-assisted" in subscription metadata; the existing marketing-initiated / product-initiated values are promoted to StripeConstants.MetadataKeys alongside it.
  • Add IsSalesAssisted to FreeTrialWarning, populated from subscription metadata in GetOrganizationWarningsQuery. PM-38575 (hide payment modal) and PM-38885 (banner copy) consume this on the client.
  • New theory test pins the initiation-path → metadata mapping with literal wire strings, since the path sentence must byte-match the clients-repo InitiationPath enum and the metadata values persist in Stripe.

Notes for reviewers:

  • The ticket AC says sales-initiated; Product confirmed that is a typo — sales-assisted is the intended value.
  • The initiation path remains client-asserted at organization creation; validating the sales-assisted registration token on this path is a known follow-up (ticket to be filed).
  • Merge order with the clients PR is flexible: the client defaults isSalesAssisted to false when the field is absent.

Sibling PR: bitwarden/clients#22757

Sales-assisted trial orgs previously fell through to
"marketing-initiated" in Stripe subscription metadata, leaving the
warnings API unable to distinguish invoice-billed prospects. Map the
sales-assisted InitiationPath sent by the client to a dedicated
"sales-assisted" metadata value and expose IsSalesAssisted on
FreeTrialWarning for PM-38575/PM-38885 to consume.
@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the trialInitiationPath switch expression in OrganizationBillingService.CreateSubscriptionAsync, the new IsSalesAssisted flag on FreeTrialWarning and its read in GetOrganizationWarningsQuery, the new StripeConstants.TrialInitiationPaths value class, and the two new test theories. Behavior for pre-existing inputs is preserved: the guarded marketing-initiated arm is evaluated first, its when clause keeps the later arms reachable, and "Sales assisted trial from admin portal" does not contain "trial from marketing website", so only the new exact-match arm changes what a sale produces. Both prior review threads are addressed by the head of the branch — the metadata values now live in a sibling TrialInitiationPaths class matching the PurchasableReferences / CancellationOrigins convention, and the two initiation-path sentences are private const fields rather than inline literals. subscription.Metadata is read without a null guard, matching the unguarded reads in SubscriberId.cs:30 and SubscriberService.cs:235, and OrganizationWarnings is serialized directly by OrganizationBillingVNextController, so the new field reaches the client without a response-model change.

Code Review Details

No findings. InitiationPath remains client-asserted at organization creation, but the new flag only drives client-side warning copy — the trial still carries trial_settings.end_behavior.missing_payment_method = cancel whenever no payment method is present, so no server-side entitlement depends on it, and the PR description already records token validation on this path as a tracked follow-up.

@cyprain-okeke cyprain-okeke added the t:feature Change Type - Feature Development label Aug 27, 2026
Comment on lines +114 to +116
public const string TrialInitiationPathMarketingInitiated = "marketing-initiated";
public const string TrialInitiationPathProductInitiated = "product-initiated";
public const string TrialInitiationPathSalesAssisted = "sales-assisted";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ DEBT: These are metadata values, not keys — this file already has a convention for that.

Details and fix

MetadataKeys holds Stripe metadata key names. The values carried under a key live in their own sibling class in this same file — see PurchasableReferences (documented as "the stable, plan-agnostic values carried in MetadataKeys.PurchasableReference") and CancellationOrigins for MetadataKeys.CancellationOrigin.

/// <summary>
/// The values carried in <see cref="MetadataKeys.TrialInitiationPath"/>.
/// </summary>
public static class TrialInitiationPaths
{
    public const string MarketingInitiated = "marketing-initiated";
    public const string ProductInitiated = "product-initiated";
    public const string SalesAssisted = "sales-assisted";
}

That also reads better at the call sites, where trialInitiationPath == MetadataKeys.TrialInitiationPathSalesAssisted currently compares a value against something named as a key.

@codecov

codecov Bot commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.67%. Comparing base (0c32f30) to head (3dd6310).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8272   +/-   ##
=======================================
  Coverage   63.66%   63.67%           
=======================================
  Files        2467     2467           
  Lines      105693   105702    +9     
  Branches     9551     9554    +3     
=======================================
+ Hits        67294    67304   +10     
+ Misses      36083    36082    -1     
  Partials     2316     2316           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@cyprain-okeke
cyprain-okeke marked this pull request as ready for review August 27, 2026 17:12
@cyprain-okeke
cyprain-okeke requested a review from a team as a code owner August 27, 2026 17:12
Comment thread src/Core/Billing/Organizations/Services/OrganizationBillingService.cs Outdated

@sbrown-livefront sbrown-livefront left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great refactor with the other magic strings. Thanks! ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review t:feature Change Type - Feature Development

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants