Skip to content

Comments

Refactored hasActiveOffer and _getActiveDiscount to share common logic#26543

Open
mike182uk wants to merge 1 commit intomainfrom
mike-ber-3338
Open

Refactored hasActiveOffer and _getActiveDiscount to share common logic#26543
mike182uk wants to merge 1 commit intomainfrom
mike-ber-3338

Conversation

@mike182uk
Copy link
Member

ref https://linear.app/ghost/issue/BER-3338

Both functions independently computed discount windows using the same three data paths (trial periods, Stripe coupon data, and legacy offer duration fallback). This extracts the shared computation into a single getDiscountWindow utility so the two callers stay in sync as the discount model evolves.

…logic

ref https://linear.app/ghost/issue/BER-3338

Both functions independently computed discount windows using the same
three data paths (trial periods, Stripe coupon data, and legacy offer
duration fallback). This extracts the shared computation into a single
`getDiscountWindow` utility so the two callers stay in sync as the
discount model evolves.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 23, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 519483f and 01dfb7a.

📒 Files selected for processing (5)
  • ghost/core/core/server/services/members/members-api/services/next-payment-calculator.js
  • ghost/core/core/server/services/members/members-api/utils/get-discount-window.js
  • ghost/core/core/server/services/members/members-api/utils/has-active-offer.js
  • ghost/core/test/unit/server/services/members/members-api/utils/get-discount-window.test.js
  • ghost/core/test/unit/server/services/members/members-api/utils/has-active-offer.test.js

Walkthrough

This change introduces a new utility function getDiscountWindow that consolidates discount window calculation logic across multiple files. The function handles three offer types: free_months (trial-based), Stripe coupon discounts with explicit start/end dates, and legacy offers with duration specifications. The next-payment-calculator.js and has-active-offer.js files were refactored to delegate discount window computation to this new utility. Corresponding unit tests were added to validate discount scenarios across all three offer handling paths, and existing test helpers were expanded to support trial_start_at field mapping.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main refactoring objective: extracting common logic from hasActiveOffer and _getActiveDiscount into a shared utility.
Description check ✅ Passed The description clearly explains the rationale for the refactoring and identifies the specific shared computation patterns being consolidated.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch mike-ber-3338

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@sagzy sagzy left a comment

Choose a reason for hiding this comment

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

Nice! Refactoring makes sense. Naming: getActiveDiscount instead of getDiscountWindow?

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.

2 participants