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

MSD - Stats Card - Handle Error Inside Cards Response #2251

Merged
merged 12 commits into from
Feb 18, 2022

Conversation

ashiagr
Copy link
Contributor

@ashiagr ashiagr commented Jan 27, 2022

Parent wordpress-mobile/WordPress-Android#15749

This draft PR:

  • Updates the today's stats and posts cards responses to include error.
  • Updates the today's stats and posts card model to include error.
  • Maps error in responses to an error in models.
  • Makes no changes to storing and retrieving logic for dashboard cards into the database. This means the error inside the card model will get saved into the database whenever it is present and returned to the client as part of the card model. We can discuss any changes to this behavior.

To test:

  • There is actually nothing to test in this PR, just review the updated CardsRestClient and CardsStore classes and make sure that the accompanying tests are working as expected.

Notes:

  • As the differential is still to be deployed, the fetchCards endpoint is not replaced with v1_1 version yet to avoid rest route not found errors.
  • To manual test: Apply D73653-code on your sandbox, proxy your device on it. Replace the fetchCards endpoint with v1_1 version.
  • Let's keep the PR in draft mode till the corresponding WordPress-Android PR is up and we've tested cards with errors feature.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Feb 2, 2022

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

Copy link
Contributor

@zwarm zwarm left a comment

Choose a reason for hiding this comment

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

@ashiagr All looks good. The initial data values have been set to “0” and “empty” as we discussed. The errors are specific to the card type and embedded as such, In addition the errors will flow through to the client. Even if we only show a generic card today, we are ready to handle the specific errors in the future.

Tests work as expected. +1

…o issue/15749-msd-todays-stats-handle-error-response

# Conflicts:
#	fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpcom/dashboard/CardsRestClient.kt
…o issue/15749-msd-todays-stats-handle-error-response

# Conflicts:
#	example/src/test/java/org/wordpress/android/fluxc/network/rest/wpcom/dashboard/CardsRestClientTest.kt
#	fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpcom/dashboard/CardsRestClient.kt
@ashiagr ashiagr marked this pull request as ready for review February 17, 2022 09:46
…o issue/15749-msd-todays-stats-handle-error-response

# Conflicts:
#	example/src/test/java/org/wordpress/android/fluxc/store/dashboard/CardsStoreTest.kt
…o issue/15749-msd-todays-stats-handle-error-response
@zwarm
Copy link
Contributor

zwarm commented Feb 17, 2022

@ashiagr - I retested this PR using WPAndroid#[15893 ](MSD - Handle Single Dashboard Card Error in UI).

I was able to see the stats card properly AND no stats card when using a site in which I don't have permission to view stats. I intercepted the response to verify this.

I'm going to leave the merging for you because I'm not 100% sure which PRs need to go first.
Thanks for all the great work! 🙇

Base automatically changed from issue/15749-msd-stats-card-handle-success-response to trunk February 18, 2022 05:17
@ashiagr ashiagr merged commit 71f2665 into trunk Feb 18, 2022
@ashiagr ashiagr deleted the issue/15749-msd-todays-stats-handle-error-response branch February 18, 2022 05:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants