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

Beta Fix: Fix broken "reset password" link on site creds login screen #3363

Merged
merged 2 commits into from
Dec 19, 2020

Conversation

AmandaRiu
Copy link
Contributor

Fixes #3349 The issue was stripping the protocol from the URL for the label but then using that same URL for routing to the reset password view. Logic has been updated to strip the protocol for display only when needed for display.

To Test

  1. Freshly install the app
  2. Tap Enter your store address
  3. Enter a store address and tap Continue
  4. On the login email screen, verify the store address doesn't contain any protocol information like "http://" or "https://").
  5. Tap Continue with store credentials
  6. Tap Reset your password - verify a webview opens to the reset password page.

cc: @AliSoftware for a beta release

Update release notes:

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

The issue was stripping the protocol from the URL for the label but then using that same URL for routing to the reset password view. Logic has been updated to strip the protocol for display only when needed.
This was somehow removed when the app translations were updated.
@AmandaRiu AmandaRiu added the type: bug A confirmed bug. label Dec 18, 2020
@AmandaRiu AmandaRiu added this to the 5.7 ❄️ milestone Dec 18, 2020
@AmandaRiu AmandaRiu requested a review from a team December 18, 2020 23:19
@peril-woocommerce
Copy link

Messages
📖

This PR contains changes in the subtree libs/login/. It is your responsibility to ensure these changes are merged back into wordpress-mobile/WordPress-Login-Flow-Android. Follow these handy steps!
WARNING: Make sure your git version is 2.19.x or lower - there is currently a bug in later versions that will corrupt the subtree history!

  1. cd woocommerce-android
  2. git checkout betafix/3349-password-reset-fix
  3. git subtree push --prefix=libs/login/ https://github.com/wordpress-mobile/WordPress-Login-Flow-Android.git merge/woocommerce-android/3363
  4. Browse to https://github.com/wordpress-mobile/WordPress-Login-Flow-Android/pull/new/merge/woocommerce-android/3363 and open a new PR.

Generated by 🚫 dangerJS

@peril-woocommerce
Copy link

You can test the changes on this Pull Request by downloading the APK here.

@nbradbury nbradbury self-assigned this Dec 19, 2020
@nbradbury nbradbury self-requested a review December 19, 2020 13:48
@@ -201,8 +201,9 @@ protected void setupLabel(@NonNull TextView label) {
break;
case WOO_LOGIN_MODE:
if (mOptionalSiteCredsLayout) {
String siteAddressClean = mLoginSiteUrl.replaceFirst("^(http[s]?://)", "");
Copy link
Contributor

Choose a reason for hiding this comment

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

NP: Changes look good and I'll merge this, but wanted to mention that it would be good to move this logic to StringUtils since there's more than one place we remove the protocol for display.

Copy link
Contributor

@nbradbury nbradbury left a comment

Choose a reason for hiding this comment

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

:shipit:

@nbradbury nbradbury merged commit abc7832 into release/5.7 Dec 19, 2020
@nbradbury nbradbury deleted the betafix/3349-password-reset-fix branch December 19, 2020 13:57
AliSoftware added a commit that referenced this pull request Jan 11, 2021
Not sure if this change is correct. Matches what we see in GlotPress indeed, but don't think that's what we wanted?
See #3363
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A confirmed bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants