Skip to content

feat: Next generation Stripe billing integration#2161

Open
niemyjski wants to merge 50 commits intomainfrom
feature/next-stripe
Open

feat: Next generation Stripe billing integration#2161
niemyjski wants to merge 50 commits intomainfrom
feature/next-stripe

Conversation

@niemyjski
Copy link
Copy Markdown
Member

Summary

  • Adds new Stripe billing components: ChangePlanDialog and StripeProvider
  • Implements Stripe Checkout session creation via API
  • Adds billing features to organization API: getBilling, changePlan, getInvoices
  • Updates billing pages to use new components
  • Updates organization and invoice mappers
  • Adds Stripe integration plan document

Changes

  • New Svelte components for Stripe integration
  • New backend endpoints for billing management
  • Updated usage pages to show billing info

- Upgrade Stripe.net to v50.4.1 and update the backend to support modern PaymentMethods while maintaining legacy token compatibility.
- Implement a new billing feature in the Svelte UI with lazy-loaded Stripe integration and a functional plan change dialog.
- Add TanStack Query hooks for fetching available plans and processing plan changes with coupon support.
Copilot AI review requested due to automatic review settings March 17, 2026 03:14
@niemyjski niemyjski force-pushed the feature/next-stripe branch from 985add7 to ee79cb9 Compare March 17, 2026 03:15
@niemyjski niemyjski self-assigned this Mar 17, 2026
Comment thread src/Exceptionless.Web/Controllers/OrganizationController.cs Fixed
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a “next generation” Stripe billing integration spanning backend Stripe.net v50 updates, new billing endpoints/DTO mapping, and a new Svelte 5 billing UI (Stripe provider + change-plan dialog) wired into organization/project usage and billing pages.

Changes:

  • Upgrades Stripe.net usage on the backend (invoice status handling, discounts, subscription updates, PaymentMethod support).
  • Adds a new Svelte billing feature module (StripeProvider, ChangePlanDialog) and hooks it into usage/billing routes.
  • Extends org client API with plan query + change-plan mutation; updates invoice mapping/tests accordingly.

Reviewed changes

Copilot reviewed 18 out of 19 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/Exceptionless.Tests/Mapping/InvoiceMapperTests.cs Updates Stripe invoice test data to use Status instead of Paid.
tests/Exceptionless.Tests/Controllers/TokenControllerTests.cs Simplifies test comments (no functional change).
src/Exceptionless.Web/Mapping/InvoiceMapper.cs Maps “paid” via Invoice.Status string comparison.
src/Exceptionless.Web/Controllers/OrganizationController.cs Updates Stripe invoice retrieval/discount handling and modernizes change-plan flow to support PaymentMethods + Stripe.net 50 API changes.
src/Exceptionless.Web/ClientApp/src/routes/(app)/project/[projectId]/usage/+page.svelte “Change plan” now navigates to the org billing page.
src/Exceptionless.Web/ClientApp/src/routes/(app)/organization/[organizationId]/usage/+page.svelte “Change plan” now navigates to the org billing page.
src/Exceptionless.Web/ClientApp/src/routes/(app)/organization/[organizationId]/billing/+page.svelte Uses new billing module ChangePlanDialog and passes loaded org data into it.
src/Exceptionless.Web/ClientApp/src/lib/features/organizations/components/dialogs/change-plan-dialog.svelte Removes the legacy placeholder change-plan dialog component.
src/Exceptionless.Web/ClientApp/src/lib/features/organizations/api.svelte.ts Adds getPlansQuery() and changePlanMutation() + related query keys/types.
src/Exceptionless.Web/ClientApp/src/lib/features/billing/stripe.svelte.ts Adds Stripe context + lazy singleton loader utilities.
src/Exceptionless.Web/ClientApp/src/lib/features/billing/schemas.ts Adds zod schema/types for the change-plan form.
src/Exceptionless.Web/ClientApp/src/lib/features/billing/models.ts Adds billing model re-exports + local billing form/params types.
src/Exceptionless.Web/ClientApp/src/lib/features/billing/index.ts Barrel exports for billing feature module.
src/Exceptionless.Web/ClientApp/src/lib/features/billing/constants.ts Defines FREE_PLAN_ID.
src/Exceptionless.Web/ClientApp/src/lib/features/billing/components/stripe-provider.svelte Adds Stripe Elements provider wrapper with loading/error states.
src/Exceptionless.Web/ClientApp/src/lib/features/billing/components/change-plan-dialog.svelte Implements the plan change dialog UI including Stripe payment collection.
src/Exceptionless.Web/ClientApp/package.json Adds Stripe JS + svelte-stripe dependencies.
src/Exceptionless.Web/ClientApp/package-lock.json Locks Stripe JS + svelte-stripe dependencies.
src/Exceptionless.Web/ClientApp/STRIPE-INTEGRATION-PLAN.md Adds the Stripe integration plan document.
src/Exceptionless.Core/Exceptionless.Core.csproj Upgrades Stripe.net to 50.4.1.
.claude/agents/engineer.md Adds guidance for rerunning flaky CI jobs via gh run rerun.
Files not reviewed (1)
  • src/Exceptionless.Web/ClientApp/package-lock.json: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/Exceptionless.Web/Controllers/OrganizationController.cs Outdated
Comment thread src/Exceptionless.Web/Controllers/OrganizationController.cs Outdated
Copilot AI review requested due to automatic review settings April 16, 2026 04:09
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a “next generation” Stripe billing integration across the backend (Stripe.net 50.x) and the Svelte client, enabling plan selection/changes and invoice display with the updated Stripe API surface.

Changes:

  • Upgrades Stripe.net to 50.4.1 and updates invoice/plan-change logic to use Status, Price, Discounts, and PaymentMethod APIs.
  • Adds a new client-side billing feature module (Stripe context/provider + change plan dialog) and wires it into usage/billing pages.
  • Adds client API helpers for fetching plans and performing plan changes, with cache invalidation.

Reviewed changes

Copilot reviewed 18 out of 19 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
tests/Exceptionless.Tests/Mapping/InvoiceMapperTests.cs Updates invoice mapping tests to reflect Stripe Status replacing Paid.
tests/Exceptionless.Tests/Controllers/TokenControllerTests.cs Simplifies test comments (no functional change).
src/Exceptionless.Web/Mapping/InvoiceMapper.cs Switches paid logic to derive from Invoice.Status.
src/Exceptionless.Web/Controllers/OrganizationController.cs Updates invoice retrieval and plan change flows for Stripe.net 50.x (Price/Discounts/PaymentMethod changes).
src/Exceptionless.Web/ClientApp/src/routes/(app)/project/[projectId]/usage/+page.svelte Replaces placeholder “change plan” handler with navigation to billing page.
src/Exceptionless.Web/ClientApp/src/routes/(app)/organization/[organizationId]/usage/+page.svelte Same as above for org usage page.
src/Exceptionless.Web/ClientApp/src/routes/(app)/organization/[organizationId]/billing/+page.svelte Switches to the new billing module’s ChangePlanDialog and passes organization data.
src/Exceptionless.Web/ClientApp/src/lib/features/organizations/components/dialogs/change-plan-dialog.svelte Removes the old placeholder change-plan dialog component.
src/Exceptionless.Web/ClientApp/src/lib/features/organizations/api.svelte.ts Adds getPlansQuery + changePlanMutation and associated query keys/types.
src/Exceptionless.Web/ClientApp/src/lib/features/billing/stripe.svelte.ts Adds Stripe context utilities and a singleton Stripe loader.
src/Exceptionless.Web/ClientApp/src/lib/features/billing/schemas.ts Adds Zod schema for the change-plan form.
src/Exceptionless.Web/ClientApp/src/lib/features/billing/models.ts Adds billing-focused types and re-exports generated API models.
src/Exceptionless.Web/ClientApp/src/lib/features/billing/index.ts Exposes billing module public API (components/hooks/constants/types).
src/Exceptionless.Web/ClientApp/src/lib/features/billing/constants.ts Introduces FREE_PLAN_ID.
src/Exceptionless.Web/ClientApp/src/lib/features/billing/components/stripe-provider.svelte Adds a Stripe <Elements> provider and sets Stripe context for children.
src/Exceptionless.Web/ClientApp/src/lib/features/billing/components/change-plan-dialog.svelte Implements the new plan change UX using Stripe PaymentElement + plans query + mutation.
src/Exceptionless.Web/ClientApp/package.json Adds @stripe/stripe-js and svelte-stripe.
src/Exceptionless.Web/ClientApp/package-lock.json Locks new Stripe dependencies.
src/Exceptionless.Core/Exceptionless.Core.csproj Upgrades Stripe.net from 47.4.0 to 50.4.1.
Files not reviewed (1)
  • src/Exceptionless.Web/ClientApp/package-lock.json: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/Exceptionless.Web/Mapping/InvoiceMapper.cs Outdated
Comment thread src/Exceptionless.Web/Controllers/OrganizationController.cs Outdated
Comment thread src/Exceptionless.Web/Controllers/OrganizationController.cs Outdated
Comment thread src/Exceptionless.Web/Controllers/OrganizationController.cs
Comment thread src/Exceptionless.Web/Controllers/OrganizationController.cs
Comment thread src/Exceptionless.Web/ClientApp/src/lib/features/billing/stripe.svelte.ts Outdated
Comment thread src/Exceptionless.Web/Controllers/OrganizationController.cs
Copilot AI review requested due to automatic review settings April 16, 2026 05:03
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a new Stripe-based billing UX in the Svelte app and updates the backend Stripe integration to newer Stripe.net APIs, alongside some infra/telemetry adjustments.

Changes:

  • Adds new billing feature module on the frontend (Stripe provider + change-plan dialog) and wires it into usage/billing pages.
  • Updates backend invoice/payment handling to Stripe.net 50.x patterns (invoice status, discounts, line item pricing/price lookup).
  • Updates dependencies/config (Stripe.net upgrade, adds @stripe/stripe-js + svelte-stripe, adjusts OpenTelemetry Prometheus wiring and deploy workflow conditions).

Reviewed changes

Copilot reviewed 25 out of 26 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
tests/Exceptionless.Tests/Mapping/InvoiceMapperTests.cs Updates invoice mapping tests to use Status instead of Paid.
tests/Exceptionless.Tests/Controllers/TokenControllerTests.cs Simplifies test comments (no functional change).
src/Exceptionless.Web/Startup.cs Removes Prometheus scraping endpoint middleware.
src/Exceptionless.Web/Mapping/InvoiceMapper.cs Maps Paid from Invoice.Status == "paid".
src/Exceptionless.Web/Exceptionless.Web.csproj Removes Prometheus exporter package reference; formatting changes.
src/Exceptionless.Web/Controllers/OrganizationController.cs Updates invoice and plan-change flows for Stripe.net 50.x (prices/discounts/payment methods).
src/Exceptionless.Web/ClientApp/src/routes/(app)/project/[projectId]/usage/+page.svelte Navigates to org billing page with changePlan=true.
src/Exceptionless.Web/ClientApp/src/routes/(app)/organization/[organizationId]/usage/+page.svelte Navigates to org billing page with changePlan=true.
src/Exceptionless.Web/ClientApp/src/routes/(app)/organization/[organizationId]/billing/+page.svelte Switches to new billing ChangePlanDialog and passes organization data.
src/Exceptionless.Web/ClientApp/src/lib/features/organizations/components/dialogs/change-plan-dialog.svelte Deletes placeholder dialog (replaced by billing feature).
src/Exceptionless.Web/ClientApp/src/lib/features/organizations/api.svelte.ts Adds plans query + change-plan mutation/query keys.
src/Exceptionless.Web/ClientApp/src/lib/features/billing/stripe.svelte.ts Adds Stripe context + lazy loader utilities.
src/Exceptionless.Web/ClientApp/src/lib/features/billing/schemas.ts Adds Zod schema for change-plan form.
src/Exceptionless.Web/ClientApp/src/lib/features/billing/models.ts Adds billing types and form state types.
src/Exceptionless.Web/ClientApp/src/lib/features/billing/index.ts Adds billing feature barrel exports.
src/Exceptionless.Web/ClientApp/src/lib/features/billing/constants.ts Adds FREE_PLAN_ID constant.
src/Exceptionless.Web/ClientApp/src/lib/features/billing/components/stripe-provider.svelte Adds Stripe Elements provider component.
src/Exceptionless.Web/ClientApp/src/lib/features/billing/components/change-plan-dialog.svelte Adds new Stripe-backed change-plan dialog UI.
src/Exceptionless.Web/ClientApp/package.json Adds Stripe JS dependencies.
src/Exceptionless.Web/ClientApp/package-lock.json Locks Stripe JS dependencies.
src/Exceptionless.Web/ApmExtensions.cs Removes Prometheus exporter from OpenTelemetry metrics pipeline.
src/Exceptionless.Job/Program.cs Removes Prometheus scraping endpoint middleware from job host.
src/Exceptionless.Job/Exceptionless.Job.csproj Removes Prometheus exporter package reference; formatting changes.
src/Exceptionless.Core/Exceptionless.Core.csproj Upgrades Stripe.net from 47.4.0 to 50.4.1; formatting changes.
.vscode/settings.json Updates workspace editor/TypeScript SDK settings.
.github/workflows/build.yaml Updates deploy job condition to allow dev deploys from a configured PR branch.
Files not reviewed (1)
  • src/Exceptionless.Web/ClientApp/package-lock.json: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/Exceptionless.Job/Program.cs Outdated
Comment thread src/Exceptionless.Web/Controllers/OrganizationController.cs Outdated
Comment thread src/Exceptionless.Web/Controllers/OrganizationController.cs Outdated
Comment thread src/Exceptionless.Web/Startup.cs
Comment thread src/Exceptionless.Web/ApmExtensions.cs
Comment thread src/Exceptionless.Web/Controllers/OrganizationController.cs
Comment thread src/Exceptionless.Web/Controllers/OrganizationController.cs Outdated
Copilot AI review requested due to automatic review settings April 16, 2026 15:28
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a next-generation Stripe-based billing flow across the Svelte app and the organization API, updating both frontend plan-change UX and backend Stripe integration to newer Stripe.net APIs.

Changes:

  • Adds a new Billing feature module (Stripe context/provider + ChangePlanDialog) and wires it into the organization billing page and usage pages.
  • Updates backend Stripe integration (Stripe.net 50.x) for invoice/payment/plan handling and updates invoice mapping/tests accordingly.
  • Updates build/ops tooling (Docker MinVer build arg, workflow tweaks) and removes Prometheus OpenTelemetry exporter/scraping.

Reviewed changes

Copilot reviewed 26 out of 27 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
tests/Exceptionless.Tests/Mapping/InvoiceMapperTests.cs Updates tests to reflect invoice Status -> paid mapping.
tests/Exceptionless.Tests/Controllers/TokenControllerTests.cs Simplifies test comments (no behavior change).
src/Exceptionless.Web/Startup.cs Removes OTEL Prometheus scraping endpoint middleware.
src/Exceptionless.Web/Mapping/InvoiceMapper.cs Maps invoice “paid” from Stripe Status instead of Paid.
src/Exceptionless.Web/Exceptionless.Web.csproj Removes Prometheus exporter package + formatting changes.
src/Exceptionless.Web/Controllers/OrganizationController.cs Updates Stripe invoice/plan change logic for Stripe.net 50.x and PaymentMethod support.
src/Exceptionless.Web/ClientApp/src/routes/(app)/project/[projectId]/usage/+page.svelte Implements navigation to billing page on “Change plan”.
src/Exceptionless.Web/ClientApp/src/routes/(app)/organization/[organizationId]/usage/+page.svelte Implements navigation to billing page on “Change plan”.
src/Exceptionless.Web/ClientApp/src/routes/(app)/organization/[organizationId]/billing/+page.svelte Switches to new billing ChangePlanDialog component and passes organization data.
src/Exceptionless.Web/ClientApp/src/lib/features/organizations/components/dialogs/change-plan-dialog.svelte Removes placeholder dialog from organizations feature.
src/Exceptionless.Web/ClientApp/src/lib/features/organizations/api.svelte.ts Adds plans query + change-plan mutation and query keys.
src/Exceptionless.Web/ClientApp/src/lib/features/billing/stripe.svelte.ts Adds Stripe lazy-loader + context hooks.
src/Exceptionless.Web/ClientApp/src/lib/features/billing/schemas.ts Adds Zod schema for change-plan form.
src/Exceptionless.Web/ClientApp/src/lib/features/billing/models.ts Adds billing feature types and re-exports generated API types.
src/Exceptionless.Web/ClientApp/src/lib/features/billing/index.ts Public entrypoint for billing feature exports.
src/Exceptionless.Web/ClientApp/src/lib/features/billing/constants.ts Adds FREE plan id constant.
src/Exceptionless.Web/ClientApp/src/lib/features/billing/components/stripe-provider.svelte Adds provider wrapping Stripe Elements + loading/error UI.
src/Exceptionless.Web/ClientApp/src/lib/features/billing/components/change-plan-dialog.svelte Adds full Stripe Elements-based plan change dialog.
src/Exceptionless.Web/ClientApp/package.json Adds Stripe JS dependencies.
src/Exceptionless.Web/ClientApp/package-lock.json Locks Stripe JS dependencies.
src/Exceptionless.Web/ApmExtensions.cs Removes Prometheus exporter wiring.
src/Exceptionless.Job/Program.cs Removes OTEL Prometheus scraping endpoint middleware.
src/Exceptionless.Job/Exceptionless.Job.csproj Removes Prometheus exporter package + formatting changes.
src/Exceptionless.Core/Exceptionless.Core.csproj Upgrades Stripe.net to 50.4.1.
Dockerfile Adds MinVerVersionOverride build arg passthrough.
.vscode/settings.json Disables format-on-save and changes TypeScript SDK path setting key.
.github/workflows/build.yaml Ensures checkout uses correct ref and passes MinVer build arg into Docker builds; expands deploy conditions.
Files not reviewed (1)
  • src/Exceptionless.Web/ClientApp/package-lock.json: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .vscode/settings.json Outdated
