Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[MBL-1613] Handle errors for late pledges without dismissing the view controller #2171

Merged
merged 4 commits into from
Oct 14, 2024

Conversation

ifosli
Copy link
Contributor

@ifosli ifosli commented Oct 9, 2024

📲 What

Handle errors in the late pledge flow without a confirm details page by just showing an error banner (not dismissing the vc). The error banner will persist if the error happens when creating the checkout. If it's an error that happens in a part of the flow the user can retry (after hitting "pledge" or adding a new card) the banner disappears.

👀 See

Jira

Simulator Screen Recording - iPhone 16 Pro Max - 2024-10-09 at 15 37 42

✅ Acceptance criteria

  • The same errors show as before, but they no longer navigate backwards
  • If in debug, the server error always shows in addition to the general "Something went wrong"
  • I tested the "checkout error causes banner to persist" by hardcoding the view model to say it happened. So I've tested all the UI changes, but I haven't tried to generate new (real) errors.

@ifosli ifosli force-pushed the removeConfirmDetailsErrorHandling branch from ad875bf to 5ec631f Compare October 9, 2024 21:45
@ifosli ifosli self-assigned this Oct 9, 2024
@ifosli ifosli marked this pull request as ready for review October 10, 2024 15:16
@ifosli ifosli requested a review from scottkicks October 10, 2024 15:16
Copy link
Contributor

@scottkicks scottkicks left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@@ -34,12 +34,11 @@ public protocol NoShippingPostCampaignCheckoutViewModelOutputs {
var goToLogin#: Signal<(LoginIntent, Project, Reward), Never> { get }
var paymentMethodsViewHidden: Signal<Bool, Never> { get }
var processingViewIsHidden: Signal<Bool, Never> { get }
var showErrorBannerWithMessage: Signal<String, Never> { get }
var showErrorBannerWithMessage: Signal<(String, /* persist: */ Bool), Never> { get }
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Adding parameter names instead of an inline comment might clarify that this output has the option to persist.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh cool! Updated! TIL that swift allows you to name fields in a tuple (obj-c does not!)

@ifosli ifosli merged commit 0089505 into main Oct 14, 2024
5 checks passed
@ifosli ifosli deleted the removeConfirmDetailsErrorHandling branch October 14, 2024 17:59
stevestreza-ksr pushed a commit that referenced this pull request Oct 16, 2024
… controller (#2171)

* Fix late pledge error handling

* Use named tuple fields for error banner
scottkicks added a commit that referenced this pull request Dec 2, 2024
@scottkicks scottkicks mentioned this pull request Dec 2, 2024
4 tasks
scottkicks added a commit that referenced this pull request Dec 9, 2024
* Revert "[MBL-1613] Handle errors for late pledges without dismissing the view controller (#2171)"

This reverts commit 0089505.

* pr feedback
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants