From c4843433aae3be88268d42365b55f984fc8536c1 Mon Sep 17 00:00:00 2001 From: Isabel Martin Date: Tue, 8 Sep 2020 12:46:45 -0700 Subject: [PATCH 01/11] - Bug when updating reward, the reward used when changing reward was the one inside backingObject, instead of the new selected reward - Reward objects from graphQL where not filling all the necessary fields, fill up the ones from V1 as well to avoid side effects - Added operator NeverError on ProjectViewModel to avoid possible crashes --- .../kickstarter/libs/utils/RewardUtils.java | 3 ++- .../kickstarter/services/KSApolloClient.kt | 21 +++---------------- .../BackingAddOnsFragmentViewModel.kt | 16 +++++++------- .../viewmodels/ProjectViewModel.kt | 3 +++ 4 files changed, 15 insertions(+), 28 deletions(-) diff --git a/app/src/main/java/com/kickstarter/libs/utils/RewardUtils.java b/app/src/main/java/com/kickstarter/libs/utils/RewardUtils.java index 661f3870fd..4c59fcef50 100644 --- a/app/src/main/java/com/kickstarter/libs/utils/RewardUtils.java +++ b/app/src/main/java/com/kickstarter/libs/utils/RewardUtils.java @@ -84,7 +84,8 @@ public static boolean isReward(final @NonNull Reward reward) { */ public static boolean isShippable(final @NonNull Reward reward) { final String shippingType = reward.shippingType(); - return shippingType != null && !Reward.SHIPPING_TYPE_NO_SHIPPING.equals(shippingType); + final boolean no_shipping_types = reward.shippingPreferenceType() == Reward.ShippingPreference.NONE; + return shippingType != null && !(Reward.SHIPPING_TYPE_NO_SHIPPING.equals(shippingType) || no_shipping_types); } /** diff --git a/app/src/main/java/com/kickstarter/services/KSApolloClient.kt b/app/src/main/java/com/kickstarter/services/KSApolloClient.kt index 2e754715a2..3d3beec9c4 100644 --- a/app/src/main/java/com/kickstarter/services/KSApolloClient.kt +++ b/app/src/main/java/com/kickstarter/services/KSApolloClient.kt @@ -606,24 +606,7 @@ private fun createBackingObject(backingGr: fragment.Backing?): Backing { val shippingAmount = backingGr?.shippingAmount()?.fragments() val reward = backingGr?.reward()?.fragments()?.reward()?.let { reward -> - val rewardId = decodeRelayId(reward.id()) ?: -1 - val rewardAmount = reward.amount().fragments().amount().amount()?.toDouble() - val rewardSingleLocation = location?.let { location -> - return@let Reward.SingleLocation.builder() - .localizedName(location.displayableName()) - .id(decodeRelayId(location.id())?:-1) - .build() - } - - return@let Reward.builder() - .title(reward.name()) - .minimum(rewardAmount?: -1.0) - .description(reward.description()) - .isAddOn(false) - .estimatedDeliveryOn(DateTime(reward.estimatedDeliveryOn())) - .shippingSingleLocation(rewardSingleLocation) - .id(rewardId) - .build() + return@let rewardTransformer(reward) } val backerData = backingGr?.backer()?.fragments()?.user() @@ -754,7 +737,9 @@ private fun rewardTransformer(rewardGr: fragment.Reward): Reward { .isAddOn(true) .addOnsItems(items) .id(rewardId) + .shippingPreference(shippingPreference.name) .shippingPreferenceType(shippingPreference) + .shippingType(shippingPreference.name) .shippingRules(shippingRules) .build() } diff --git a/app/src/main/java/com/kickstarter/viewmodels/BackingAddOnsFragmentViewModel.kt b/app/src/main/java/com/kickstarter/viewmodels/BackingAddOnsFragmentViewModel.kt index a67eac2bb6..ddc67ebde3 100644 --- a/app/src/main/java/com/kickstarter/viewmodels/BackingAddOnsFragmentViewModel.kt +++ b/app/src/main/java/com/kickstarter/viewmodels/BackingAddOnsFragmentViewModel.kt @@ -113,7 +113,7 @@ class BackingAddOnsFragmentViewModel { val project = projectData .map { it.project() } - val rewardPledge = pledgeData + val reward = pledgeData .map { it.reward() } val backing = projectData @@ -121,13 +121,6 @@ class BackingAddOnsFragmentViewModel { .filter { ObjectUtils.isNotNull(it) } .map { requireNotNull(it) } - val backingReward = backing - .map { it.reward() } - .filter { ObjectUtils.isNotNull(it) } - .map { requireNotNull(it) } - - val reward = Observable.merge(rewardPledge, backingReward) - val projectAndReward = project .compose>(combineLatestPair(reward)) @@ -167,6 +160,7 @@ class BackingAddOnsFragmentViewModel { .map { it.addOns()?.toList() } .filter { ObjectUtils.isNotNull(it) } .map { requireNotNull(it) } + .share() val addOnsFromGraph = project .switchMap { pj -> this.apolloClient.getProjectAddOns(pj.slug()?.let { it }?: "") } @@ -325,11 +319,15 @@ class BackingAddOnsFragmentViewModel { private fun filterByLocationAndUpdateQuantity(addOns: List, pData: ProjectData, rule: ShippingRule, rw: Reward): Triple, ShippingRule> { val filteredAddOns = when (rw.shippingPreference()){ + Reward.ShippingPreference.UNRESTRICTED.name, Reward.ShippingPreference.UNRESTRICTED.toString().toLowerCase() -> { addOns.filter { - it.shippingPreferenceType() == Reward.ShippingPreference.UNRESTRICTED || isDigital(it) + (it.shippingPreferenceType() == Reward.ShippingPreference.UNRESTRICTED ) || + containsLocation(rule, it) || + isDigital(it) } } + Reward.ShippingPreference.RESTRICTED.name, Reward.ShippingPreference.RESTRICTED.toString().toLowerCase() -> { addOns.filter { containsLocation(rule, it) || isDigital(it) } } diff --git a/app/src/main/java/com/kickstarter/viewmodels/ProjectViewModel.kt b/app/src/main/java/com/kickstarter/viewmodels/ProjectViewModel.kt index 0aca09313c..53c50f30b2 100644 --- a/app/src/main/java/com/kickstarter/viewmodels/ProjectViewModel.kt +++ b/app/src/main/java/com/kickstarter/viewmodels/ProjectViewModel.kt @@ -546,6 +546,9 @@ interface ProjectViewModel { .switchMap { this.apolloClient.getProjectBacking(it.slug()?: "") } + .compose(neverError()) + .filter { ObjectUtils.isNotNull(it) } + .share() // - Update fragments with the backing data projectData From f2ec283dbd36ae6e44d19f69b0fb9fda99724cc1 Mon Sep 17 00:00:00 2001 From: Isabel Martin Date: Tue, 8 Sep 2020 13:27:31 -0700 Subject: [PATCH 02/11] - Updated to be able to show the alert if changing from a selectedReward with location to another without location --- .../viewmodels/RewardsFragmentViewModel.kt | 31 +++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/kickstarter/viewmodels/RewardsFragmentViewModel.kt b/app/src/main/java/com/kickstarter/viewmodels/RewardsFragmentViewModel.kt index 26ffe88492..b12d18d364 100644 --- a/app/src/main/java/com/kickstarter/viewmodels/RewardsFragmentViewModel.kt +++ b/app/src/main/java/com/kickstarter/viewmodels/RewardsFragmentViewModel.kt @@ -9,6 +9,7 @@ import com.kickstarter.libs.rx.transformers.Transformers.combineLatestPair import com.kickstarter.libs.rx.transformers.Transformers.takeWhen import com.kickstarter.libs.utils.BackingUtils import com.kickstarter.libs.utils.ObjectUtils +import com.kickstarter.mock.factories.RewardFactory import com.kickstarter.models.Project import com.kickstarter.models.Reward import com.kickstarter.ui.data.PledgeData @@ -82,6 +83,10 @@ class RewardsFragmentViewModel { .filter { ObjectUtils.isNotNull(it) } .map { requireNotNull(it) } + val backedNoReward = project + .filter { it.backing() != null && it.backing()?.reward() == null } + .map { RewardFactory.noReward() } + val backedAddOns = project .map { it?.backing()?.addOns() } .filter { ObjectUtils.isNotNull(it) } @@ -97,7 +102,7 @@ class RewardsFragmentViewModel { .compose(bindToLifecycle()) .subscribe(this.backedRewardPosition) - // - If the selected Reward do not have AddOns + // - If the selected Reward to update to do not have AddOns val pledgeDataAndReason = this.projectDataInput .compose>(Transformers.takePairWhen(this.rewardClicked)) .filter { !it.second.hasAddons() } @@ -138,10 +143,26 @@ class RewardsFragmentViewModel { this.showAddOnsFragment.onNext(it) } + val shouldShowAlert = pledgeDataAndReasonWithAddOns + .filter { it.second == PledgeReason.UPDATE_REWARD } + .map { it.first } + .compose>(combineLatestPair(backedReward)) + .map { differentShippingTypes(it.first.reward(), it.second) } + + // - In case choosing a reward with AddOns, from a NoReward Reward pledgeDataAndReasonWithAddOns .filter { it.second == PledgeReason.UPDATE_REWARD } + .compose, Reward>>(combineLatestPair(backedNoReward)) + .map { it.first } + .subscribe(this.showAddOnsFragment) + + pledgeDataAndReasonWithAddOns + .compose, Boolean>>(combineLatestPair(shouldShowAlert)) .compose(bindToLifecycle()) - .subscribe(this.showAlert) + .subscribe { data -> + if (data.second) this.showAlert.onNext(data.first) + else this.showAddOnsFragment.onNext(data.first) + } project .map { it.rewards()?.size?: 0 } @@ -158,6 +179,12 @@ class RewardsFragmentViewModel { } } + private fun differentShippingTypes(newRW: Reward, backedRW: Reward): Boolean = + if (newRW.id() == backedRW.id()) false + else { + newRW.shippingType()?.toLowerCase() ?: "" != backedRW.shippingType()?.toLowerCase() ?: "" + } + private fun pledgeDataAndPledgeReason(projectData: ProjectData, reward: Reward): Pair { val pledgeReason = if (projectData.project().isBacking) PledgeReason.UPDATE_REWARD else PledgeReason.PLEDGE val pledgeData = PledgeData.with(PledgeFlowContext.forPledgeReason(pledgeReason), projectData, reward) From 6f1d81b2e183e3d0e28564de8cbc6fb119c92666 Mon Sep 17 00:00:00 2001 From: Isabel Martin Date: Tue, 8 Sep 2020 13:48:00 -0700 Subject: [PATCH 03/11] - Get the default shippingRule for the selected reward, do not load the previously selected while backing --- .../viewmodels/BackingAddOnsFragmentViewModel.kt | 7 ------- 1 file changed, 7 deletions(-) diff --git a/app/src/main/java/com/kickstarter/viewmodels/BackingAddOnsFragmentViewModel.kt b/app/src/main/java/com/kickstarter/viewmodels/BackingAddOnsFragmentViewModel.kt index ddc67ebde3..f0f683039b 100644 --- a/app/src/main/java/com/kickstarter/viewmodels/BackingAddOnsFragmentViewModel.kt +++ b/app/src/main/java/com/kickstarter/viewmodels/BackingAddOnsFragmentViewModel.kt @@ -141,12 +141,6 @@ class BackingAddOnsFragmentViewModel { .filter { ObjectUtils.isNotNull(it) } .map { requireNotNull(it) } - backingShippingRule - .compose(bindToLifecycle()) - .subscribe { - this.shippingRuleSelected.onNext(it) - } - // - In case of digital Reward to follow the same flow as the rest of use cases use and empty shippingRule reward .filter { isDigital(it) } @@ -182,7 +176,6 @@ class BackingAddOnsFragmentViewModel { shippingRules .filter { it.isNotEmpty() } .compose, PledgeReason>>(combineLatestPair(pledgeReason)) - .filter { it.second == PledgeReason.PLEDGE } .switchMap { defaultShippingRule(it.first) } .subscribe(this.shippingRuleSelected) From 5b194fc5e38b72fa56c12707ca41bd4ec08aaa1a Mon Sep 17 00:00:00 2001 From: Isabel Martin Date: Tue, 8 Sep 2020 14:06:40 -0700 Subject: [PATCH 04/11] - Shipping Name emited before the entire ViewModel was loaded --- .../java/com/kickstarter/viewmodels/PledgeFragmentViewModel.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/com/kickstarter/viewmodels/PledgeFragmentViewModel.kt b/app/src/main/java/com/kickstarter/viewmodels/PledgeFragmentViewModel.kt index 14ad6e67d9..42241517a6 100644 --- a/app/src/main/java/com/kickstarter/viewmodels/PledgeFragmentViewModel.kt +++ b/app/src/main/java/com/kickstarter/viewmodels/PledgeFragmentViewModel.kt @@ -296,7 +296,7 @@ interface PledgeFragmentViewModel { private val newCardButtonClicked = PublishSubject.create() private val pledgeButtonClicked = PublishSubject.create() private val pledgeInput = PublishSubject.create() - private val shippingRule = PublishSubject.create() + private val shippingRule = BehaviorSubject.create() private val stripeSetupResultSuccessful = PublishSubject.create() private val stripeSetupResultUnsuccessful = PublishSubject.create() private val decreaseBonusButtonClicked = PublishSubject.create() From 3faa07d76f444a510bf974b283cee9520f38045b Mon Sep 17 00:00:00 2001 From: Isabel Martin Date: Tue, 8 Sep 2020 14:24:13 -0700 Subject: [PATCH 05/11] - Added tests to cover new cases --- .../RewardsFragmentViewModelTest.kt | 42 ++++++++++++++++--- 1 file changed, 37 insertions(+), 5 deletions(-) diff --git a/app/src/test/java/com/kickstarter/viewmodels/RewardsFragmentViewModelTest.kt b/app/src/test/java/com/kickstarter/viewmodels/RewardsFragmentViewModelTest.kt index 6418aceaa5..9fb6f7e21e 100644 --- a/app/src/test/java/com/kickstarter/viewmodels/RewardsFragmentViewModelTest.kt +++ b/app/src/test/java/com/kickstarter/viewmodels/RewardsFragmentViewModelTest.kt @@ -92,8 +92,8 @@ class RewardsFragmentViewModelTest: KSRobolectricTestCase() { } @Test - fun testShowAlert_whenBackingProject_withAddOns() { - val reward = RewardFactory.reward().toBuilder().hasAddons(true).build() + fun testShowAlert_whenBackingProject_withAddOns_sameReward() { + val reward = RewardFactory.rewardWithShipping().toBuilder().hasAddons(true).build() val backedProject = ProjectFactory.backedProject() .toBuilder() .backing(BackingFactory.backing() @@ -109,19 +109,51 @@ class RewardsFragmentViewModelTest: KSRobolectricTestCase() { this.vm.inputs.rewardClicked(reward) this.showPledgeFragment.assertNoValues() - this.showAddOnsFragment.assertNoValues() - this.showAlert.assertValue(Pair(PledgeData.builder() + this.showAddOnsFragment.assertValue(Pair(PledgeData.builder() .pledgeFlowContext(PledgeFlowContext.CHANGE_REWARD) .reward(reward) .projectData(ProjectDataFactory.project(backedProject)) .build(), PledgeReason.UPDATE_REWARD)) + this.showAlert.assertNoValues() + } + + @Test + fun testShowAlert_whenBackingProject_withAddOns_otherReward() { + val rewarda = RewardFactory.rewardWithShipping().toBuilder().id(4).hasAddons(true).build() + val rewardb = RewardFactory.rewardHasAddOns().toBuilder().id(2).hasAddons(true).build() + val backedProject = ProjectFactory.backedProject() + .toBuilder() + .backing(BackingFactory.backing() + .toBuilder() + .reward(rewardb) + .rewardId(rewardb.id()) + .build()) + .rewards(listOf(RewardFactory.noReward(), rewarda, rewardb)) + .build() + setUpEnvironment(environment()) + + this.vm.inputs.configureWith(ProjectDataFactory.project(backedProject)) + + this.vm.inputs.rewardClicked(rewarda) + this.showPledgeFragment.assertNoValues() + this.showAddOnsFragment.assertNoValues() + this.showAlert.assertNoValues() this.vm.inputs.alertButtonPressed() this.showPledgeFragment.assertNoValues() + this.showAddOnsFragment.assertNoValues() + this.showAlert.assertValue(Pair(PledgeData.builder() + .pledgeFlowContext(PledgeFlowContext.CHANGE_REWARD) + .reward(rewarda) + .projectData(ProjectDataFactory.project(backedProject)) + .build(), PledgeReason.UPDATE_REWARD)) + + this.vm.inputs.alertButtonPressed() this.showAddOnsFragment.assertValue(Pair(PledgeData.builder() .pledgeFlowContext(PledgeFlowContext.CHANGE_REWARD) - .reward(reward) + .reward(rewarda) .projectData(ProjectDataFactory.project(backedProject)) .build(), PledgeReason.UPDATE_REWARD)) + this.showPledgeFragment.assertNoValues() } @Test From 723124406059492efbbb8d56863916eef326af69 Mon Sep 17 00:00:00 2001 From: Isabel Martin Date: Tue, 8 Sep 2020 14:43:05 -0700 Subject: [PATCH 06/11] - remove unnecessary operator --- .../com/kickstarter/viewmodels/BackingAddOnsFragmentViewModel.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/app/src/main/java/com/kickstarter/viewmodels/BackingAddOnsFragmentViewModel.kt b/app/src/main/java/com/kickstarter/viewmodels/BackingAddOnsFragmentViewModel.kt index f0f683039b..cb047d8d47 100644 --- a/app/src/main/java/com/kickstarter/viewmodels/BackingAddOnsFragmentViewModel.kt +++ b/app/src/main/java/com/kickstarter/viewmodels/BackingAddOnsFragmentViewModel.kt @@ -154,7 +154,6 @@ class BackingAddOnsFragmentViewModel { .map { it.addOns()?.toList() } .filter { ObjectUtils.isNotNull(it) } .map { requireNotNull(it) } - .share() val addOnsFromGraph = project .switchMap { pj -> this.apolloClient.getProjectAddOns(pj.slug()?.let { it }?: "") } From bcf85259819d2deff2a682d2abbd18b779842468 Mon Sep 17 00:00:00 2001 From: Isabel Martin Date: Tue, 8 Sep 2020 14:55:58 -0700 Subject: [PATCH 07/11] - fix test --- .../com/kickstarter/viewmodels/RewardsFragmentViewModelTest.kt | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/src/test/java/com/kickstarter/viewmodels/RewardsFragmentViewModelTest.kt b/app/src/test/java/com/kickstarter/viewmodels/RewardsFragmentViewModelTest.kt index 9fb6f7e21e..decb1d2375 100644 --- a/app/src/test/java/com/kickstarter/viewmodels/RewardsFragmentViewModelTest.kt +++ b/app/src/test/java/com/kickstarter/viewmodels/RewardsFragmentViewModelTest.kt @@ -138,9 +138,6 @@ class RewardsFragmentViewModelTest: KSRobolectricTestCase() { this.showPledgeFragment.assertNoValues() this.showAddOnsFragment.assertNoValues() this.showAlert.assertNoValues() - this.vm.inputs.alertButtonPressed() - this.showPledgeFragment.assertNoValues() - this.showAddOnsFragment.assertNoValues() this.showAlert.assertValue(Pair(PledgeData.builder() .pledgeFlowContext(PledgeFlowContext.CHANGE_REWARD) .reward(rewarda) From 1d31068e97309c3803f6183b8ad13ee2e6ec54e5 Mon Sep 17 00:00:00 2001 From: Isabel Martin Date: Tue, 8 Sep 2020 15:13:48 -0700 Subject: [PATCH 08/11] - fix assert --- .../com/kickstarter/viewmodels/RewardsFragmentViewModelTest.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/app/src/test/java/com/kickstarter/viewmodels/RewardsFragmentViewModelTest.kt b/app/src/test/java/com/kickstarter/viewmodels/RewardsFragmentViewModelTest.kt index decb1d2375..550b417cf0 100644 --- a/app/src/test/java/com/kickstarter/viewmodels/RewardsFragmentViewModelTest.kt +++ b/app/src/test/java/com/kickstarter/viewmodels/RewardsFragmentViewModelTest.kt @@ -137,7 +137,6 @@ class RewardsFragmentViewModelTest: KSRobolectricTestCase() { this.vm.inputs.rewardClicked(rewarda) this.showPledgeFragment.assertNoValues() this.showAddOnsFragment.assertNoValues() - this.showAlert.assertNoValues() this.showAlert.assertValue(Pair(PledgeData.builder() .pledgeFlowContext(PledgeFlowContext.CHANGE_REWARD) .reward(rewarda) From 22b2f2b04ce1c2157b4530a5af94363d8fc53852 Mon Sep 17 00:00:00 2001 From: Isabel Martin Date: Wed, 9 Sep 2020 08:47:00 -0700 Subject: [PATCH 09/11] - PRComments --- .../viewmodels/RewardsFragmentViewModel.kt | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/com/kickstarter/viewmodels/RewardsFragmentViewModel.kt b/app/src/main/java/com/kickstarter/viewmodels/RewardsFragmentViewModel.kt index b12d18d364..316717385b 100644 --- a/app/src/main/java/com/kickstarter/viewmodels/RewardsFragmentViewModel.kt +++ b/app/src/main/java/com/kickstarter/viewmodels/RewardsFragmentViewModel.kt @@ -179,11 +179,12 @@ class RewardsFragmentViewModel { } } - private fun differentShippingTypes(newRW: Reward, backedRW: Reward): Boolean = - if (newRW.id() == backedRW.id()) false - else { - newRW.shippingType()?.toLowerCase() ?: "" != backedRW.shippingType()?.toLowerCase() ?: "" - } + private fun differentShippingTypes(newRW: Reward, backedRW: Reward): Boolean { + return if (newRW.id() == backedRW.id()) false + else { + newRW.shippingType()?.toLowerCase() ?: "" != backedRW.shippingType()?.toLowerCase() ?: "" + } + } private fun pledgeDataAndPledgeReason(projectData: ProjectData, reward: Reward): Pair { val pledgeReason = if (projectData.project().isBacking) PledgeReason.UPDATE_REWARD else PledgeReason.PLEDGE From 143513c78fea11033ac4ed2143284d1ca58e991d Mon Sep 17 00:00:00 2001 From: Isabel Martin Date: Wed, 9 Sep 2020 14:14:30 -0700 Subject: [PATCH 10/11] - Simplified logic for showing alert - Fixed when changing to a reward without addOns to a reward with addOns alert presented and also the pledgeFragment behind --- .../viewmodels/RewardsFragmentViewModel.kt | 103 ++++++++---------- 1 file changed, 43 insertions(+), 60 deletions(-) diff --git a/app/src/main/java/com/kickstarter/viewmodels/RewardsFragmentViewModel.kt b/app/src/main/java/com/kickstarter/viewmodels/RewardsFragmentViewModel.kt index 316717385b..c2ed7d26aa 100644 --- a/app/src/main/java/com/kickstarter/viewmodels/RewardsFragmentViewModel.kt +++ b/app/src/main/java/com/kickstarter/viewmodels/RewardsFragmentViewModel.kt @@ -10,6 +10,7 @@ import com.kickstarter.libs.rx.transformers.Transformers.takeWhen import com.kickstarter.libs.utils.BackingUtils import com.kickstarter.libs.utils.ObjectUtils import com.kickstarter.mock.factories.RewardFactory +import com.kickstarter.models.Backing import com.kickstarter.models.Project import com.kickstarter.models.Reward import com.kickstarter.ui.data.PledgeData @@ -79,22 +80,10 @@ class RewardsFragmentViewModel { .map { it.project() } val backedReward = project - .map { it.backing()?.reward() } + .map { it.backing()?.let { backing -> getReward(backing) } } .filter { ObjectUtils.isNotNull(it) } .map { requireNotNull(it) } - val backedNoReward = project - .filter { it.backing() != null && it.backing()?.reward() == null } - .map { RewardFactory.noReward() } - - val backedAddOns = project - .map { it?.backing()?.addOns() } - .filter { ObjectUtils.isNotNull(it) } - .map { requireNotNull(it) } - - val hasAddOnsBacked = backedAddOns - .map { it.isNotEmpty() } - project .filter { it.isBacking } .map { indexOfBackedReward(it) } @@ -102,66 +91,53 @@ class RewardsFragmentViewModel { .compose(bindToLifecycle()) .subscribe(this.backedRewardPosition) - // - If the selected Reward to update to do not have AddOns val pledgeDataAndReason = this.projectDataInput .compose>(Transformers.takePairWhen(this.rewardClicked)) - .filter { !it.second.hasAddons() } .map { pledgeDataAndPledgeReason(it.first, it.second) } + .distinctUntilChanged() pledgeDataAndReason .filter { it.second == PledgeReason.PLEDGE} - .compose(bindToLifecycle()) - .subscribe(this.showPledgeFragment) - - pledgeDataAndReason - .compose, Reward>>(combineLatestPair(backedReward)) - .filter { !it.second.hasAddons() } - .map { it.first } - .compose(bindToLifecycle()) - .subscribe(this.showPledgeFragment) - - pledgeDataAndReason - .filter { it.second == PledgeReason.UPDATE_REWARD } - .compose, Boolean>>(combineLatestPair(hasAddOnsBacked)) + .compose>(takeWhen(this.rewardClicked)) .compose(bindToLifecycle()) .subscribe { - if (!it.second && !it.first.first.reward().hasAddons()) - this.showPledgeFragment.onNext(it.first) - else this.showAlert.onNext(it.first) - } + val rw = it.first.reward() - // - If the selected Reward have AddOns - val pledgeDataAndReasonWithAddOns = this.projectDataInput - .compose>(Transformers.takePairWhen(this.rewardClicked)) - .filter { it.second.hasAddons() } - .map { pledgeDataAndPledgeReason(it.first, it.second) } + if (rw.hasAddons()) + this.showAddOnsFragment.onNext(it) + else + this.showPledgeFragment.onNext(it) - pledgeDataAndReasonWithAddOns - .filter { it.second == PledgeReason.PLEDGE } - .compose(bindToLifecycle()) - .subscribe { - this.showAddOnsFragment.onNext(it) } - val shouldShowAlert = pledgeDataAndReasonWithAddOns - .filter { it.second == PledgeReason.UPDATE_REWARD } - .map { it.first } - .compose>(combineLatestPair(backedReward)) - .map { differentShippingTypes(it.first.reward(), it.second) } - - // - In case choosing a reward with AddOns, from a NoReward Reward - pledgeDataAndReasonWithAddOns - .filter { it.second == PledgeReason.UPDATE_REWARD } - .compose, Reward>>(combineLatestPair(backedNoReward)) - .map { it.first } - .subscribe(this.showAddOnsFragment) - - pledgeDataAndReasonWithAddOns - .compose, Boolean>>(combineLatestPair(shouldShowAlert)) + pledgeDataAndReason + .compose>(takeWhen(this.rewardClicked)) + .compose, Reward>>(combineLatestPair(backedReward)) .compose(bindToLifecycle()) - .subscribe { data -> - if (data.second) this.showAlert.onNext(data.first) - else this.showAddOnsFragment.onNext(data.first) + .subscribe { + val pledgeAndData = it.first + val newRw = it.first.first.reward() + val prevRw = it.second + val reason = it.first.second + + when(reason) { + PledgeReason.UPDATE_REWARD -> { + if (prevRw.hasAddons() && !newRw.hasAddons()) + this.showAlert.onNext(pledgeAndData) + + if (!prevRw.hasAddons() && !newRw.hasAddons()) + this.showPledgeFragment.onNext(pledgeAndData) + + if (prevRw.hasAddons() && newRw.hasAddons()) { + if (differentShippingTypes(prevRw, newRw)) this.showAlert.onNext(it.first) + else this.showAddOnsFragment.onNext(pledgeAndData) + } + + if (!prevRw.hasAddons() && newRw.hasAddons()) { + this.showAddOnsFragment.onNext(pledgeAndData) + } + } + } } project @@ -179,6 +155,13 @@ class RewardsFragmentViewModel { } } + private fun getReward(backingObj: Backing): Reward { + return backingObj.reward()?.let { rw -> + if (backingObj.addOns().isNullOrEmpty()) rw + else rw.toBuilder().hasAddons(true).build() + } ?: RewardFactory.noReward() + } + private fun differentShippingTypes(newRW: Reward, backedRW: Reward): Boolean { return if (newRW.id() == backedRW.id()) false else { From cfc9bdfce6574e568cf42b9f56b0b1b4651df062 Mon Sep 17 00:00:00 2001 From: Isabel Martin Date: Thu, 10 Sep 2020 12:50:28 -0700 Subject: [PATCH 11/11] [NT-1474]: Refresh backing fragment (#976) --- .../ui/adapters/RewardAndAddOnsAdapter.kt | 6 ++---- .../com/kickstarter/ui/fragments/BackingFragment.kt | 3 ++- .../com/kickstarter/ui/fragments/RewardsFragment.kt | 7 ++++--- .../com/kickstarter/viewmodels/ProjectViewModel.kt | 13 ++++++++++--- .../kickstarter/viewmodels/ProjectViewModelTest.kt | 2 ++ 5 files changed, 20 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/com/kickstarter/ui/adapters/RewardAndAddOnsAdapter.kt b/app/src/main/java/com/kickstarter/ui/adapters/RewardAndAddOnsAdapter.kt index 12dcc96860..f7e880f57b 100644 --- a/app/src/main/java/com/kickstarter/ui/adapters/RewardAndAddOnsAdapter.kt +++ b/app/src/main/java/com/kickstarter/ui/adapters/RewardAndAddOnsAdapter.kt @@ -29,10 +29,8 @@ class RewardAndAddOnsAdapter() : KSAdapter() { } fun populateDataForAddOns(rewards: List>) { - if (rewards.isNotEmpty()) { - setSection(SECTION_ADD_ONS_CARD, rewards) - notifyDataSetChanged() - } + setSection(SECTION_ADD_ONS_CARD, rewards) + notifyDataSetChanged() } fun populateDataForReward(reward: Pair) { diff --git a/app/src/main/java/com/kickstarter/ui/fragments/BackingFragment.kt b/app/src/main/java/com/kickstarter/ui/fragments/BackingFragment.kt index 862b14fae4..8558aa1f34 100644 --- a/app/src/main/java/com/kickstarter/ui/fragments/BackingFragment.kt +++ b/app/src/main/java/com/kickstarter/ui/fragments/BackingFragment.kt @@ -22,6 +22,7 @@ import com.kickstarter.libs.SwipeRefresher import com.kickstarter.libs.qualifiers.RequiresFragmentViewModel import com.kickstarter.libs.rx.transformers.Transformers import com.kickstarter.libs.transformations.CircleTransformation +import com.kickstarter.libs.utils.ObjectUtils import com.kickstarter.libs.utils.ViewUtils import com.kickstarter.models.Reward import com.kickstarter.ui.adapters.RewardAndAddOnsAdapter @@ -184,7 +185,7 @@ class BackingFragment : BaseFragment() { .subscribe { total_summary_amount.text = it } this.viewModel.outputs.projectDataAndAddOns() - .filter { it.second.isNotEmpty() } + .filter { ObjectUtils.isNotNull(it) } .distinctUntilChanged() .compose(bindToLifecycle()) .compose(Transformers.observeForUI()) diff --git a/app/src/main/java/com/kickstarter/ui/fragments/RewardsFragment.kt b/app/src/main/java/com/kickstarter/ui/fragments/RewardsFragment.kt index 57e9ecc0a4..b8ac77addd 100644 --- a/app/src/main/java/com/kickstarter/ui/fragments/RewardsFragment.kt +++ b/app/src/main/java/com/kickstarter/ui/fragments/RewardsFragment.kt @@ -95,7 +95,8 @@ class RewardsFragment : BaseFragment(), Rewa } private fun showAlert() { - dialog.show() + if (this.isVisible) + dialog.show() } private fun scrollToReward(position: Int) { @@ -140,7 +141,7 @@ class RewardsFragment : BaseFragment(), Rewa } private fun showPledgeFragment(pledgeData: PledgeData, pledgeReason: PledgeReason) { - if (this.fragmentManager?.findFragmentByTag(PledgeFragment::class.java.simpleName) == null) { + if (this.isVisible && this.fragmentManager?.findFragmentByTag(PledgeFragment::class.java.simpleName) == null) { val pledgeFragment = PledgeFragment.newInstance(pledgeData, pledgeReason) this.fragmentManager?.beginTransaction() ?.setCustomAnimations(R.anim.slide_in_right, 0, 0, R.anim.slide_out_right) @@ -153,7 +154,7 @@ class RewardsFragment : BaseFragment(), Rewa } private fun showAddonsFragment(pledgeDataAndReason: Pair) { - if (this.fragmentManager?.findFragmentByTag(BackingAddOnsFragment::class.java.simpleName) == null) { + if (this.isVisible && this.fragmentManager?.findFragmentByTag(BackingAddOnsFragment::class.java.simpleName) == null) { val addOnsFragment = BackingAddOnsFragment.newInstance(pledgeDataAndReason) this.fragmentManager?.beginTransaction() ?.setCustomAnimations(R.anim.slide_in_right, 0, 0, R.anim.slide_out_right) diff --git a/app/src/main/java/com/kickstarter/viewmodels/ProjectViewModel.kt b/app/src/main/java/com/kickstarter/viewmodels/ProjectViewModel.kt index 53c50f30b2..428e945df6 100644 --- a/app/src/main/java/com/kickstarter/viewmodels/ProjectViewModel.kt +++ b/app/src/main/java/com/kickstarter/viewmodels/ProjectViewModel.kt @@ -545,20 +545,27 @@ interface ProjectViewModel { val backing = backedProject .switchMap { this.apolloClient.getProjectBacking(it.slug()?: "") + .doOnSubscribe { + progressBarIsGone.onNext(false) + } + .doAfterTerminate { + progressBarIsGone.onNext(true) + } + .materialize() } .compose(neverError()) + .compose(values()) .filter { ObjectUtils.isNotNull(it) } .share() // - Update fragments with the backing data projectData - .filter { it.project().hasRewards() && it.project().isBacking } + .filter { it.project().hasRewards() } .compose>(combineLatestPair(backing)) .map { val updatedProject = it.first.project().toBuilder().backing(it.second).build() projectData(it.first.refTagFromIntent(), it.first.refTagFromCookie(), updatedProject) } - .distinctUntilChanged() .compose(bindToLifecycle()) .subscribe(this.updateFragments) @@ -683,7 +690,7 @@ interface ProjectViewModel { this.fragmentStackCount .compose>(combineLatestPair(currentProject)) - .map { if (it.second.isBacking) it.first > 3 else it.first > 3} + .map { if (it.second.isBacking) it.first > 4 else it.first > 3 } .distinctUntilChanged() .compose(bindToLifecycle()) .subscribe(this.scrimIsVisible) diff --git a/app/src/test/java/com/kickstarter/viewmodels/ProjectViewModelTest.kt b/app/src/test/java/com/kickstarter/viewmodels/ProjectViewModelTest.kt index 8fc96b5764..c36c2d89f5 100644 --- a/app/src/test/java/com/kickstarter/viewmodels/ProjectViewModelTest.kt +++ b/app/src/test/java/com/kickstarter/viewmodels/ProjectViewModelTest.kt @@ -1388,6 +1388,7 @@ class ProjectViewModelTest : KSRobolectricTestCase() { ProjectDataFactory.project(refreshedProject)) this.showUpdatePledgeSuccess.assertValueCount(1) this.updateFragments.assertValues(ProjectDataFactory.project(initialBackedProject), + ProjectDataFactory.project(refreshedProject), ProjectDataFactory.project(refreshedProject)) } @@ -1413,6 +1414,7 @@ class ProjectViewModelTest : KSRobolectricTestCase() { ProjectDataFactory.project(refreshedProject)) this.showUpdatePledgeSuccess.assertValueCount(1) this.updateFragments.assertValues(ProjectDataFactory.project(initialBackedProject), + ProjectDataFactory.project(refreshedProject), ProjectDataFactory.project(refreshedProject)) }