Comment thread src/Exceptionless.Web/Controllers/OrganizationController.cs Outdated
Comment thread src/Exceptionless.Web/ApmExtensions.cs
Comment thread src/Exceptionless.Web/ClientApp/src/lib/features/organizations/api.svelte.ts Outdated
Comment thread src/Exceptionless.Web/ClientApp/src/lib/features/organizations/api.svelte.ts Outdated
niemyjski and others added 5 commits April 18, 2026 10:22
Stripe v9 requires either clientSecret OR mode when calling stripe.elements().
The previous code passed neither when no clientSecret was provided, causing
PaymentElement to fail to render.

Uses a discriminated union Props type to enforce mutual exclusivity of
clientSecret and mode at compile time, matching Stripe's own type constraints.
Defaults to mode='setup' for collecting payment methods for future use.
Fix two issues preventing the Stripe PaymentElement from rendering in
the Change Plan dialog:

1. Missing currency in Stripe Elements options: Stripe.js v9+ requires
   a currency string when using mode: 'setup'. Added currency: 'usd'
   to the elements creation options.

2. svelte-stripe Svelte 5 incompatibility: svelte-stripe's <Elements>
   and <PaymentElement> components use $bindable()/onMount patterns
   that don't trigger Svelte 5 template re-renders from async callbacks.
   Replaced both components with an imperative approach that loads Stripe,
   creates elements, and mounts the PaymentElement directly to the DOM
   via onMount, bypassing Svelte's reactive template system entirely.

Additionally fixed the Change Plan dialog not being scrollable by adding
max-h-[90vh] and overflow-y-auto to the Dialog.Content wrapper.

Changes:
- stripe-provider.svelte: Rewritten to imperatively mount PaymentElement
  without svelte-stripe components or Svelte reactive conditionals
- change-plan-dialog.svelte: Removed svelte-stripe PaymentElement import,
  updated StripeProvider usage (no longer takes children), added dialog
  scroll constraints, removed debug markup

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
var paymentMethodService = new PaymentMethodService(client);

// Detect if stripeToken is a legacy token (tok_) or modern PaymentMethod (pm_)
bool isPaymentMethod = model.StripeToken?.StartsWith("pm_", StringComparison.Ordinal) == true;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Suggested change
bool isPaymentMethod = model.StripeToken?.StartsWith("pm_", StringComparison.Ordinal) == true;
bool isPaymentMethod = model.StripeToken?.StartsWith("pm_", StringComparison.Ordinal) is true;

var paymentMethodService = new PaymentMethodService(client);

// Detect if stripeToken is a legacy token (tok_) or modern PaymentMethod (pm_)
bool isPaymentMethod = model.StripeToken?.StartsWith("pm_", StringComparison.Ordinal) == true;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

can you verify if we really do send pm_ scoped tokens, I never remember seeing this ever being _tok, but I take your word for it.


var customer = await customerService.CreateAsync(createCustomer);

// Create subscription separately (Plan on CustomerCreateOptions is deprecated)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

