Skip to content

[5.x] Only count non-expired subscriptions on the plans index - #4382

Open
nfourtythree wants to merge 3 commits into
5.xfrom
nathaniel/com-688-5x-craft-commerce-subscription-plans-screen-counts-expired
Open

nfourtythree wants to merge 3 commits into
5.xfrom
nathaniel/com-688-5x-craft-commerce-subscription-plans-screen-counts-expired

Conversation

@nfourtythree

@nfourtythree nfourtythree commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Description

Fixes #4381.

The "Active subscriptions" column on Commerce → Subscription Plans was counting all subscriptions for a plan, including expired ones.

Added Subscriptions::getActiveSubscriptionCountByPlanId() and Plan::getActiveSubscriptionCount(), and the plans index now uses them. The count uses a subscription element query with status(Subscription::STATUS_ACTIVE), so it follows the same rules as the "Active" status on the subscriptions index and Plan::getActiveUserSubscriptions(). Subscriptions that are expired, suspended, not yet started or trashed are not counted.

getSubscriptionCountByPlanId() and Plan::getSubscriptionCount() are unchanged and still return the total count, so nothing that relies on them is affected.

@nfourtythree nfourtythree self-assigned this Oct 9, 2026
@linear-code

linear-code Bot commented Oct 9, 2026

Copy link
Copy Markdown

COM-688

@nfourtythree
nfourtythree requested a balanced review from Copilot October 9, 2026 14:14
@nfourtythree
nfourtythree marked this pull request as ready for review October 9, 2026 14:14
@nfourtythree
nfourtythree requested a review from a team as a code owner October 9, 2026 14:14

Copilot AI left a comment

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.

🟡 Changes recommended

The count still includes suspended and not-yet-started subscriptions, unlike the subscriptions index’s Active status.

1 open finding
What changed in this PR

Updates subscription plan counts to exclude expired subscriptions.

Changes:

  • Adds active subscription count APIs.
  • Uses the new count on the plans index.
  • Documents the fix for #4381.
File Description
CHANGELOG.md Documents the APIs and fix.
src/​base/​Plan.php Exposes the active count.
src/​services/​Subscriptions.php Implements the filtered count.
src/​templates/​subscriptions/​plans/​index.twig Displays the active count.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/services/Subscriptions.php Outdated

Copilot AI left a comment

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.

🟢 Approval recommended

The implementation matches existing active-status semantics and includes comprehensive focused tests.

0 open findings

1 resolved since last review

🧠 Review effort: Balanced

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[5.x]: Craft Commerce: Subscription Plans screen counts expired subscriptions as active

2 participants