Add Subscriptions - #6
Conversation
|
This pull request has merge conflicts. Please resolve those before requesting a review. |
Pass TestContext.Current.CancellationToken to CancellationToken-accepting calls in the taxation tests to satisfy the xUnit1051 analyzer, matching the repository test convention. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Wire the Taxation framework into the subscription checkout (invoice) flow as the single authoritative tax engine, consumed through an optional feature seam: - Add ISubscriptionTaxService seam with a no-op NullSubscriptionTaxService default so subscriptions keep working when Taxation is disabled, and a taxation-aware SubscriptionTaxService registered only via a [RequireFeatures(Taxation)] Startup. - Add SubscriptionTaxContextFactory to translate invoice line items into a deterministic TaxCalculationContext (checkout + recurring), excluding delayed subscription lines that are not due now. - Add SubscriptionTaxProfile + ISubscriptionTaxProfileProvider to resolve origin/destination/customer/classification, reusing the framework's models. - Persist tax on the Invoice (TaxAmount, TaxLines, immutable TaxSnapshot) and on the initial PaymentInfo; GrandTotal adds only non-inclusive tax. - Replace the checkout '// TODO, add tax.' with ApplyTaxAsync. - Add integration tests covering exclusive/inclusive pricing, multiple tax lines, taxation-disabled path, due-now filtering, and per-cycle recurring redetermination with historical snapshot immutability. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Every recurring charge is now redetermined through the Taxation framework and captures its own immutable snapshot; historical snapshots never change. - ISubscriptionTaxService.ApplyRecurringTaxAsync records TaxAmount and an immutable TaxSnapshot on each cycle's PaymentInfo. NullSubscriptionTaxService keeps recurring charges tax-free when Taxation is disabled. - The recurring charge taxes the amount the provider actually charged for that specific webhook (authoritative, scoped per subscription) treated as tax-inclusive, so tax is redetermined with current rules without ever claiming uncollected tax or over-taxing multi-subscription sessions. - Persist the checkout tax classification on the Invoice so recurring cycles reuse it; the profile provider gains a session overload that re-resolves the destination (address changes take effect on future cycles). - Wire ApplyRecurringTaxAsync into the Stripe webhook handler with an idempotency guard that skips duplicate deliveries before any tax work. - Persist the card issuing country at checkout for destination-based tax. - Tests: recurring redetermination + snapshot immutability, session profile resolution (classification + destination), taxation-disabled path, and an end-to-end webhook cycle recording a tax snapshot. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add ProductTaxableItemProvider that exposes product content items as taxable items, mapping ProductType to TaxableItemKind and reading price from ProductPart. Registered only when the Taxation feature is enabled, keeping taxation optional. The provider never calculates tax; it exposes tax-relevant classification for the taxation engine. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Checkout computed exclusive tax into the invoice GrandTotal, but the up-front PaymentIntent charges InitialPaymentAmount, which never included that tax, so tax was displayed but never collected. Fold the exclusive tax into InitialPaymentAmount so the payment provider charges the final taxed total. Payment providers remain tax-agnostic and simply charge the amount the taxation framework determined. Document the intentional boundary: the initial checkout charge is taxed exclusive (app-controlled), while provider-driven renewals extract tax from the charged amount as tax-inclusive; the first cycle is never taxed twice. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
No refund workflow existed. Add ITaxRefundCalculator + default implementation that derives full and partial (proportional) refund tax from the original transaction's immutable TaxSnapshot, never from current rules. Partial refunds are allocated across the original tax lines so each jurisdiction is refunded per the original determination. This makes the taxation framework refund-ready and is covered by unit tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contribute six admin reports through the Reports framework: subscription revenue, subscriptions dashboard, expiring subscriptions, new-subscription trend, tax collected, and product performance. Reports query the existing subscription indexes and are registered only when the Reports feature is enabled. Make tax queryable by adding a TaxAmount column to SubscriptionTransactionIndex (populated from the payment tax) with a schema migration. Pure aggregation logic is factored into a database-free helper covered by unit tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Reconcile the module docs with the taxation integration: document how Products, Subscriptions, Checkout, and Payments consume the Taxation framework, the snapshot-driven refund calculator, and the new subscription and commerce reports. Fix the Products type table (Digital, not Plan), add Reports and Taxation to the feature reference, and add a changelog entry for the integration and reports. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ice plan detection - Move Subscriptions and Stripe settings menus to top-level Settings (Configuration node removed in OC) - Auto-apply the Subscription stereotype to any content type with SubscriptionPart via IContentDefinitionHandler, so custom subscription types are detected, indexed, listed on /ServicePlans, and synced with Stripe - Convert front-end route mappings from Startup.Configure to [Route]/[HttpGet]/[HttpPost] attributes and fix the SubscriptionSignupStep route-name mismatch - Modernize the admin Subscriptions list UI (card action-bar, list-group, shared list-management-ui) and replace notification-module leftovers/copy Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Centralize plan billing on the always-present Payment step so plain subscription plans (ProductPart + SubscriptionPart, no extra content types) produce a correct invoice instead of a $0.00 total. - Remove duplicated billing-item creation from Content and Tenant onboarding handlers to prevent double-charging. - Enrich the admin Subscriptions list item: show the customer name as the title with plan title/email, and render status, recurring price and setup-fee badges instead of the raw SubscriptionSession type name. - Restore bootstrap-select (selectpicker) for the Status/Sort list filters to match the Manage Content admin list; auto-submit handled by the shared list-management-ui script (changed.bs.select). - Add regression tests covering the plan-billing invoice flow. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ocs cleanup - Attach OrchardCore RateLimits groups to sensitive subscription routes: 'subscription-checkout' on the signup form/step actions and 'subscription-payment' on the anonymous Stripe/pay-later endpoints, so admins can throttle them once the Rate Limiting feature is enabled. - Feature-gate StripeSyncController with [Feature(Subscriptions.Stripe)] (it depends on StripePriceSyncService, only registered under that feature) and document why it stays in the Subscriptions module. - Apply ocat- edit-view styling to all new admin part/settings editors (Products, Taxation, Subscriptions, Stripe). - Preserve Stripe local-testing and Subscriptions Stripe checkout-mode docs on the site, then remove the redundant per-module README.md placeholders. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix ViewDataDictionary model mismatch on manage-subscriptions: the Title/Tags SummaryAdmin views are rendered via View(shapeType, model) which wraps the model in ShapeViewModel<T>, so declare that model type and read Model.Value. - Block paid checkout when no payment provider is enabled: add an UpdateStepAsync guard to PaymentStepSubscriptionFlowDisplayDriver that fails validation with a clear message instead of retrying 60s and throwing a generic error. Extract testable internal static guards. - Add PaymentStepGuardTests covering the guard logic. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Stripe Payment Elements checkout flow called showError() on every failure branch but never re-enabled the pay button, and the promise chains had no catch handler. Any Stripe, network, or endpoint error left the button permanently stuck on "Processing..." with no way to retry. - Route every failure through a single fail() helper that shows the error and re-enables the button; add a top-level catch for rejected promises and guard non-OK fetch responses. - Harden the Pay Later flow with a catch/non-OK guard so it re-enables the button instead of hanging. - Make the hosted Stripe Checkout click handler respect the selected payment method so it does not hijack submission for other methods. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…y Later checkout - Admin Edit now resolves sessions via SubscriptionSessionIndex (all sessions) instead of the Completed-only SubscriptionIndex, so Pending sessions no longer 404. - Remove the empty SubscriptionsButtonActions_SummaryAdmin shape/view that rendered a blank Actions dropdown. - Pay Later endpoint now wraps state mutation in try/catch with logging and returns structured JSON errors so the checkout surfaces the real reason instead of a generic 'Unexpected error'; the button no longer sticks. - Add unit tests proving the Pay Later commitment satisfies the payment completion handler and that a misconfigured plan throws. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ules Implements extensible CRUD admin management for tax categories, jurisdictions, and rules using OrchardCore display management (drivers, shapes, and display manager) rather than static HTML tables, mirroring the catalog-CRUD pattern. - Adds display drivers, catalog entry handlers, and admin controllers for each tax entity, guarded by a new ManageTaxation permission. - Adds a Commerce > Taxation admin menu with Categories, Jurisdictions, and Rules nodes. - Renders list, create, and edit views through the display system with ocat-* admin styling. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…s to Core - Move TaxationPermissions to the Taxation.Core project so the permission is available to consumers outside the module, updating all references. - Replace the free-text tax category and classification inputs in the TaxationPart settings and item editor with dropdowns populated from the tax categories catalog, avoiding out-of-sync codes. - Document the admin management UI and the end-to-end workflow for applying taxes in the taxation module docs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…assification - Stamp CreatedUtc/Author/OwnerId on tax categories, jurisdictions, and rules via their handlers, and render the author with the user-display-name shape. - Add recipe steps and JSON schemas (TaxCategory/TaxJurisdiction/TaxRule) plus a TaxationPart schema definition, gated behind CrestApps.OrchardCore.Recipes. - Add deployment steps/sources/drivers for the three tax catalog entities. - Add ITaxClassificationProvider with a taxonomy-term provider so items inherit their tax category from tagged taxonomy terms; explicit item codes still win. - Document taxonomy-based per-category taxation, common scenarios, and recipes. - Add unit tests for the deployment serializer round-trip and classification inheritance/override precedence. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
#611) * Add Taxation management admin UI for categories, jurisdictions, and rules Implements extensible CRUD admin management for tax categories, jurisdictions, and rules using OrchardCore display management (drivers, shapes, and display manager) rather than static HTML tables, mirroring the catalog-CRUD pattern. - Adds display drivers, catalog entry handlers, and admin controllers for each tax entity, guarded by a new ManageTaxation permission. - Adds a Commerce > Taxation admin menu with Categories, Jurisdictions, and Rules nodes. - Renders list, create, and edit views through the display system with ocat-* admin styling. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Use category dropdowns for TaxationPart codes and relocate permissions to Core - Move TaxationPermissions to the Taxation.Core project so the permission is available to consumers outside the module, updating all references. - Replace the free-text tax category and classification inputs in the TaxationPart settings and item editor with dropdowns populated from the tax categories catalog, avoiding out-of-sync codes. - Document the admin management UI and the end-to-end workflow for applying taxes in the taxation module docs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add taxation recipes, deployment, audit fields, and taxonomy-based classification - Stamp CreatedUtc/Author/OwnerId on tax categories, jurisdictions, and rules via their handlers, and render the author with the user-display-name shape. - Add recipe steps and JSON schemas (TaxCategory/TaxJurisdiction/TaxRule) plus a TaxationPart schema definition, gated behind CrestApps.OrchardCore.Recipes. - Add deployment steps/sources/drivers for the three tax catalog entities. - Add ITaxClassificationProvider with a taxonomy-term provider so items inherit their tax category from tagged taxonomy terms; explicit item codes still win. - Document taxonomy-based per-category taxation, common scenarios, and recipes. - Add unit tests for the deployment serializer round-trip and classification inheritance/override precedence. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Introduce a standalone CrestApps.OrchardCore.Checkout module (plus Checkout.Abstractions/Core and Payments Money/CurrencyScale primitives) so checkout/payment processing can be reused by subscriptions and one-time goods purchases. Subscriptions now depends on the shared checkout foundation; Stripe and Pay Later gateways integrate through it, taxation is applied to invoices, and completion is distributed-safe. Subscription checkout fixes and hardening: - Add missing OrchardCore.Title dependency (fixes /ServicePlans 500). - Persist the pending session in the authenticated (payment-first) Signup GET so Stripe/Pay Later endpoints can load a durable, resumable session (fixes checkout 404); only newly created sessions are saved to avoid lost-update / double-charge on resume. - Serialize Pay Later / signup-POST completion under the same per-session IDistributedLock used by the Stripe return, with a Completed re-check. - Update Stripe ui_mode 'hosted' -> 'hosted_page'. Docs and tests: - Rewrite Stripe settings-page instructions and Stripe CLI local-dev docs; add checkout module docs and 3.0.0 changelog entry. - Add regression tests (invoice-persistence round-trip, feature dependency) and Checkout test suite. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ecipes/deployment Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Taxation Create/Edit forms for Categories, Jurisdictions, and Rules
rendered empty. Their editor view models were sealed, but the display
manager builds a Castle DynamicProxy subclass of each editor view model;
a sealed type makes proxy creation throw a TypeLoadException that the
display pipeline swallows, silently rendering an empty form. Un-seal the
three view models so every field displays.
Add an icon to the top-level Commerce admin menu node using the Orchard
Core NavigationItemText-{id}.Id.cshtml convention, and give the node a
stable id/class so the view binds.
Add a regression test asserting the driver-backed tax editor view models
are not sealed, and a 3.0.0 changelog entry.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…n module and use RequireFeatures
Removes confusing per-scenario payment sub-features in favor of a single, reusable
module layout gated by [RequireFeatures], and fixes the subscription checkout not
showing a payment option when only one method (e.g. Pay Later) was enabled.
Feature architecture (clean break; no compat shims):
- Remove the `Checkout - Taxation` sub-feature. Taxation-aware checkout now auto-wires
via [RequireFeatures(Checkout, Taxation)] in Checkout/TaxationStartup.cs.
- Add a new standalone `CrestApps.OrchardCore.PayLater` module that owns the generic
`ICheckoutPaymentProvider` (PayLaterCheckoutPaymentProvider), reusable by subscriptions
and one-time purchases. Remove the `Checkout - Pay Later` and `Subscriptions - Pay Later`
sub-features; the live subscription Pay Later path stays in Subscriptions, gated by
[RequireFeatures("CrestApps.OrchardCore.PayLater")].
- Remove the `Subscriptions - Stripe` sub-feature. Stripe auto-wires via
[RequireFeatures(Subscriptions, Stripe)] in Subscriptions/Startup.cs. Drop the
auto-run StripePriceSyncHandler (destructive full sync); price sync stays manual via
StripeSyncController, which now checks the Stripe feature at runtime and requires
ManageSubscriptionSettings on both actions.
- Subscriptions now hard-depends on the Checkout framework (Manifest + ProjectReference).
Bug fix:
- Move DefaultPaymentMethodConfigurations to the base Subscriptions startup so a default
payment method always exists (previously only registered by the Stripe path).
- Render a single payment method server-side with a hidden PaymentMethod input so the
option shows and completes even when scripting is off and only one method is enabled.
Tests & docs:
- Add DefaultPaymentMethodConfigurations tests (single method, processor preference,
configured/unavailable default, no-methods) and feature-dependency assertions
(Subscriptions depends on Checkout; removed sub-features are gone).
- Update Checkout, Subscriptions, new Pay Later docs, feature reference, and the 3.0.0
changelog.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
MikeAlhayek
left a comment
There was a problem hiding this comment.
All interfaces and domain models should be documented and each property should be documented there.
Also add cancelation tidal to public methods where they should be added
|
|
||
| </PropertyGroup> | ||
|
|
||
| <!-- Inherit parent props file if one exists. For example to override TargetFrameworks. --> |
There was a problem hiding this comment.
I don't think this is needed
| # Playwright MCP planning scaffolding (not part of the shipped product). | ||
| .playwright-mcp/ | ||
|
|
||
| # Local Stripe/checkout browser-test scratch (not part of the shipped product). |
There was a problem hiding this comment.
Do it need to be committed then?
| @@ -0,0 +1,9 @@ | |||
| @echo off | |||
There was a problem hiding this comment.
This file should be removed or at least not committed
| /// <summary> | ||
| /// The customer/ship-to destination address. | ||
| /// </summary> | ||
| public TaxAddress Destination { get; set; } |
There was a problem hiding this comment.
Why is the object called TaxAddress if this is a shipping address? Can't we use a better name to represent Address which can be used for any address like shipping address, mailing address etc.
| // Zero-decimal currencies are exchanged as whole units (no multiplication). | ||
| private static readonly HashSet<string> _zeroDecimalCurrencies = new(StringComparer.OrdinalIgnoreCase) | ||
| { | ||
| "BIF", "CLP", "DJF", "GNF", "ISK", "JPY", "KMF", "KRW", "PYG", |
There was a problem hiding this comment.
Maybe we should create. currency object where it will provides info about the currency instead? The Currency object will provide the ISO code, name, and if it supports decimals etc..
| /// <remarks> | ||
| /// Tax types are represented as strings so that additional, region-specific types can be introduced | ||
| /// without changing the framework. The engine never embeds country-specific behavior in a tax type. | ||
| /// </remarks> |
There was a problem hiding this comment.
I am not sure this should be part of the code. These values could be stored in a catalog with a recipe to seed the store from a migration. At least with this approach the user can add/remove values without code
| /// <summary> | ||
| /// Provides shared constant values for the taxation framework. | ||
| /// </summary> | ||
| public static class TaxationConstants |
There was a problem hiding this comment.
If there is a Taxation.Core project, move it there instead
| } | ||
|
|
||
| /// <inheritdoc/> | ||
| public Task SaveAsync(CheckoutSession session) |
There was a problem hiding this comment.
This should not be needed because session saveAsync is called by orchard core before disposing the request automatically
| .FirstOrDefaultAsync(); | ||
|
|
||
| /// <inheritdoc/> | ||
| public async Task<CheckoutSession> GetAsync(string sessionId, CheckoutSessionStatus status) |
There was a problem hiding this comment.
All these methods should have Canellation token
|
|
||
| namespace CrestApps.OrchardCore.Stripe.Core; | ||
|
|
||
| public interface IStripeCustomerService |
| /// </summary> | ||
| /// <param name="scope">A logical bucket, typically the endpoint name (for example "payment-intent").</param> | ||
| /// <param name="discriminator">A per-caller discriminator such as the client IP and/or session id.</param> | ||
| Task<bool> TryAcquireAsync(string scope, string discriminator); |
There was a problem hiding this comment.
Rename to AccuireAsync without Try because this is not a Try pattern where we have out parameter
|
|
||
| namespace CrestApps.OrchardCore.Subscriptions; | ||
|
|
||
| public interface ISubscriptionFlowSession : IEntity |
| /// (scope, discriminator) pair. | ||
| /// </param> | ||
| /// <returns><see langword="true"/> when the attempt is permitted; otherwise <see langword="false"/>.</returns> | ||
| Task<bool> TryAcquireAsync(string scope, string discriminator); |
There was a problem hiding this comment.
Don't use Try since this is not a true Try pattern that has out parameter
Refactors the payments/checkout/subscriptions/taxation surface for a clean, documented code path, addressing the PR #6 self-review comments. StripeSyncController - Remove the IShellFeaturesManager runtime feature check; gate the actions on the presence of the Stripe-only StripePriceSyncService (registered solely by the RequireFeatures-gated StripeStartup), returning NotFound when absent, so the route is inert unless the Stripe integration is active. - Fix the broken StripeSyncPrices named route (distinct GET/POST route names). De-duplication (clean code forward, no obsolete copies) - Remove the triplicate IPaymentAttemptLimiter/PaymentAttemptLimiter and the duplicate/dead PaymentRateLimitOptions from Subscriptions.Core; all callers use the single Checkout limiter. - Rename TryAcquireAsync -> AcquireAsync across the limiter contract. - Remove four empty tax store interfaces (ITaxJurisdiction/Category/Rule/Table Store) and impls; consumers inject INamedCatalog<T> directly (same backing). - Rename TaxAddress -> Address (Taxation.Models). - Remove two dead bool props (InitialPaymentAmount/BillingAmount) from SubscriptionSession; the real amounts live on Invoice. Money-safety / correctness - Thread CancellationToken through ICheckoutSessionStore and observe it in NewAsync; SaveAsync documents the untracked-until-saved contract. Documentation - Add XML documentation to every public interface, model, view model, driver, handler, service, endpoint, migration, and index across the branch. - Fix a stray-space file name (DataNotFoundException .cs -> DataNotFoundException.cs). Restore the accidentally removed Directory.Build.props parent import and the .gitignore rules for local browser-test scratch (.playwright-cli/, .octest/, .playwright-mcp/). Build passes with 0 warnings under -warnaserror; all 2020 unit tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tractions Move the Address model out of Taxation.Abstractions into a new provider-agnostic Addresses.Abstractions project so Taxation, Checkout, and Subscriptions share a single address contract. Clean cutover (no shims); consumers reference the new CrestApps.OrchardCore.Addresses.Models namespace. Adds Address unit tests, changelog note, and solution wiring. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Unseal SubscriptionsMetadata: it is used as the model of an Initialize<T> editor shape, and the display framework cannot build a runtime proxy from a sealed type, which produced a blank admin edit page. - Remove the Settings menu icons for Subscriptions and Payments (keep the Stripe icon): delete the Payments icon template and give the Settings > Subscriptions entry a unique id so the shared top-level icon no longer applies to it. - Tax rule and tax jurisdiction effective dates are now date-only selectors without a time component and no longer labeled UTC. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ensible tax rules Group all subscription/commerce reports under a single "Reports -> Commerce" category. Add a printable payment receipt to the subscriber dashboard, served only to the owning subscriber and formatted with ISO-4217 currency scale. Make tax rules extensible by calculation method: each ITaxCalculationMethod declares its required inputs via the new TaxCalculationMethodInputs flags, and the editor captures Rate, Fixed amount, or Tax table accordingly. Harden save-time validation to protect money-affecting configuration: - A table-driven method cannot be saved without a Tax table. - Selecting a calculation method whose module is disabled reports an error and preserves the submitted values instead of silently discarding them, and the unknown method stays selectable in the editor. - A stored jurisdiction country code that is not in the dropdown is preserved so re-saving can never blank the country and broaden the rule to all countries. Add the canonical CountryProvider (ISO 3166-1) in Addresses.Abstractions and use it for the jurisdiction Country dropdown. Add unit tests for the tax rule validation paths, calculation-method inputs, and the country provider. Update taxation/subscriptions docs and the 3.0.0 changelog. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…Apps.OrchardCore into ma/subscribtions
Fixes #11
Tasks:
SubscriptionIdin a cookie for retrieval and do not create new session on first GET request.ReCaptchaon the very first step.