MSD - Handle Single Dashboard Card Error in UI #15893
Merged
Conversation
This file contains 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
Temporarily points to FluxC draft PR handling error inside dashboard cards.
ashiagr
added
Part of a WIP Feature
|
You can trigger optional UI/connected tests for these changes by visiting CircleCI here. |
ashiagr
changed the title
1 task
ashiagr
changed the title
|
You can test the changes on this Pull Request by downloading the APKs: |
…d class This is done to allow enclosing error card within parent sealed class for today's stats in a future commit.
unauthorized, jetpack_disconnected, jetpack_disabled errors are not shown as they're irrecoverable on pull to refresh.
To test that unauthorized, jetpack_disconnected, jetpack_disabled errors are not shown and that cards with empty state (default values) are also shown when these error exist.
Since this PR only handles errors within a card, the existing mocked response JSON file is modified to include only errors instead of creating a new JSON file specifically for errors. This fixes lint issue complaining about unused mocked resource file. Anyone using the mocked JSON file after this point should appropriately modify the file content.
3 tasks
AjeshRPai
reviewed
Feb 16, 2022
...main/java/org/wordpress/android/ui/mysite/cards/dashboard/error/ErrorWithinCardViewHolder.kt
Outdated
Show resolved
Hide resolved
...n/java/org/wordpress/android/ui/mysite/cards/dashboard/todaysstats/TodaysStatsCardBuilder.kt
Outdated
Show resolved
Hide resolved
...n/java/org/wordpress/android/ui/mysite/cards/dashboard/todaysstats/TodaysStatsCardBuilder.kt
Outdated
Show resolved
Hide resolved
…le-card-error-state # Conflicts: # WordPress/src/main/res/raw/mocked_site_dashboard_cards.json # build.gradle
…le-card-error-state # Conflicts: # build.gradle
…le-card-error-state
|
As discussed on Slack, merging it now. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Parent #15750
References pbArwn-3ny-p2 (
if one card fails - error inside cardstate and title copy change in this comment)References wordpress-mobile/WordPress-FluxC-Android#2251
This PR handles errors within
PostsandToday's Statscards using mocked data.Note: As
unauthorised,jetpack_disconnectedandjetpack_disablederrors are irrecoverable (see p1643789176707579-slack-C0290FLA0RM), these error are filtered out from displaying. The condition for showing these error cards can be modified in the future in the corresponding builder classes. The tests ensure that no card (neither with error nor with data) is displayed when these errors are returned for that card type.To test:
Prerequisites
wpcomsite.My Site->App Settings->Debug Settings.MySiteDashboardPhase2FeatureConfigis to set to on.Notice that no card exists for
Today's StatsandPostsin theMy Site Dashboardwith the given mocked JSON.[Optional, covered in unit tests] Replace mocked JSON
errornode with different supported error types and reinstall the app:Notice that these errors are never shown.
Replace mocked JSON
errornode with a fallbackgeneric_errorand reinstall the app. Notice that a generic error within a card is displayed as shown. We can further discuss showing fallback errors in this Slack thread (p1644303679654909/1643829887.600439-slack-C0290FLA0RM). An error within a card, when shown, is tracked as follows (see pbArwn-2zs-p2#comment-4731). Tracking can be tested by enablingMy Site->App Settings->Privacy settings->Collect informationand switching between sites.Merge Instructions
This PR is still in draft mode as it depends on the previous unmerged
Today's CardUI with mocked data and the linked draftFluxCPR requiring changes after the endpoint is live. Please wait to merge the PR until mentioned PRs are merged todevelop.Regression Notes
Potential unintended areas of impact
Cards should not be displayed with default values (like 0 likes, 0 views) when an error exists.
What I did to test those areas of impact (or what existing automated tests I relied on)
Manually tested.
What automated tests I added (or what prevented me from doing so)
Add unit tests.
PR submission checklist:
RELEASE-NOTES.txtif necessary.