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

[MBL-1197] Show unavailable rewards last #1947

Merged
merged 5 commits into from Feb 15, 2024
Merged

[MBL-1197] Show unavailable rewards last #1947

merged 5 commits into from Feb 15, 2024

Conversation

ifosli
Copy link
Contributor

@ifosli ifosli commented Feb 14, 2024

📲 What

Order rewards so that unavailable ones are shown last. Note that we will need to update the rewardIsAvailable utility method to include post campaign backings, but outside of that, this sorting will work for both.

This also deletes unnecessary snapshots for the rewards tests in the first commit. The first commit looks pretty large, but it's mostly indentation changes and deleted snapshots. The second commit is the actual sorting code, with some test changes to make the tests more relevant. I recommend reviewing this one commit at a time.

👀 See

Jira and snapshot tests.

✅ Acceptance criteria

  • Unavailable rewards show last
  • Rewards can still be selected and interacted with without any weirdness

@@ -44,6 +44,7 @@ final class RewardsCollectionViewControllerTests: TestCase {
func testRewards_NonBacker_LiveProject_Landscape() {
let project = Project.cosmicSurgery
|> Project.lens.state .~ .live
|> Project.lens.rewardData.rewards %~ { Array($0[1...3]) }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This just reduces the number of items so that the unavailable one shows as well, even when it's last

@@ -77,24 +74,23 @@ final class RewardsCollectionViewControllerTests: TestCase {
|> Backing.lens.rewardId .~ reward.id
|> Backing.lens.shippingAmount .~ 10
|> Backing.lens.amount .~ 700.0
|> Backing.lens.addOns .~ []
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Opting out of add ons here ensures that this reward will be shown as unavailable when it's not selected. The default addOn has the same reward id as one of the rewards, which messes with our count for the reward.

@ifosli ifosli marked this pull request as ready for review February 14, 2024 23:32
Copy link
Contributor

@scottkicks scottkicks left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@ifosli ifosli merged commit 8aa9945 into main Feb 15, 2024
5 checks passed
@ifosli ifosli deleted the sortRewards branch February 15, 2024 17:45
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.

None yet

2 participants