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

[MBL-1491] [MBL-1663] Handle 3DS authentication with informational snackbar #2236

Merged
merged 14 commits into from
Jan 15, 2025

Conversation

stevestreza-ksr
Copy link
Contributor

@stevestreza-ksr stevestreza-ksr commented Jan 9, 2025

📲 What

This PR adds support for letting a user handle 3DS authentication via Stripe.

🤔 Why

Stripe won't process some transactions otherwise.

🛠 How

When the user taps the "authorize card" button, I added a loading indicator to the button and updated the enum cases to include a way to turn that loading indicator off.

Then I added fetching the client secret from GraphQL and passing that through the enum case for triggering 3DS authentication.

With those pieces in place, I updated PPOViewModel to call Stripe's authentication check. In debug mode, this is simulated for ease of testing. When the callback completes, the app informs the user that the call has either succeeded or failed. This was done with a touched up version of the MessageBannerView that already existed and had some basic hookups in the PPOViewModel.

If successful, the item is hidden. This was done by adding filtering capability to PPOViewModel and tying it to the existing Publisher chain for populating results.

I also added some convenience helpers for clarity, such as withEmptyValues() on Publisher, a pattern that I was using in a few places, and a way to map the values of a Paginator's Results (in this case, to facilitate filtering).

👀 See

Screen Recording 2025-01-08 at 23 01 46

Figma

✅ Acceptance criteria

Tests should pass.

End-to-end testing should succeed.

Copy link
Contributor

@amy-at-kickstarter amy-at-kickstarter left a comment

Choose a reason for hiding this comment

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

LGTM.

I have some thoughts/suggestions/nits/discussions, but nothing that's blocking.

Copy link
Contributor

@ifosli ifosli left a comment

Choose a reason for hiding this comment

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

I like the banner changes - very nice! I am concerned about this approach to 3DS - I know we talked about the complication with where to present what (considering banners) before, but it feels weird to me to have a view model presenting UI instead of a view controller. I trust your judgement, though. But I do think it'd be helpful to test this end to end now, while you still have alternatives fresh in your mind, in case the 3DS UI doesn't get presented correctly or something.

@stevestreza-ksr stevestreza-ksr force-pushed the stevestreza/ppo/handle-3ds-auth branch 3 times, most recently from 6082cf0 to c21dfa9 Compare January 14, 2025 21:28
@nativeksr
Copy link
Collaborator

nativeksr commented Jan 14, 2025

1 Warning
⚠️ Big PR

SwiftFormat found issues:

File Rules
Kickstarter-iOS/Features/PledgedProjectsOverview/PPOContainerViewController.swift:181:1 warning: (blankLinesAtStartOfScope) Remove leading blank line at the start of a scope.
Kickstarter-iOS/Features/PledgedProjectsOverview/PPOContainerViewController.swift:227:1 warning: (unusedArguments) Mark unused function arguments with _.

Generated by 🚫 Danger

Copy link
Contributor

@amy-at-kickstarter amy-at-kickstarter left a comment

Choose a reason for hiding this comment

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

LGTM! I like the refactor. A couple non-blocking nits and commentary.

@stevestreza-ksr stevestreza-ksr force-pushed the stevestreza/ppo/handle-3ds-auth branch from 4e6303d to b11d9ef Compare January 15, 2025 18:44
@stevestreza-ksr stevestreza-ksr force-pushed the stevestreza/ppo/handle-3ds-auth branch from b11d9ef to e0656e7 Compare January 15, 2025 19:11
@stevestreza-ksr stevestreza-ksr merged commit 64d5971 into main Jan 15, 2025
5 checks passed
@stevestreza-ksr stevestreza-ksr deleted the stevestreza/ppo/handle-3ds-auth branch January 15, 2025 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants