Feature/6618 limit stats refresh #7411

JorgeMucientes · 2022-09-17T16:39:55Z

⚠️ Do not merge until FluxC changes are merged

Part of: #6618

Description

wc-analytics endpoint is one of the slowest endpoints we hit from the app, and is one that we hit a lot. We fetch site stats and top performers every time we open My Store tab and when we change between tabs. This translates into the "slow/endless loading" perception of the app. This PR alongside changes applied in this other PR in woocommerce-android aims to reduce that loading "slowness" for top performer products data and provide better offline support with the following changes:

Migrate top performers cache to Room to easily expose an observable
Simplify top performers cached model to avoid having to parse multiple JSON or query different sql tables to load top performers from cache.
Invalidate top performers cache when a new order is placed
Invalidate top performers cache after 1h. Maybe this is to often? please share your thoughts

Finally, I updated all broken unit tests, remove some that did not apply anymore, and added a couple more.

Testing instructions

Log into the app and check the following:

Top performers are loaded correctly for all stats granularities
Top performers are invalidated when a new order notification is received. Check for logcat messageInvalidating top performer products after receiving the notification
Check that after data is invalidated when changing stat granularity, the data is fetched from API and refeshed accordingly with new values.. See the log fetchLeaderboards is displayed in logcat as well as notice how the top performer list is refershed
Check that once all 4 tabs data is loaded at least once, switching between tabs doesn't trigger fetching data from API. Basically, check that fetchLeaderboards is never called
Check that after swipe to refresh is called, then switching between tabs will refresh data fro API
Check that when we try to refresh top performer data without connectivity, when connectivity is back (enable/disable airplane mode for example) the top performer list is refreshed with data from API.

…ditions between states

…/woocommerce/woocommerce-android into feature/6618-limit-stats-refresh # Conflicts: # WooCommerce/src/main/kotlin/com/woocommerce/android/ui/mystore/MyStoreViewModel.kt # WooCommerce/src/main/kotlin/com/woocommerce/android/ui/mystore/data/StatsRepository.kt # WooCommerce/src/main/kotlin/com/woocommerce/android/ui/mystore/domain/GetTopPerformers.kt

wpmobilebot · 2022-09-17T19:41:42Z

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

# Conflicts: # WooCommerce/src/main/kotlin/com/woocommerce/android/ui/mystore/data/StatsRepository.kt # build.gradle

codecov-commenter · 2022-09-25T19:20:57Z

Codecov Report

Base: 44.10% // Head: 44.10% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (f7f592a) compared to base (9ee1e74).
Patch coverage: 66.66% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff            @@
##              trunk    #7411   +/-   ##
=========================================
  Coverage     44.10%   44.10%           
- Complexity     3286     3290    +4     
=========================================
  Files           582      582           
  Lines         33126    33146   +20     
  Branches       4306     4310    +4     
=========================================
+ Hits          14609    14620   +11     
- Misses        17184    17188    +4     
- Partials       1333     1338    +5

Impacted Files	Coverage Δ
...ommerce/android/ui/mystore/data/StatsRepository.kt	`3.77% <0.00%> (-0.03%)`	⬇️
...woocommerce/android/ui/mystore/MyStoreViewModel.kt	`76.52% <80.00%> (-2.78%)`	⬇️
...erce/android/ui/mystore/domain/GetTopPerformers.kt	`93.10% <91.66%> (-0.23%)`	⬇️
...ommerce/android/push/NotificationMessageHandler.kt	`66.66% <100.00%> (+0.38%)`	⬆️

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.

…nnectivity

hichamboushaba

Thanks for working on this @JorgeMucientes, the code looks good, but there is a minor issue, please check the comments, and let me know what you think.

hichamboushaba · 2022-10-05T14:34:22Z

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/mystore/MyStoreViewModel.kt

-            refreshStoreStats[index] = true
+    private fun observeTopPerformerUpdates() {
+        viewModelScope.launch {
+            _activeStatsGranularity.collectLatest { granularity ->


np, flatMapLatest would fit the need here better than collectLatest, as it allows transforming items of the Flow receive (granulity) into another Flow (DB Results).

Makes sense! I have updated it here

hichamboushaba · 2022-10-05T14:42:48Z

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/mystore/MyStoreViewModel.kt

+                            isLoading = false,
+                            isError = false,


Setting isLoading is problematic, it'll cause this behavior:

screen-20221005-153227.mp4

The first loading of top performers of the Year tab didn't have any loading indicator.

I think the change should be part of loadTopPerformersStats, as it's the one that decides whether we need the loading state or not, WDYT?
I think the same applies for isError even though it doesn't seem to cause any visible issues in the current status, but it'll be more consistent, and respects the responsibilities better.

Well spotted! I've removed isLoading or isError state updates done from observeTopPerformerUpdates function. This fixes the issue and also makes de loading behaviour more consistent across forced updates.
Thanks!

Thanks @JorgeMucientes for making the change.
For the isError, I belive we still miss one case, we don't reset it to false after the loading, so in case of an error, it would stuck in the true state forever, do you think we should update it in the onSuccess here?

Oh yes, completely right @hichamboushaba

I decided to update it when we set the isLoading to true. Basically, because that way we ensure those 2 states are mutually exclusive and if someone decides to change the order of this code inside MyStoreFragment putting isError before isLoading, we'd still see the loading state:

viewModel.topPerformersState.observe(viewLifecycleOwner) { topPerformers -> when { topPerformers.isError -> showTopPerformersError() topPerformers.isLoading -> showTopPerformersLoading() else -> showTopPerformers(topPerformers.topPerformers) } }

peril-woocommerce · 2022-10-06T12:26:46Z

	Warnings
⚠️	PR is missing at least one label.

Generated by 🚫 dangerJS

# Conflicts: # build.gradle

wpmobilebot · 2022-10-06T15:03:38Z

Found 1 violations:

The PR caused the following dependency changes:

expand

-+--- org.wordpress:fluxc:trunk-87eadb05bd327745850ff8accf8ca4dbaa03cece
-|    +--- org.wordpress:wellsql:1.7.0
-|    |    \--- org.wordpress.wellsql:wellsql-annotations:1.7.0
-|    +--- org.wordpress.fluxc:fluxc-annotations:trunk-87eadb05bd327745850ff8accf8ca4dbaa03cece
-|    +--- org.greenrobot:eventbus:3.3.1 (*)
-|    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.9.3 (*)
-|    +--- com.android.volley:volley:1.1.1 -> 1.2.0
-|    +--- androidx.paging:paging-runtime:2.1.2
-|    |    +--- androidx.paging:paging-common:2.1.2
-|    |    |    +--- androidx.annotation:annotation:1.0.0 -> 1.4.0 (*)
-|    |    |    \--- androidx.arch.core:core-common:2.0.0 -> 2.1.0 (*)
-|    |    +--- androidx.arch.core:core-runtime:2.0.0 -> 2.1.0 (*)
-|    |    +--- androidx.lifecycle:lifecycle-runtime:2.0.0 -> 2.5.1 (*)
-|    |    +--- androidx.lifecycle:lifecycle-livedata:2.0.0 -> 2.5.1 (*)
-|    |    \--- androidx.recyclerview:recyclerview:1.0.0 -> 1.2.1 (*)
-|    +--- com.goterl:lazysodium-android:5.0.2
-|    +--- net.java.dev.jna:jna:5.5.0
-|    +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.10 -> 1.6.21 (*)
-|    +--- org.jetbrains.kotlin:kotlin-android-extensions-runtime:1.6.10
-|    |    \--- org.jetbrains.kotlin:kotlin-stdlib:1.6.10 -> 1.6.21 (*)
-|    +--- androidx.appcompat:appcompat:1.0.2 -> 1.4.2 (*)
-|    +--- androidx.recyclerview:recyclerview:1.0.0 -> 1.2.1 (*)
-|    +--- androidx.exifinterface:exifinterface:1.0.0 -> 1.3.3
-|    |    \--- androidx.annotation:annotation:1.2.0 -> 1.4.0 (*)
-|    +--- com.squareup.okhttp3:okhttp-urlconnection:4.9.0
-|    |    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.9.3 (*)
-|    |    \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.4.10 -> 1.6.21 (*)
-|    +--- com.google.code.gson:gson:2.8.5 -> 2.9.0
-|    +--- org.apache.commons:commons-text:1.1 -> 1.8 (*)
-|    +--- androidx.room:room-runtime:2.4.2 (*)
-|    +--- androidx.room:room-ktx:2.4.2
-|    |    +--- androidx.room:room-common:2.4.2 (*)
-|    |    +--- androidx.room:room-runtime:2.4.2 (*)
-|    |    +--- org.jetbrains.kotlin:kotlin-stdlib:1.6.10 -> 1.6.21 (*)
-|    |    \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.5.2 -> 1.6.4 (*)
-|    +--- com.google.dagger:dagger:2.42
-|    |    \--- javax.inject:javax.inject:1
-|    +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.3.9 -> 1.6.4 (*)
-|    \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.3.9 -> 1.6.4 (*)
++--- org.wordpress:fluxc:trunk-6459816749ce343afcc072dba807f24db1164966
+|    +--- org.wordpress:wellsql:1.7.0
+|    |    \--- org.wordpress.wellsql:wellsql-annotations:1.7.0
+|    +--- org.wordpress.fluxc:fluxc-annotations:trunk-6459816749ce343afcc072dba807f24db1164966
+|    +--- org.greenrobot:eventbus:3.3.1 (*)
+|    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.9.3 (*)
+|    +--- com.android.volley:volley:1.1.1 -> 1.2.0
+|    +--- androidx.paging:paging-runtime:2.1.2
+|    |    +--- androidx.paging:paging-common:2.1.2
+|    |    |    +--- androidx.annotation:annotation:1.0.0 -> 1.4.0 (*)
+|    |    |    \--- androidx.arch.core:core-common:2.0.0 -> 2.1.0 (*)
+|    |    +--- androidx.arch.core:core-runtime:2.0.0 -> 2.1.0 (*)
+|    |    +--- androidx.lifecycle:lifecycle-runtime:2.0.0 -> 2.5.1 (*)
+|    |    +--- androidx.lifecycle:lifecycle-livedata:2.0.0 -> 2.5.1 (*)
+|    |    \--- androidx.recyclerview:recyclerview:1.0.0 -> 1.2.1 (*)
+|    +--- com.goterl:lazysodium-android:5.0.2
+|    +--- net.java.dev.jna:jna:5.5.0
+|    +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.10 -> 1.6.21 (*)
+|    +--- org.jetbrains.kotlin:kotlin-android-extensions-runtime:1.6.10
+|    |    \--- org.jetbrains.kotlin:kotlin-stdlib:1.6.10 -> 1.6.21 (*)
+|    +--- androidx.appcompat:appcompat:1.0.2 -> 1.4.2 (*)
+|    +--- androidx.recyclerview:recyclerview:1.0.0 -> 1.2.1 (*)
+|    +--- androidx.exifinterface:exifinterface:1.0.0 -> 1.3.3
+|    |    \--- androidx.annotation:annotation:1.2.0 -> 1.4.0 (*)
+|    +--- com.squareup.okhttp3:okhttp-urlconnection:4.9.0
+|    |    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.9.3 (*)
+|    |    \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.4.10 -> 1.6.21 (*)
+|    +--- com.google.code.gson:gson:2.8.5 -> 2.9.0
+|    +--- org.apache.commons:commons-text:1.1 -> 1.8 (*)
+|    +--- androidx.room:room-runtime:2.4.2 (*)
+|    +--- androidx.room:room-ktx:2.4.2
+|    |    +--- androidx.room:room-common:2.4.2 (*)
+|    |    +--- androidx.room:room-runtime:2.4.2 (*)
+|    |    +--- org.jetbrains.kotlin:kotlin-stdlib:1.6.10 -> 1.6.21 (*)
+|    |    \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.5.2 -> 1.6.4 (*)
+|    +--- com.google.dagger:dagger:2.42
+|    |    \--- javax.inject:javax.inject:1
+|    +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.3.9 -> 1.6.4 (*)
+|    \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.3.9 -> 1.6.4 (*)
-\--- org.wordpress.fluxc.plugins:woocommerce:trunk-87eadb05bd327745850ff8accf8ca4dbaa03cece
-     +--- org.wordpress:wellsql:1.7.0 (*)
-     +--- org.wordpress.fluxc:fluxc-annotations:trunk-87eadb05bd327745850ff8accf8ca4dbaa03cece
-     +--- androidx.room:room-ktx:2.4.2 (*)
-     +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.10 -> 1.6.21 (*)
-     +--- org.wordpress:fluxc:trunk-87eadb05bd327745850ff8accf8ca4dbaa03cece (*)
-     +--- org.apache.commons:commons-lang3:3.7 -> 3.9
-     +--- com.google.code.gson:gson:2.8.5 -> 2.9.0
-     +--- com.google.dagger:dagger:2.42 (*)
-     +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.3.9 -> 1.6.4 (*)
-     +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.3.9 -> 1.6.4 (*)
-     \--- androidx.room:room-runtime:2.4.2 (*)
+\--- org.wordpress.fluxc.plugins:woocommerce:trunk-6459816749ce343afcc072dba807f24db1164966
+     +--- org.wordpress:wellsql:1.7.0 (*)
+     +--- org.wordpress.fluxc:fluxc-annotations:trunk-6459816749ce343afcc072dba807f24db1164966
+     +--- androidx.room:room-ktx:2.4.2 (*)
+     +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.10 -> 1.6.21 (*)
+     +--- org.wordpress:fluxc:trunk-6459816749ce343afcc072dba807f24db1164966 (*)
+     +--- org.apache.commons:commons-lang3:3.7 -> 3.9
+     +--- com.google.code.gson:gson:2.8.5 -> 2.9.0
+     +--- com.google.dagger:dagger:2.42 (*)
+     +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.3.9 -> 1.6.4 (*)
+     +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.3.9 -> 1.6.4 (*)
+     \--- androidx.room:room-runtime:2.4.2 (*)

Please review and act accordingly

hichamboushaba

Nice work @JorgeMucientes.

~~This can be merged after fixing the merge conflicts.~~

JorgeMucientes added 22 commits September 1, 2022 12:55

Make top performers force refresh optional

128fa15

Remove force refresh responsibility out viewModel for top performers

62bc217

Merge branch 'trunk' into feature/6618-limit-stats-refresh

14a5c3e

Refresh top performers when a new order notification comes in

fb5dc65

Soft reset this commit

a4f398e

Revert updating stats on notification received from viewmodel

3e2b5ab

Refactor top performers to observable approach to keep data fresh

c69de83

Change how view state is modeled for top performers to avoid race con…

cfc9386

…ditions between states

Remove old methods to fetch top performer products

2ec854a

Make top performers force refresh optional

d1490f8

Remove force refresh responsibility out viewModel for top performers

92f9a0e

Refresh top performers when a new order notification comes in

d56a766

Soft reset this commit

260f642

Revert updating stats on notification received from viewmodel

5f65ef4

Refactor top performers to observable approach to keep data fresh

d069e7e

Change how view state is modeled for top performers to avoid race con…

d7dba9a

…ditions between states

Remove old methods to fetch top performer products

19621a4

Move top performers count constant to use case

0084509

Observe DB updates from IO dispatcher

09481b2

Invalidate top performer products cache on new order placed

4f8de79

Fix failing unit tests

a8133e1

JorgeMucientes added the status: do not merge Dependent on another PR, ready for review but not ready for merge. label Sep 17, 2022

JorgeMucientes added 2 commits September 17, 2022 21:18

Update fluxC version

6cf2d6e

Fix detekt

c74ea17

This was referenced Sep 17, 2022

Feature/6619 limit stats refresh wordpress-mobile/WordPress-FluxC-Android#2526

Merged

Only refresh My Store stats data when a new order is placed for the site. #6618

Open

JorgeMucientes added 2 commits September 25, 2022 20:36

Merge branch 'trunk' into feature/6618-limit-stats-refresh

87c4de8

# Conflicts: # WooCommerce/src/main/kotlin/com/woocommerce/android/ui/mystore/data/StatsRepository.kt # build.gradle

Fix import ordering detekt issue after update with trunk

0aec233

Mark as refresh pending when trying to load top performers without co…

84ea243

…nnectivity

JorgeMucientes marked this pull request as ready for review October 3, 2022 10:50

JorgeMucientes requested review from hichamboushaba and 0nko October 3, 2022 10:50

hichamboushaba self-assigned this Oct 5, 2022

hichamboushaba reviewed Oct 5, 2022

View reviewed changes

JorgeMucientes added 2 commits October 6, 2022 13:53

Use flatmap latest for better readability

19b8e4f

Handle loading and error states only from loadTopPerformersStats

4ba28e7

JorgeMucientes added 2 commits October 6, 2022 15:26

Update FluxC version

46437fa

Ensure isError state is set to false whenever data is fetch again

b2b39cd

JorgeMucientes added this to the 10.7 milestone Oct 6, 2022

JorgeMucientes added 2 commits October 6, 2022 16:33

Update FluxC version with latest trunk changeset

c4c86f6

Merge branch 'trunk' into feature/6618-limit-stats-refresh

f7f592a

# Conflicts: # build.gradle

hichamboushaba approved these changes Oct 6, 2022

View reviewed changes

hichamboushaba merged commit 8cf276c into trunk Oct 6, 2022

hichamboushaba deleted the feature/6618-limit-stats-refresh branch October 6, 2022 15:25

hichamboushaba removed the status: do not merge Dependent on another PR, ready for review but not ready for merge. label Oct 6, 2022

atorresveiga mentioned this pull request Oct 25, 2022

Top performers with date period #7626

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/6618 limit stats refresh #7411

Feature/6618 limit stats refresh #7411

JorgeMucientes commented Sep 17, 2022 •

edited

Loading

wpmobilebot commented Sep 17, 2022 •

edited

Loading

codecov-commenter commented Sep 25, 2022 •

edited

Loading

hichamboushaba left a comment

hichamboushaba Oct 5, 2022

JorgeMucientes Oct 6, 2022

hichamboushaba Oct 5, 2022

JorgeMucientes Oct 6, 2022 •

edited

Loading

hichamboushaba Oct 6, 2022

JorgeMucientes Oct 6, 2022

peril-woocommerce bot commented Oct 6, 2022 •

edited

Loading

wpmobilebot commented Oct 6, 2022

hichamboushaba left a comment •

edited

Loading

Feature/6618 limit stats refresh #7411

Feature/6618 limit stats refresh #7411

Conversation

JorgeMucientes commented Sep 17, 2022 • edited Loading

Description

Testing instructions

wpmobilebot commented Sep 17, 2022 • edited Loading

codecov-commenter commented Sep 25, 2022 • edited Loading

Codecov Report

hichamboushaba left a comment

Choose a reason for hiding this comment

hichamboushaba Oct 5, 2022

Choose a reason for hiding this comment

JorgeMucientes Oct 6, 2022

Choose a reason for hiding this comment

hichamboushaba Oct 5, 2022

Choose a reason for hiding this comment

JorgeMucientes Oct 6, 2022 • edited Loading

Choose a reason for hiding this comment

hichamboushaba Oct 6, 2022

Choose a reason for hiding this comment

JorgeMucientes Oct 6, 2022

Choose a reason for hiding this comment

peril-woocommerce bot commented Oct 6, 2022 • edited Loading

wpmobilebot commented Oct 6, 2022

The PR caused the following dependency changes:

hichamboushaba left a comment • edited Loading

Choose a reason for hiding this comment

JorgeMucientes commented Sep 17, 2022 •

edited

Loading

wpmobilebot commented Sep 17, 2022 •

edited

Loading

codecov-commenter commented Sep 25, 2022 •

edited

Loading

JorgeMucientes Oct 6, 2022 •

edited

Loading

peril-woocommerce bot commented Oct 6, 2022 •

edited

Loading

hichamboushaba left a comment •

edited

Loading