-
Notifications
You must be signed in to change notification settings - Fork 988
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-1893 && MBL-1877: Separate rewards from project query #2173
base: master
Are you sure you want to change the base?
Conversation
- In RewardsSelectionViewModel we fetch all rewards again with simpleShippingRules this time, but this information is not moved via intent to any other screen.
…f github.com:kickstarter/android-oss into imartin/separate-rewards-from-project
…wardsSelectionVM - Removed from project query the rewards, only added the reward no reward with minimum pledge for a project. - When updating/managing a reward the information for reward/addons was collected from backing, therefore shippingRules was not available within baking that nested object. - Use as default the fresly queried information, and update with the backed information rather that the other way around.
…tin/separate-rewards-from-project
…mation to who "change payment method"/ "Fix Pledge" do require the rewards information for the project
…rd", but always for fixing changing payment method AND choosing another reward it will show "Manage pledge"
…e" option was removed from the menu
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #2173 +/- ##
============================================
+ Coverage 68.83% 68.85% +0.02%
- Complexity 2163 2165 +2
============================================
Files 346 346
Lines 22909 22912 +3
Branches 3353 3350 -3
============================================
+ Hits 15769 15776 +7
+ Misses 5339 5337 -2
+ Partials 1801 1799 -2 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
@@ -28,7 +28,7 @@ fun PledgeData.shippingCostIfShipping(): Double { | |||
var addOnsShippingCost = 0.0 | |||
this.addOns()?.map { | |||
if (RewardUtils.shipsWorldwide(it) || RewardUtils.shipsToRestrictedLocations(it)) { | |||
addOnsShippingCost += (it.shippingRules()?.first()?.cost() ?: 0.0) * (it.quantity() ?: 0) | |||
addOnsShippingCost += (it.shippingRules()?.firstOrNull()?.cost() ?: 0.0) * (it.quantity() ?: 0) |
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.
The crash was due to using the reward and addOns from the backing object, where shipping locations information is not available
@@ -220,7 +220,8 @@ fun RewardCarouselScreen( | |||
id = R.string.Back_it_because_you_believe_in_it | |||
), | |||
onRewardSelectClicked = { onRewardSelected(reward) }, | |||
isCTAButtonVisible = project.isAllowedToPledge() | |||
isCTAButtonVisible = project.isAllowedToPledge(), | |||
yourSelectionIsVisible = project.backing()?.isBacked(reward) ?: 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.
Blue pill was not visible when the baked reward is a "Reward no reward"
val aux = holder[backedAddOn.id()] | ||
if (aux != null) { | ||
val updated = aux.toBuilder().quantity(backedAddOn.quantity()).build() | ||
holder[backedAddOn.id()] = updated |
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.
before we were using the information from the baked object, now we update the "quantity" field alone to not loose the "shipping rules" field
when (flowContext) { | ||
PledgeFlowContext.NEW_PLEDGE, | ||
PledgeFlowContext.CHANGE_REWARD -> getPledgeInfoFrom(pData) | ||
PledgeFlowContext.MANAGE_REWARD, |
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.
"Manage_Reward" is used for both updating selected reward and changing reward, it describes the fact of having to present to the user RewardsCarousel not granular enough.
Exchanged to use PledgeReason
instead of PledgeFlowContext
as it's uniquely identified.
// - API does not provide the Reward no reward, we need to add it first | ||
val minPledge = data.project()?.minPledge()?.toDouble() ?: 1.0 | ||
val modifiedRewards = rwList.toMutableList() | ||
modifiedRewards.add(0, RewardFactory.noReward().toBuilder().minimum(minPledge).build()) |
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.
Adding reward no reward to the list of rewards. It is not provided by the API, but the minimum value to be pledged is configured in a project level.
@@ -150,7 +150,6 @@ class AddOnsViewModel(val environment: Environment, bundle: Bundle? = null) : Vi | |||
bonusAmount = b.amount() | |||
} else { | |||
backedAddOns = b.addOns() ?: emptyList() | |||
currentUserReward = b.reward() ?: currentUserReward |
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 causing the issue mentioned here
📲 What
RewardsSelectionViewModel
.🤔 Why
Project query has modified to obtain all simple shipping rules, for all rewards, (Globally shipping rewards then will hold a collection of 246 shipping rules each). Project (as any other data model within the app) can be parzelized to be serialized/deserialized from Intents, but unfortunately we have hit again the
transactionTooLarge
exception due to Project being too big (we need to find other ways to persist information within the app, passing the hole object from screen to screen via intent to avoid extra network calls has clear limitations).👀 See
filteringRewards.mp4
modifiedRewardShippingAndAddOn.mp4
ChangerewardToNoRewardAndBluePill.mp4
changePaymentMethod.mp4
chooseAnotherRewardFromDigitalToShipping.mp4
| | |
📋 QA
Story 📖
MBL-1877
MBL-1893