8000 [NT-1417][NT-1510]:Fix change reward flow by Arkariang · Pull Request #975 · kickstarter/android-oss · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[NT-1417][NT-1510]:Fix change reward flow #975

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 11 commits into from
Sep 10, 2020
Merged

[NT-1417][NT-1510]:Fix change reward flow #975

merged 11 commits into from
Sep 10, 2020

Conversation

Arkariang
Copy link
Contributor
@Arkariang Arkariang commented Sep 8, 2020 •
edited
Loading

📲 What

  • We were using the backing shipping location when choosing a new reward, that's incorrect, the shippingLocation should be always the one from the selected reward, except if it's digital, then hide shipping selector.
  • Unified shippingType & shippingPreference for both Reward from V1 and Reward from Graph. When doing a pledge for the first time the model for Reward is from V1, when taking the reward from the backingObject the model is from Graph there where some discrepancies on the ShippingPreference and shippingType.

🛠 How

  • In case the reward has restrited shipping rules, and none of those rules matches our default location, we load the first one of the Reward shipping rules
  • In case the reward is unrestricted, load the defaultLocation
    Those two AC are true for both flows new pledge and choosing new reward.
  • Updated logic for showing the Native alert on choosing another reward flow to check the id of the selected reward not the same as the backed one (Do not show alert in this case)
  • Updated logic for showing the Native alert to be shown if the selected reward has different shipping rules from the backed one.
  • show the alert in case we move from a reward with AddOns to a NoReward reward.
  • showing back the name of the selected shippingrule in PledgeFragment.

👀 See

| Before 🐛 | After 🦋 |
change-reward-flow

| | |

📋 QA

  • Update reward from digital to unrestricted with addOns
  • update reward from unrestricted to restricted
  • update reward from restricted to digital
  • updated from any of the previous one to a digital without addOns
  • updated from any of the previous one to a regular reward without addOns
  • updated from any of the previous one to a NoReward

Story 📖

NT-1417
NT-1510

@Arkariang
- Bug when updating reward, the reward used when changing reward was …
c484343 
…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
@Arkariang
- Updated to be able to show the alert if changing from a selectedRew…
f2ec283 
…ard with location to another without location
@Arkariang
- Get the default shippingRule for the selected reward, do not load t…
6f1d81b 
…he previously selected while backing
@Arkariang
- Shipping Name emited before the entire ViewModel was loaded
5b194fc
@Arkariang
- Added tests to cover new cases
3faa07d
@Arkariang
- remove unnecessary operator
7231244
@Arkariang
- fix test
bcf8525
@Arkariang
- fix assert
1d31068
@Arkariang Arkariang marked this pull request as ready for review September 8, 2020 22:50
@Arkariang Arkariang changed the title NT-1417:Fix change reward flow [NT-1417][NT-1510]:Fix change reward flow Sep 8, 2020
@jgsamudio
Copy link
Contributor

When selecting another reward that is not the currently selected reward, the app transitions to the manage pledge screen and also presents the alert. We want to present the alert without transitioning to the next screen. Only when the user selects, "yes, continue" that we want to transition to the next screen.

@Arkariang
- PRComments
22b2f2b
@Arkariang
- Simplified logic for showing alert
143513c 
- Fixed when changing to a reward without addOns to a reward with addOns alert presented and also the pledgeFragment behind
@Arkariang Arkariang requested a review from jgsamudio September 9, 2020 21:22
@Arkariang
Copy link
Contributor Author

When selecting another reward that is not the currently selected reward, the app transitions to the manage pledge screen and also presents the alert. We want to present the alert without transitioning to the next screen. Only when the user selects, "yes, continue" that we want to transition to the next screen.

Fixed! and simplified logic :)

jgsamudio
jgsamudio approved these changes Sep 9, 2020
View reviewed changes
Copy link
Contributor
@jgsamudio jgsamudio left a comment

Choose a reason for hiding this comment

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

LGTM! Great Work!

@Arkariang Arkariang merged commit ece4887 into imartin/fix-amounts-with-addOns Sep 10, 2020
@Arkariang Arkariang deleted the imartin/NT-1417-fix-change-reward-flow branch September 10, 2020 19:50
Arkariang added a commit that referenced this pull request Sep 10, 2020
@Arkariang
[NT-1453][NT-1344][NT-1399][NT-1509]: Fix Pledge with Digital addOns (#…
0a2ae1d 
…974)

- NT-1470 & NT-1471
-- Update visible elements configuration when Changing payment methods
-- Update visible elements configuration when Updating Pledge

** bugs fixed on BackingAddOns
- Once AddOns selected going to plegdeFragment, go back to AddOns screen and change any selection, that update forced to present again PledgeFragment

* [NT-1417][NT-1510]:Fix change reward flow (#975)
Arkariang added a commit that referenced this pull request Oct 15, 2020
@Arkariang @JadeByfield89
@leighdouglas @jgsamudio
NT-1382: AddOns (#1017)
e5eb628 
* NT-1231 Creator Perspective query + View your pledge updated (#895)
* NT-1141: Refactor existing queries (#897)
* NT-1294: RecyclerView structure for reward & Add-ons (#896)
* NT-1222:Retrieve Add-ons data from Graph (#899)
* NT- 1290: Add-ons UI in BackingFragment (#900)
* NT-1171: Manage Pledge View Backing Info updates (#902)
* NT-1290: Add-ons UI(with real data from staging) (#905)
* NT-1326: Pledge header ui just for rewards (#906)
* NT-1327: Header animation (#911)
* NT-1171: (Fixed) Manage Pledge View Backing Info updates (#912)
* NT-1326 : Add total amount back to the header (#914)
* NT-1326: Reward Title going two lines (#918)
* NT-1345: Bonus support validation fixes (#917)
* NT-1345: Bonus support validation fixes (#917)
* NT-1383: New add-ons screen (#923)
* NT-1384: Add-ons list (#925)
* NT-1384- AddOns query (#927)
* NT-1411: Creator crash fixed (#929)
* NT-1385: Add Ons Card UI  (#930)
* NT-1386: Stepper UI on Add-On card (#937)
* NT-1422 && NT-1402 && NT-1383 && NT-1385 && NT-1382: Fix rejected (#946)
* NT-1386:Rejected fix (#947)
* NT-1381: Pledge header design polish (#948)
* NT-1426: Android Manage Pledge View Design Polish (#949)
* NT-1380 Bonus support design polish (#950)
* NT-1344: Updated checkout with new mutation  (#951)
* NT-1344: Updated checkout with new mutation 
* NT- 1462&& NT-1463: UpdateBacking Mutation  (#952)
* NT-1462:Update pledge flow (#954)
* NT-1445: Selected Reward Tag  (#955)
* NT-1390: Previously selected reward logic (#956)
* NT-1440:Display Empty State when add-ons unavailable (#961)
* NT-1453:Maximum pledge string updates (#962)
* NT-1460:Update add ons selection (#959)
* NT-1417: Show a native alert during Edit Reward if changes would remove add ons (#965)
* NT-1387: Fix Add-Ons Available tag for Digital Reward (#970)
* NT-1387: Fix - Select AddOns for digital reward (#971)
* [NT-1453][NT-1344][NT-1399][NT-1509]: Fix Pledge with Digital addOns  (#974)
* [NT-1417][NT-1510]:Fix change reward flow (#975)
* NT-1442: Android Display an error state if add-ons fail to load (#980)
* NT-1516: Update the Bonus Support Base Amount (#981)
* NT-1534: Sold-out add-ons (#983)
* [NT-1453] Bonus Support Max Pledge String Fix (#987)
* NT-1442: Hide shipping selector on network error (#986)
* NT-1471 | NT-1539 - Prevent Add-ons Alert & Add Pledge Label (#991)
* NT-1549 : Choose another reward flow (#993)
* NT-1539: Unprompted Edit Rewards Alert (#994)
* NT-1541: Updated Add-ons Quantity Not Updated (#995)
* NT-1534: Android Sold-out add-ons fix (#996)
* NT-1534: Modify previously backed AddOns if unavailable (#1005)
* NT-606: FIX Display a string indicating backing state for creators on View/Manage Pledge screen 
* NT-1585:Reward with starting time restriction not started yet. (#1009)
* https://kickstarter.atlassian.net/browse/NT-1587 (#1011)
* NT- 1591:Blanck screen edit reward flow (#1012)
* NT-1531: Android Refactor RewardUtils Java to Kotlin (#1008)
* NT-1587: Improve performance (#1013)
* NT-1587: Query with filtering for shippingRules (#1015)

Co-authored-by: Jade Byfield <jjbyfield@gmail.com>
Co-authored-by: Jade Byfield <jade@thoughtbot.com>
Co-authored-by: l
6B8A
eighdouglas <leighcdouglas1@gmail.com>
Co-authored-by: jgsamudio <jonathan2457@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jgsamudio jgsamudio jgsamudio approved these changes

@leighdouglas leighdouglas Awaiting requested review from leighdouglas

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Arkariang @jgsamudio
0