Skip to content
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

[PAY-1829] Card Selection With Newly Created Payment Sheet Cards #1711

Merged
merged 8 commits into from Aug 12, 2022

Conversation

msadoon
Copy link
Contributor

@msadoon msadoon commented Aug 10, 2022

📲 What

This PR address the selection logic of picking a card after it has been added via the new payment sheet.

🤔 Why

User should see newly added card for project with/without backing pre-selected. User should also be able to pick between pre-existing cards and newly created ones. (Both with the payment sheet and no).

🛠 How

configuredCards is updated with new setup intent card data newSetupIntentCards after the flow controller has returned the display data (it's coupled with the setup intent for that display data only.)

updatedCards is now based on configuredCards instead of cards, after its' been combined with newSetupIntentCards. Previously updatedCards was used with didSelectRowAt which is the same as now, except the setup intent card data is combined in here as well.

Previous flow:

cards = user's fetched cards + new cards added through add new card page
configuredCards = cards (after some card selection logic)
configuredCards --> reloadPaymentMethods
updatedCards = didSelectRowAt + cards
updatedCards --> reloadPaymentMethods

New flow:

cards = user's fetched cards + new cards added through add new card page
configuredCards = cards (after some card selection logic)

setupIntentConfiguredCards = configuredCards OR (inclusive) newSetupIntentCards (re-select correct card if setup intent cards added)
setupIntentConfiguredCards --> reloadPaymentMethods

updatedCards = setupIntentConfiguredCards OR (inclusive) configuredCards
updatedCards + didSelectRowAt (re-select cards if setup intent cards were added) --> reloadPaymentMethods

Whenever setup intent card logic is combined with pre-existing cards, we re-configure the pledge display data for either one or the other card type display, because they are two different models.

Also the payment source id OR the setup intent is passed to the pledge view for making the backing.

👀 See

Trello, screenshots, external resources?

After 🐛

Simulator.Screen.Recording.-.iPhone.8.-.2022-08-11.at.15.33.51.mp4

Before 🦋

Simulator.Screen.Recording.-.iPhone.8.-.2022-08-11.at.15.38.31.mp4

✅ Acceptance criteria

With ios_payment_sheet ON

  • User is able to add a new card through payment sheet, select it (or any pre-existing card) within the pledge page only for the .pledge context

With ios_payment_sheet OFF

  • User is able to add a new card through add new card screen, select it (or any pre-existing card) within the pledge page only for the .pledge context

⏰ TODO

  • Allow manual selection of new payment sheet cards
  • Not passing the CreateBackingData to PledgeViewModel in this ticket, as that is more aligned with the creation of the backing in the next ticket.

…etup intent methods will update existing signals in pledge payment methods view and pledge view.
@msadoon msadoon self-assigned this Aug 10, 2022
@msadoon msadoon requested a review from a team August 10, 2022 19:56
@msadoon msadoon added the WIP label Aug 10, 2022
@msadoon msadoon added this to the release-5.5.0 milestone Aug 10, 2022
@msadoon
Copy link
Contributor Author

msadoon commented Aug 10, 2022

@scottkicks Hey Scott, just getting the hang of adding you to new PR's -- no need to review it or anything, feel free to post questions/comments if you want.

@nativeksr
Copy link
Collaborator

1 Warning
⚠️ Big PR

Generated by 🚫 Danger

@@ -8,6 +8,7 @@ public struct CreateBackingInput: GraphMutationInput, Encodable {
let projectId: String
let refParam: String?
let rewardIds: [String]
let setupIntentClientSecret: String?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Useful in the next ticket.

@msadoon msadoon marked this pull request as ready for review August 12, 2022 03:35
@msadoon msadoon added needs review and removed WIP labels Aug 12, 2022
@codecov
Copy link

codecov bot commented Aug 12, 2022

Codecov Report

Merging #1711 (b9c6ca6) into main (0049919) will increase coverage by 0.03%.
The diff coverage is 91.27%.

@@            Coverage Diff             @@
##             main    #1711      +/-   ##
==========================================
+ Coverage   85.23%   85.26%   +0.03%     
==========================================
  Files        1269     1269              
  Lines      113329   113813     +484     
  Branches    30068    30143      +75     
==========================================
+ Hits        96598    97046     +448     
- Misses      15684    15721      +37     
+ Partials     1047     1046       -1     
Impacted Files Coverage Δ
...ws/Cells/PledgePaymentSheetPaymentMethodCell.swift 0.00% <0.00%> (ø)
...PledgePaymentSheetPaymentMethodCellViewModel.swift 0.00% <0.00%> (ø)
...ntrollers/PledgePaymentMethodsViewController.swift 49.15% <7.14%> (-3.58%) ⬇️
...ary/ViewModels/PledgePaymentMethodsViewModel.swift 96.22% <95.02%> (-0.72%) ⬇️
...iewModels/PledgePaymentMethodsViewModelTests.swift 97.49% <95.90%> (-0.88%) ⬇️
...aSources/PledgePaymentMethodsDataSourceTests.swift 99.12% <100.00%> (+0.23%) ⬆️
...aphAPI.CreateBackingInput+CreateBackingInput.swift 100.00% <100.00%> (ø)
...I.CreateBackingInput+CreateBackingInputTests.swift 100.00% <100.00%> (ø)
KsApi/mutations/inputs/CreateBackingInput.swift 100.00% <100.00%> (ø)
...Api/mutations/inputs/CreateBackingInputTests.swift 100.00% <100.00%> (ø)
... and 2 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@msadoon msadoon merged commit 86a89f2 into main Aug 12, 2022
@msadoon msadoon deleted the pay-1829/select-payment-method branch August 12, 2022 05:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants