-
Notifications
You must be signed in to change notification settings - Fork 138
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
CheckoutPricing: Coupons #385
Conversation
9eb79fd
to
8003c08
Compare
d5a895a
to
cd31e99
Compare
b51454a
to
3de9002
Compare
bed9622
to
51d443a
Compare
- Manages a single coupon on the item set Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
- /plans/:plan_code/coupons/:coupon_code -> /coupons/:id - optional `plan_id` as `plan` Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
- Vestigial IE8 support Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
- Still WIP: need to determine API capabilities for plan-specific and plan-agnostic coupons Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
- For spreading a long set request across multiple requests Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
- Uses pipedRequest to spread a large number of plans - Adds expectation for invoice-level coupon properties Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
- Supports discounts against adjustments, single subscriptions, and free trials Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
- Ignores 404s until done - Immediately rejects other errors - Collects and concats array responses - Immediately resolves non-array responses Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
- Avoids an extraneous API call when assigning a coupon to a subscription Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
- CheckoutPricing must accept SubscriptionPricing instances before they are given plans Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
- Checks for a subscription price before adding it to the price, since price is not guaranteed to exist Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
- /v1/coupons will return coupons which may not necessarily match the requested plan codes, if they may also apply to adjustments - this adds a validity detection method to SubscriptionPricing coupon getter Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
- Compatible with new coupon validity checks Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
87e8fb5
to
fbb652c
Compare
Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
- Also fixes setup fee exclusion rules for discounts Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
- Fixed amount coupons - subscription-level coupons - Free trial coupons Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
const discountableNow = this.price.now.subtotal + this.price.now.setup_fee; | ||
const discountableNext = this.price.next.subtotal; | ||
this.price.now.discount = Math.min(discountableNow, coupon.discount.amount[this.items.currency]); | ||
this.price.next.discount = Math.min(discountableNext, coupon.discount.amount[this.items.currency]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an important change to current pricing behavior.
Previously, if a coupon with a fixed value higher than the subscription total (now + next cycle) was applied, it would apply a discount larger than the total. Now, the discount amount is limited to the total per cycle.
@@ -346,7 +346,7 @@ describe('Recurly.Pricing.Subscription', function () { | |||
.coupon('coop') | |||
.done(function (price) { | |||
assert.equal(price.now.discount, '20.00'); | |||
assert.equal(price.next.discount, '20.00'); | |||
assert.equal(price.next.discount, '19.99'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -63,11 +63,10 @@ export default class Calculations { | |||
this.price.now.subtotal += this.price.now.addons; | |||
this.price.next.subtotal += this.price.next.addons; | |||
|
|||
this.setupFee(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was moved prior to discount calculation in order to determine whether a coupon discount amount has exceeded the discountable subtotal for the subscription cycles.
@@ -1,12 +1,12 @@ | |||
import Emitter from 'component-emitter'; | |||
import errors from '../errors'; | |||
import {CheckoutPricing} from './pricing/checkout'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this going to be used in one of the followup PR's?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. Prior to writing the CheckoutPricing/Attachment
class, I'll adapt these modules to use the Pricing
base class
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #394
lib/recurly.js
Outdated
* @private | ||
*/ | ||
|
||
pipedRequest ({ method, route, data, by, size }) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about setting the default for size
here like it appears in the doc block above. Since the only use of this function at this time using the default, we could remove the arg in that call. Is the method
generally always going to be a GET
? If so, we could set that default as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually fixed the default assignment in #393 when I added specs for this method. I'll add it here too.
+1 to setting the method default as well
couponIsValidForSubscription (coupon) { | ||
if (!coupon) return false; | ||
if (!coupon.applies_to_plans) return false; | ||
if (coupon.applies_to_all_plans) return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
another option would be to consolidate these last three lines into:
return coupon.applies_to_all_plans || ~coupon.plans.indexOf(this.items.plan.code)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For business rules like this, I personally prefer the readability of the early returns
Signed-off-by: Christopher Rogers <chrissrogers@gmail.com>
9b701c1
to
2362f17
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Description
CheckoutPricing#coupon
CheckoutPricing/Calculations#coupon
calculation routinesExample