Skip to content

Conversation

@hichamboushaba
Copy link
Member

@hichamboushaba hichamboushaba commented Jan 9, 2023

Part of: #8110

Description

Testing instructions

Login

  1. Install a plugin that disables application passwords (I use this one)
  2. Open the app
  3. Sign in using site credentials.
  4. Confirm that the error screen is shown.
  5. Go back to the site, and deactivate the plugin.
  6. Click on Retry.
  7. Confirm the login continues.

Check when using the app

  1. After sign in, go to your site, then re-install/re-activate the plugin.
  2. Start using the app (open orders or products then force fetching, the same would happen when relaunching the app).
  3. Notice the app will log you out.

Note: regarding the last test, we can probably do better, but for now this will do, the user will see the correct error message if they try to sign in to the same site. We'll add some tracking to track the number of cases where this happens, and decide if it's worth improvement or not.

Images/gif

The recording is choppy, I don't know why 😕

device-2023-01-09-174907.mp4
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@hichamboushaba hichamboushaba added type: enhancement A request for an enhancement. feature: REST API Support connecting to sites using WordPress REST API. labels Jan 9, 2023
@hichamboushaba hichamboushaba changed the base branch from trunk to issue/8110-fetch-woo-status January 9, 2023 16:42
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Jan 9, 2023

You can test the changes on this Pull Request by downloading an installable build, or scanning this QR code:

@codecov-commenter
Copy link

codecov-commenter commented Jan 9, 2023

Codecov Report

Base: 42.96% // Head: 42.94% // Decreases project coverage by -0.01% ⚠️

Coverage data is based on head (35db205) compared to base (0f67d96).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff              @@
##              trunk    #8150      +/-   ##
============================================
- Coverage     42.96%   42.94%   -0.02%     
  Complexity     3484     3484              
============================================
  Files           685      685              
  Lines         37305    37316      +11     
  Branches       4950     4951       +1     
============================================
  Hits          16027    16027              
- Misses        19835    19846      +11     
  Partials       1443     1443              
Impacted Files Coverage Δ
...n/kotlin/com/woocommerce/android/AppInitializer.kt 0.00% <0.00%> (ø)
...n/sitecredentials/LoginSiteCredentialsViewModel.kt 0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@hichamboushaba hichamboushaba marked this pull request as ready for review January 9, 2023 17:10
@hichamboushaba hichamboushaba added this to the 11.9 milestone Jan 9, 2023
@JorgeMucientes JorgeMucientes self-assigned this Jan 10, 2023
Base automatically changed from issue/8110-fetch-woo-status to trunk January 10, 2023 12:10
Comment on lines 253 to 267
private fun isGooglePlayServicesAvailable(context: Context): Boolean {
val googleApiAvailability = GoogleApiAvailability.getInstance()

return when (val connectionResult = googleApiAvailability.isGooglePlayServicesAvailable(context)) {
ConnectionResult.SUCCESS -> true
else -> {
WooLog.w(
T.NOTIFS,
"Google Play Services unavailable, connection result: " +
googleApiAvailability.getErrorString(connectionResult)
)
return false
}
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This is unrelated to this PR, but I noticed this function is not used anymore (confirmation #6535 (comment)), so I removed it.

@JorgeMucientes
Copy link
Contributor

Nice job there! Everything works as expected @hichamboushaba :shipit:

@JorgeMucientes JorgeMucientes merged commit ae19f4b into trunk Jan 10, 2023
@JorgeMucientes JorgeMucientes deleted the issue/8110-check-application-passwords-status branch January 10, 2023 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: REST API Support connecting to sites using WordPress REST API. type: enhancement A request for an enhancement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants