-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
MBL-1316: Use isAvailable to filter visible add-ons #2012
Conversation
@@ -24,7 +24,8 @@ extension Reward { | |||
shippingRulesExpanded: $1.shippingRulesExpanded, |
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'm still formulating some thoughts about how we can replace the Foo.template.lens |> ...
pattern, so I used this as an opportunity to actually get into the guts of one to learn more about it.
isUnlimitedOrAvailable | ||
] | ||
.allSatisfy(isTrue) | ||
return addOn.isAvailable ?? false |
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.
Replace all the start time logic with isAvailable
.
@@ -467,6 +484,7 @@ final class RewardAddOnSelectionViewModelTests: TestCase { | |||
|> Reward.lens.remaining .~ nil | |||
|> Reward.lens.startsAt .~ (MockDate().timeIntervalSince1970 + 30) | |||
|> Reward.lens.endsAt .~ (MockDate().timeIntervalSince1970 + 60) | |||
|> Reward.lens.isAvailable .~ false |
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 test actually includes some unavailable rewards, so updating it to reflect that.
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.
Nice! Sounds like this change is actually a long time coming anyway.
We should note this change in our regression testing since impacts all rewards
📲 What
Use the GraphQL field
isAvailable
to filter visible add-ons🤔 Why
Our existing implementation did not take late pledges into account, so it was filtering out add-ons that were actually still available for late pledges. I spoke with @zan-rosenthal and apparently the gold-standard solution is to use the
available
field from GraphQL.Most of this PR is updating plumbing to get
available
intoReward
.