This repository was archived by the owner on Feb 5, 2025. It is now read-only.
Restore Allowing UI Overrides for Login Errors #595
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts some of the changes in 3b46ba8. That commit removed the ability for the host app to override errors. I'm applying it a bit differently here though. There was a condition added in 5cadb62 which automatically added an error dialog if the `WordPressAuthenticator.shared.configuration.enableSignUp` is `false`. The `sendEmail` is no longer called so I'm adding the override directly in `handleLoginError()`.
This was referenced Jun 3, 2021
shiki
requested review from
ScoutHarris,
emilylaguna and
momo-ozawa
and removed request for
emilylaguna
ScoutHarris
approved these changes
Jun 3, 2021
Contributor
There was a problem hiding this comment.
Hey @shiki . Thanks for creating a test branch for WP/JP!
- In WordPress, log in with an email that is not registered in WPCOM. WordPress should respond with “We've emailed you a signup link...” - ✅
- In Jetpack, log in with an email that is not registered in WPCOM. Jetpack should respond with “User does not exist”. - ✅
!
Contributor
Author
|
Thank you for the quick review, @ScoutHarris! 🙇 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes woocommerce/woocommerce-ios#3962.
Findings
The WooCommerce app does not allow automatic sign-up emails. If the entered email address is not recognized, it displays this custom error message:
However, this regressed when #574 was merged. And eventually got “fixed” again when #590 was merged. Funny how life works. It wasn't a complete fix though because it doesn't show the custom UI. It only shows an alert:
Solution
I restored a part of the changes in #574 so that the
WordPressAuthenticator.shared.delegateis now inquired if it would like to display a custom UI before callingdisplayError().WordPressAuthenticator-iOS/WordPressAuthenticator/Unified Auth/View Related/Get Started/GetStartedViewController.swift
Lines 411 to 426 in 968c1f2
This is pretty much the same code as before, only in a different location. This is enough for the WooCommerce app to handle and display the UI. See woocommerce/woocommerce-ios#4365 for the WIP branch.
Testing
WordPress and Jetpack
Please test for regressions in WordPress and Jetpack.
Use the draft branch in Fix Error Handling in Site Address Entry WordPress-iOS#16627.
In WordPress, log in with an email that is not registered in WPCOM. WordPress should respond with “We've emailed you a signup link...”:
In Jetpack, log in with an email that is not registered in WPCOM. Jetpack should respond with “User does not exist”.
WooCommerce (Optional)
If you have time, please test that WooCommerce will now display a custom UI.