8000 NT-1141: Refactor existing queries by Arkariang · Pull Request #897 · kickstarter/android-oss · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

NT-1141: Refactor existing queries #897

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 3 commits into from
Jun 9, 2020
Merged

NT-1141: Refactor existing queries #897

merged 3 commits into from
Jun 9, 2020

Conversation

Arkariang
Copy link
Contributor

📲 What

Refactor in queries Project.backing and Backing.backing to use fragments for each subtype

🤔 Why

Reusability - Readability - No code smells

🛠 How

Creating separate fragments for each type and not attaching them to any high level query.

👀 See

No UI chenges here

📋 QA

  • Check one of your backed projects, hit the manage pledge button, all data should remain the same as before
  • Go to any message os your messages with a creator of a project you already backed, go to the thread view, hit the view pledge button, all data should remain as before.
  • Log in as creator if you can, go to any message you have with a baker to one of your projects, hit the view pledge, you should see the pledge data as before.

justinswart
justinswart approved these changes Jun 9, 2020
View reviewed changes
Copy link
Contributor
@justinswart justinswart left a comment

Choose a reason for hiding this comment

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

lgtm!

app/src/main/java/com/kickstarter/services/KSApolloClient.kt Outdated
val shippingAmount = backingGr?.shippingAmount()?.fragments()

val reward = backingGr?.reward()?.fragments()?.reward()?.let { reward ->
val rewardId = decodeRelayId(reward.id()) ?: 0
Copy link
Contributor

Choose a reason for hiding this comment

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

I would coalesce this to -1 just in case. There is some historical behaviour where a Reward with and ID of 0 is treated as "No Reward" 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

got it!

jgsamudio
jgsamudio reviewed Jun 9, 2020
View reviewed changes
app/src/main/java/com/kickstarter/services/KSApolloClient.kt
.pledgedAt(backingGr?.pledgedOn())
.projectId(projectId)
.sequence(backingGr?.sequence()?.toLong() ?: 0)
.shippingAmount(backingGr?.shippingAmount()?.amount().toString().toFloat())
.shippingAmount(shippingAmount?.amount()?.amount()?.toFloat() ?: 0f)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this still be optional or should be force to 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's really never gonna 0 but retrieving it from Graph force it to be optional because of the unwrapping. I'll change the defaults as Justin suggested to -1, that would be clear that if we encounter that value something went wrong

jgsamudio
jgsamudio reviewed Jun 9, 2020
View reviewed changes
app/src/main/java/com/kickstarter/viewmodels/BackingFragmentViewModel.kt
@@ -189,6 +189,7 @@ interface BackingFragmentViewModel {
.subscribe(this.backerAvatar)

this.projectDataInput
.filter { it.project().isBacking || ProjectUtils.userIsCreator(it.project(), it.user()) }
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add a test for this or is it already covered by another test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great point, I'll add a test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! two new tests,

  • one checking as creator being able to see the reward of the backer.
  • one for checking no rewad information if the user is neither a creator or a backer

@Arkariang
- PRComments
e54a4bf 
- Added two tests
@Arkariang Arkariang merged commit 9f396ce into feature/NT-1141-add-ons Jun 9, 2020
@Arkariang Arkariang deleted the imartin/refactor-queries branch June 9, 2020 17:37
JadeByfield89 pushed a commit that referenced this pull request Jul 22, 2020
@Arkariang
NT-1141: Refactor existing queries (#897)
7530604 
- Refactor queries in a more re-usable way
- When seeing a project not being a backer, backing object is null in the request
- one checking as creator being able to see the reward of the backer.
- one for checking no rewad information if the user is neither a creator or a backer
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: leighdouglas <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 left review comments

@justinswart justinswart justinswart approved these changes

@JadeByfield89 JadeByfield89 Awaiting requested review from JadeByfield89

Assignees
No one assigned
Labels
maintenance 🗜 refactor 🛠
Projects
None yet
Milestone
No milestone
Development

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