[MBL-1317] Show Rewards Based on Reward.isAvailable #2014

scottkicks · 2024-04-02T16:26:18Z

📲 What

Uses the isAvailable property on Rewards to determine reward availability and UI.

🤔 Why

Right now, unavailable late pledge rewards are being shown as available 👎
The expected UX is to show unavailable rewards, but they should be marked as such ("no longer available"), not tappable, and appear at the end of the rewards collection.

🛠 How

More in-depth discussion of the change or implementation.

👀 See

Unavailable rewards are marked as (no late pledge) in their title 😄

✅ Acceptance criteria

Unavailable rewards are displayed as such and aren't tappable
Available rewards show up at the beginning of the collection and are selectable as usual
Normal, non-late-pledge rewards behavior is unchanged.

Library/SharedFunctionsTests.swift

scottkicks · 2024-04-02T17:04:41Z

Library/SharedFunctions.swift

-  // Both types of availability must be valid in order for this reward to be considered available.
-  return limitedAvailabilityValid && timebasedAvailabilityValid
+public func rewardIsAvailable(_ reward: Reward) -> Bool {
+  reward.isAvailable == true || reward.isNoReward


The no reward ("$1 Pledge without a reward") option is always available. The backend isn't controlling the isAvailable value for these, so I'm forcing it here.

What do you think of setting isAvailable = true when the no reward reward is created instead? That feels cleaner to me.

scottkicks · 2024-04-02T17:20:21Z

Tests are passing locally for me but not on CI 🤔

amy-at-kickstarter

LGTM, you should be able to have fewer isAvailable setters now.

scottkicks · 2024-04-03T13:40:15Z

...ViewControllerTests/testRewards_Backer_LiveProject_Landscape.lang_es_device_phone5_8inch.png

Re-recorded this snapshot to get it to pass. The only visible difference is the order, which by the looks of the original snapshot, wasn't correct to begin with.

scottkicks · 2024-04-03T13:42:01Z

KsApi/models/templates/ProjectTemplates.swift

formatting changes that I can't seem to avoid. likely related to our discussion here

@scottkicks So you can avoid these, but it's a little annoying - unstage them from the commit, and instead do git commit -m "whatever your commit was" --no-verify. My preference would be for us to solve this in its own PR, if we can, since it's such a weird and separate issue.

scottkicks · 2024-04-03T17:06:16Z

KsApi/models/templates/ProjectTemplates.swift

@@ -163,8 +163,7 @@ private let cosmicSurgeryRewards: [Reward] = [
    |> Reward.lens
    .description .~
    "You will be the first to receive a copy of the book at this special ‘earlybird’ price. Limited to the first 100 copies."
-    |> Reward.lens.localPickup .~ nil
-    |> Reward.lens.isAvailable .~ true,


Forcing this reward to available seems to break the testRewards_Backer_LiveProject_Landscape.lang_es_device_phone5_8inch test. Removing

* This is ok because we're testing that updated values don't impact whether rewards are shown or now. Just that they are shown with the correct data

scottkicks · 2024-04-03T19:25:38Z

Library/ViewModels/RewardsCollectionViewModelTests.swift

-  func testConfigureWithProject_LocalPickupRewards_NonLocalPickupRewards_IncludiongNonLocalBackedReward_ShowsAllRewards_Success() {
-    let rewards = Project.cosmicSurgery.rewards


also fixed a typo

amy-at-kickstarter

The changes LGTM but there are two things in here that should be pulled out.

amy-at-kickstarter · 2024-04-03T19:56:49Z

...ViewControllerTests/testRewards_Backer_LiveProject_Landscape.lang_es_device_phone5_8inch.png

This change looks unncessary, can you pull it out?

amy-at-kickstarter · 2024-04-03T19:57:16Z

KsApi/models/templates/ProjectTemplates.swift

@@ -26,7 +26,8 @@ extension Project {
    dates: Project.Dates(
      deadline: Date(timeIntervalSince1970: 1_475_361_315).timeIntervalSince1970 + 60.0 * 60.0 * 24.0 * 15.0,
      featuredAt: nil,
-      launchedAt: Date(timeIntervalSince1970: 1_475_361_315).timeIntervalSince1970 - 60.0 * 60.0 * 24.0 * 15.0,


Can you unstage this change from the PR? It's fixed in #2020

scottkicks added the post-campaign-pledging label Apr 2, 2024

scottkicks self-assigned this Apr 2, 2024

scottkicks force-pushed the feat/scott/mbl-1317/fix-unavailable-rewards branch 2 times, most recently from 44a9373 to 957ddee Compare April 2, 2024 16:57

scottkicks commented Apr 2, 2024

View reviewed changes

Library/SharedFunctionsTests.swift Show resolved Hide resolved

scottkicks commented Apr 2, 2024

View reviewed changes

scottkicks marked this pull request as ready for review April 2, 2024 17:20

scottkicks requested a review from amy-at-kickstarter April 2, 2024 17:20

amy-at-kickstarter approved these changes Apr 2, 2024

View reviewed changes

scottkicks force-pushed the feat/scott/mbl-1317/fix-unavailable-rewards branch 2 times, most recently from 11a7ad0 to 2a7b398 Compare April 2, 2024 18:28

kickstarter deleted a comment from nativeksr Apr 2, 2024

scottkicks force-pushed the feat/scott/mbl-1317/fix-unavailable-rewards branch 6 times, most recently from e99566a to acc13fc Compare April 2, 2024 23:07

scottkicks added 3 commits April 3, 2024 08:00

use isAvailable property on the reward model to render rewards

919bdf5

update Library tests

681f212

update snapshot tests

82d1d9a

scottkicks force-pushed the feat/scott/mbl-1317/fix-unavailable-rewards branch from 7f2f147 to 82d1d9a Compare April 3, 2024 13:39

scottkicks commented Apr 3, 2024

View reviewed changes

scottkicks force-pushed the feat/scott/mbl-1317/fix-unavailable-rewards branch from dfa1cdb to af65f22 Compare April 3, 2024 14:34

oops missed a line when committing

21bd8d3

scottkicks force-pushed the feat/scott/mbl-1317/fix-unavailable-rewards branch from af65f22 to 21bd8d3 Compare April 3, 2024 15:16

scottkicks and others added 2 commits April 3, 2024 11:50

Merge branch 'main' into feat/scott/mbl-1317/fix-unavailable-rewards

7d31e6b

hopefully fixing snapshots

83f1c73

scottkicks commented Apr 3, 2024

View reviewed changes

set rewards availability to false in failing tests

53a7f4f

* This is ok because we're testing that updated values don't impact whether rewards are shown or now. Just that they are shown with the correct data

scottkicks requested a review from amy-at-kickstarter April 3, 2024 19:24

scottkicks commented Apr 3, 2024

View reviewed changes

amy-at-kickstarter approved these changes Apr 3, 2024

View reviewed changes

undo ProjectTemplate changes

48a3470

scottkicks force-pushed the feat/scott/mbl-1317/fix-unavailable-rewards branch from 0587d00 to 48a3470 Compare April 4, 2024 13:32

scottkicks and others added 3 commits April 4, 2024 08:33

Merge branch 'main' into feat/scott/mbl-1317/fix-unavailable-rewards

859a3bb

snapshots need to include Rewards screen title

0a0a4e8

formatting

a96c66b

kickstarter deleted a comment from nativeksr Apr 4, 2024

ifosli and others added 2 commits April 4, 2024 10:43

Re-record snapshots

211bc9d

Merge branch 'main' into feat/scott/mbl-1317/fix-unavailable-rewards

619f5ab

scottkicks merged commit fd1a221 into main Apr 4, 2024
5 checks passed

scottkicks deleted the feat/scott/mbl-1317/fix-unavailable-rewards branch April 4, 2024 17:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MBL-1317] Show Rewards Based on Reward.isAvailable #2014

[MBL-1317] Show Rewards Based on Reward.isAvailable #2014

scottkicks commented Apr 2, 2024 •

edited

scottkicks Apr 2, 2024

ifosli Apr 3, 2024

scottkicks commented Apr 2, 2024

amy-at-kickstarter left a comment

scottkicks Apr 3, 2024

scottkicks Apr 3, 2024

amy-at-kickstarter Apr 3, 2024

scottkicks Apr 3, 2024 •

edited

scottkicks Apr 3, 2024

amy-at-kickstarter left a comment

amy-at-kickstarter Apr 3, 2024

amy-at-kickstarter Apr 3, 2024

		func testConfigureWithProject_LocalPickupRewards_NonLocalPickupRewards_IncludiongNonLocalBackedReward_ShowsAllRewards_Success() {
		let rewards = Project.cosmicSurgery.rewards

[MBL-1317] Show Rewards Based on Reward.isAvailable #2014

[MBL-1317] Show Rewards Based on Reward.isAvailable #2014

Conversation

scottkicks commented Apr 2, 2024 • edited

📲 What

🤔 Why

🛠 How

👀 See

✅ Acceptance criteria

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scottkicks commented Apr 2, 2024

amy-at-kickstarter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scottkicks Apr 3, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amy-at-kickstarter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scottkicks commented Apr 2, 2024 •

edited

scottkicks Apr 3, 2024 •

edited