no need to talk about deprecations

}
catch (Exception ex)
{
_logger.LogCritical(ex, "An unexpected error occurred while trying to update your billing plan");
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Suggested change
_logger.LogCritical(ex, "An unexpected error occurred while trying to update your billing plan");
_logger.LogCritical(ex, "Error occurred update billing plan: {Message}", ex.Message);


namespace Exceptionless.Web.Models;

public class ChangePlanRequest
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Suggested change
public class ChangePlanRequest
public record ChangePlanRequest

// Assert
Assert.NotNull(plans);
Assert.True(plans.Count > 0);
var unlimitedPlan = plans.FirstOrDefault(p => p.Id == _plans.UnlimitedPlan.Id);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

use String.Equal, SingleOrDefault, check all new tests

Assert.Null(token.UserId);
Assert.False(token.IsDisabled);
Assert.Equal(2, token.Scopes.Count);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

revert, newlines are great

@@ -367,8 +375,7 @@ public async Task PostAsync_WithProjectFromUnauthorizedOrganization_ReturnsValid
[Fact]
public async Task PostAsync_WithMismatchedOrgAndProjectId_UsesProjectOrganization()
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Don't use Org abbreviation

Comment on lines +200 to +202
if (clientResponse.problem) {
handleUpgradeRequired(clientResponse.problem, organization.current, () => loadData());
}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Any reason we just wouldn't call this, and this would noop if there is no problem? It would save a lot of boilerplate, name could be better, looking at this usage I don't know what it does, does it do showBillingDialogOnUpgradeProblem? or some really descriptive like that?

{/snippet}
</IntercomShell>

<UpgradeRequiredDialog />
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

conditionally show this so it's not always rendered?

console.log('Change plan clicked');
async function handleChangePlan() {
if (organization.current) {
await goto(`/next/organization/${organization.current}/billing?changePlan=true`);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

why not just show the plan change dialog?

}

if (error instanceof ProblemDetails) {
toastId = toast.error(error.title || 'Error creating organization. Please try again.');
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

seems like we have a lot of duplicate strings for toast messages, perhaps create a const once locally so we can fallback easier and bundles are smaller

// This is a placeholder for future implementation
console.log('Change plan clicked');
async function handleChangePlan() {
await goto(`/next/organization/${page.params.organizationId}/billing?changePlan=true`);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

same as my other comment, please check comments

Comment on lines -31 to +32
// TODO: Show a message to the user that they need to upgrade their subscription.
if (handleUpgradeRequired(problem, organization.current)) {
await goto(resolve('/(app)'));
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

what does the legacy app do? also shouldn't we be displaying a toast or action for upgrade or something

console.log('TODO: Upgrade to premium features');
function handleUpgrade() {
if (selectedProject?.organization_id) {
showUpgradeDialog(selectedProject.organization_id, 'Please upgrade your plan to enable occurrence level notifications.');
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

when this returns, is anything triggered to retry?

Comment on lines -168 to -174
.nullable()
.optional(),
.regex(/^[a-fA-F0-9]{24}$/, "Organization id has invalid format"),
project_id: string()
.length(24, "Project id must be exactly 24 characters")
.regex(/^[a-fA-F0-9]{24}$/, "Project id has invalid format")
.nullable()
.optional(),
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

RCA on why these changed

Comment on lines -76 to -78
toast.error(
'Promote to External is a premium feature used to promote an error stack to an external system. Please upgrade your plan to enable this feature.'
);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

what happend to these very specific messages, are they coming from the backend?

// Invalidate organization data to reflect new plan
// WebSocket OrganizationChanged also fires, but we invalidate here for immediate feedback
queryClient.invalidateQueries({ queryKey: queryKeys.id(request.route.organizationId, undefined) });
queryClient.invalidateQueries({ queryKey: queryKeys.id(request.route.organizationId, 'stats') });
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

why would this need to be invalidated, the previous line I think would invalidate this too.

* @param retryCallback - Optional callback invoked after a successful upgrade to retry the failed operation.
* Returns true if the error was a 426 and was handled, false otherwise.
*/
export function handleUpgradeRequired(error: unknown, organizationId: string | undefined, retryCallback?: () => Promise<void> | void): boolean {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

should this take an optional message?

if (!ctx) {
throw new Error('useStripe() must be called within a StripeProvider component');
}
return ctx;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

remember, always have a new line after } or return.


export { default as StripeProvider } from './components/stripe-provider.svelte';

// Upgrade required handling
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Suggested change
// Upgrade required handling


// Components
export { default as ChangePlanDialog } from './components/change-plan-dialog.svelte';

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Suggested change

Comment on lines +15 to +18
// Models
export type { BillingPlan, CardMode, ChangePlanFormState, ChangePlanRequest, ChangePlanResult } from './models';

// Context and hooks
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

these are stupid comments

<div class="flex items-baseline gap-2 text-xs leading-snug">
<Muted class="min-w-16 text-[10px] font-semibold tracking-wider uppercase">Coupon</Muted>
<Muted class="text-xs">
<strong class="text-foreground font-mono font-medium">{couponApplied}</strong> applied
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

look for usages of we should be using typography components where possible. Also seems odd we would use muted and then strong no.

· immediate, prorated credit
{:else if organization.plan_id === FREE_PLAN_ID}
Start <strong class="text-foreground font-medium">{planLabel(selectedPlanId, { includeInterval: true })}</strong>
{#if selectedPlan}· ${formatPrice(selectedPlan.price)}{interval === 'year' ? '/yr' : '/mo'}{/if}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Price formatter component, interval formatter component, if we don't have them, create one.

<Alert.Root variant="success" class="flex items-center gap-2.5 py-2.5">
<Check class="size-4" />
<Alert.Description class="flex flex-1 items-center gap-2">
<span class="font-mono text-xs font-semibold">{couponApplied}</span>
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

check all classes against typography components, if one matches use typography component.

Comment on lines +402 to +404
function formatPrice(n: number): string {
return n.toLocaleString('en-US');
}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

never should have this, always use a CurrencyFormatter

Comment on lines +296 to +298
$effect(() => {
if (plansQuery.data) {
untrack(() => {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

things like this always feels like a massive hack, anything with untrack and effect.

Comment on lines +271 to +275
await Exceptionless.createException(error instanceof Error ? error : new Error(String(error)))
.setProperty('organizationId', organization.id)
.setProperty('selectedPlanId', value.selectedPlanId)
.addTags('billing', 'change-plan')
.submit();
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Feels like we would have a more common way to log errors and error messages. for now this works, but we should add a todo in a markdown file someplace.

@@ -0,0 +1,849 @@
<script lang="ts">
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

File is massive but works, wish we had more composite components here.

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

Development

Successfully merging this pull request may close these issues.

4 participants