-
Notifications
You must be signed in to change notification settings - Fork 3
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
Free up coupons from cancelled orders #2311
base: master
Are you sure you want to change the base?
Conversation
@@ -170,12 +171,14 @@ trait ApiFixtures extends SuiteMixin with HttpSupport with PhoenixAdminApi with | |||
trait CouponFixtureBase { | |||
def couponActiveFrom: Instant = Instant.now.minus(1, DAYS) | |||
def couponActiveTo: Option[Instant] = None | |||
def couponUsageRules: CouponUsageRules = | |||
CouponUsageRules(isUnlimitedPerCode = true, isUnlimitedPerCustomer = 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.
I’d argue they shouldn’t have defaults, either.
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.
Also cake pattern here is pretty unintuitive. I have to
- add another method parameter
- provide overrideable value that this method uses as a default
instead of
- just calling the function with arguments I want
usesPerCustomer = Some(1) | ||
) | ||
|
||
// TODO: extract CheckoutFixture and reuse it here (more refactoring will be needed for that) @michalrus |
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.
form ← * <~ ObjectForms.mustFindById400(coupon.formId) | ||
couponUsage ← * <~ CouponUsages | ||
.filterByCoupon(coupon.formId) | ||
def incrementUsageCounts(codeId: Int, customer: User, incrementBy: Int)(implicit ec: EC, |
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.
If it’s None
… just… don’t call it.
Solves #1703