Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stripe refactorings #8407

Closed
wants to merge 20 commits into from
Closed

Stripe refactorings #8407

wants to merge 20 commits into from

Conversation

gbp
Copy link
Member

@gbp gbp commented Oct 10, 2024

What does this do?

Refactor Stripe wrappers classes and minor template updates.

Why was this needed?

Help with future migration to Stripe Prices.


[skip changelog]

@gbp gbp marked this pull request as ready for review October 11, 2024 08:19
@gbp gbp force-pushed the stripe-prices-refactor branch 2 times, most recently from 479c599 to 3604552 Compare October 21, 2024 08:45
@gbp gbp force-pushed the stripe-prices-refactor branch 2 times, most recently from d1e9029 to a030c3c Compare November 5, 2024 15:56
@gbp gbp mentioned this pull request Nov 6, 2024
Copy link
Member

@dracos dracos left a comment

Choose a reason for hiding this comment

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

Looks good :) I left a few tiny comments is all.

app/models/alaveteli_pro/subscription_with_discount.rb Outdated Show resolved Hide resolved
app/models/concerns/taxable.rb Outdated Show resolved Hide resolved
app/helpers/alaveteli_pro/plan_helper.rb Outdated Show resolved Hide resolved
app/models/alaveteli_pro/subscription/discount.rb Outdated Show resolved Hide resolved
@gbp gbp force-pushed the stripe-prices-refactor branch 2 times, most recently from c61fd03 to 7ada4b3 Compare November 11, 2024 12:44
@gbp
Copy link
Member Author

gbp commented Nov 11, 2024

Thanks @dracos I have rebased over the first PR, moved the one code removal to the prior commit (now 6f98575) and pushed some new commits (69dfb91 f1e1963 0994a53)

Replacing wrapped Stripe objects with new Pro Plan which includes
Taxable.

Pro Plan will be expanded in later commits while refactoring the
checkout process.
We're always returning integers there is no need to convert to floats
and back again. This can introduce rounding errors.
Check in object respond to the method before forwarding. This is needed
to fix exceptions such as:
  private method `BigDecimal' called for #<Stripe::Subscription...
Load and cache subscription.plan, automatically wrapping in our Pro
Plan class.
Automatically include Pro SubscriptionWithDiscount so we don't have to
manually wrap the instances in the controller.
This really isn't needed now this module is included in Pro
Subscription.
Use whole integers instead of floats which can cause rounding errors.
Change to `#discounted_amount` to better represent when this method is
returning.
Move Discount module under Pro Subscription namespace.
- Show discount price and name instead of coupon code
- Remove leading space around billing date
- Minor CSS layout changes to ensure subscription sign up and
  subscription show views are consistent.
Improve billing frequency helper method:
1. accept Stripe Plan as and argument
2. take into account plans interval count
3. work with all interval types

Add billing interval helper method to output `per user, per month` or
`per user, every 2 months` depending on how the Stripe Plan has been
configured.
Handle strings correctly irrespective of whether they start with the
namespace or not.
Allowing us to simplify the controller actions so we're always dealing
with Plan objects.
Again simplify controllers by moving coupons loading into delegated
classes.
Further simplification of controllers by moving default source handling
into a delegated class.
@gbp
Copy link
Member Author

gbp commented Nov 19, 2024

Merged in 63bba9f

@gbp gbp closed this Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants