woocommerce-android icon indicating copy to clipboard operation
woocommerce-android copied to clipboard

[Dynamic Dashboard] Handle hiding Blaze card when not available

Open hichamboushaba opened this issue 1 year ago • 6 comments

Part of: #11359

Description

This PR adds logic to hide the Blaze card when not available, it depends on the changes in https://github.com/wordpress-mobile/WordPress-FluxC-Android/pull/2995

I was planning on handling the Onboarding on the same PR, but given the need for the FluxC changes, I decided to split them.

Testing instructions

Private site

  1. Make a WooExpress site private.
  2. Open the app.
  3. Confirm the Blaze card is hidden from both the dashboard and its settings.

Site with published products

  1. Use a site that doesn't have any published products.
  2. Open the app.
  3. Confirm the Blaze card is hidden.
  4. Add a new product.
  5. Go back to the dashboard.
  6. Confirm the Blaze card was added.

Images/gif

  • [x] I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

hichamboushaba avatar Apr 25 '24 15:04 hichamboushaba

2 Warnings
:warning: Class ObserveBlazeWidgetStatus is missing tests, but unit-tests-exemption label was set to ignore this.
:warning: Class ObserveStatsWidgetsStatus is missing tests, but unit-tests-exemption label was set to ignore this.

Generated by :no_entry_sign: Danger

dangermattic avatar Apr 25 '24 15:04 dangermattic

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

App Name WooCommerce Android
FlavorJalapeno
Build TypeDebug
Commite64bf3b0a9b1ea04879a31fa58f62c3af27a5baf
Direct Downloadwoocommerce-prototype-build-pr11380-e64bf3b.apk

wpmobilebot avatar Apr 25 '24 16:04 wpmobilebot

Codecov Report

Attention: Patch coverage is 0% with 86 lines in your changes are missing coverage. Please review.

Project coverage is 40.91%. Comparing base (ad609b1) to head (9c50708).

:exclamation: Current head 9c50708 differs from pull request most recent head e64bf3b. Consider uploading reports for the commit e64bf3b to get more accurate results

Files Patch % Lines
...e/android/ui/dashboard/data/DashboardRepository.kt 0.00% 24 Missing :warning:
...roid/ui/dashboard/data/ObserveBlazeWidgetStatus.kt 0.00% 19 Missing :warning:
.../android/ui/products/list/ProductListRepository.kt 0.00% 19 Missing :warning:
...oid/ui/dashboard/data/ObserveStatsWidgetsStatus.kt 0.00% 12 Missing :warning:
...roid/ui/dashboard/blaze/DashboardBlazeViewModel.kt 0.00% 7 Missing :warning:
...n/com/woocommerce/android/model/DashboardWidget.kt 0.00% 4 Missing :warning:
...ard/widgeteditor/DashboardWidgetEditorViewModel.kt 0.00% 1 Missing :warning:
Additional details and impacted files
@@             Coverage Diff              @@
##              trunk   #11380      +/-   ##
============================================
- Coverage     40.95%   40.91%   -0.04%     
+ Complexity     5215     5214       -1     
============================================
  Files          1066     1068       +2     
  Lines         62607    62651      +44     
  Branches       8554     8549       -5     
============================================
- Hits          25640    25635       -5     
- Misses        34664    34713      +49     
  Partials       2303     2303              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Apr 25 '24 16:04 codecov-commenter

Just one observation: I already had some products cached in the app for a store. Then I deleted all products on the web switched to that store in the app. I could still see the Blaze card because the products didn't get refreshed until I tapped on the Products tab. Do you think we should refresh products in the My Store tab, as well?

Thanks @0nko for the review, regarding this point, in a previous PR, I updated the logic of refreshing products in the Dashboard to be only when we don't have any cached products, and during a pull-to-refresh, this was done to reduce the number of requests the app does on launch, and I discussed it with Jorge here, for a store to go from having products to none is an edge case IMO. WDYT about this? I don't have a strong opinion about it, and we might still need to change when working on iteration 2 as we'll have some other cards.

hichamboushaba avatar Apr 26 '24 17:04 hichamboushaba

@hichamboushaba Thanks for your response!

WDYT about this? I don't have a strong opinion about it, and we might still need to change when working on iteration 2 as we'll have some other cards.

Yeah, no problem, it's an edge case, let's keep it as it is. I was just wondering about it when I noticed it.

0nko avatar Apr 26 '24 19:04 0nko

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ SelectedSite$SelectedSiteResetException com.woocommerce.android.tools.SelectedSite in get View Issue

Did you find this useful? React with a 👍 or 👎

sentry[bot] avatar May 01 '24 20:05 sentry[bot]