[WellSQL Migration] Migrate `WCRevenueStatsModel` to Room #14680

ParaskP7 · 2025-10-02T13:58:00Z

Description

This PR migrates WCRevenueStatsModel from WellSQL to Room database.

Testing information

`My store` + `Performance`

Navigate on the My store tab and verify that the Performance section is being correctly populated with the expected Revenue number.
On that Performance section, click on the Calendar icon and select Today. Verify that the section's title changes to something like Today Oct 2, 2025 and that the Revenue number is updated accordingly.
Similarly, select This Week, This Month and This Year and verify everything is working as expected.
Open App Inspection, check the RevenueStatsEntity table, and notice 4 rows being populated, each for every non-custom selection of yours (so far):
- Today: Today2025-10-022025-10-02 (rangeId)
- This Week: Week to Date2025-09-282025-10-04 (rangeId)
- This Month: Month to Date2025-10-012025-10-31 (rangeId)
- This Year: Year to Date2025-01-012025-12-31 (rangeId)
Now, select Custom and edit the section's title picking a specific "single" day like Oct 2 - Oct 2, click SAVE. Verify that the section's title changes to something like Custom Oct 2, 2025 - Oct 2, 2025 and that the Revenue number is updated accordingly.
Then, edit the section's title again, picking any other range, except for a "single" day, like Oct 1 - Oct 2, click SAVE.
Verify that the section's title changes to something like Custom Oct 1, 2025 - Oct 2, 2025 and verify the Revenue number is updated accordingly.
Try the below custom dates:
- Oct 2 - Oct 2 (Full Day): Custom2025-10-022025-10-02 (rangeId)
- Oct 1 - Oct 2 (Partial Week): Custom2025-10-012025-10-02 (rangeId)
- Sep 21 - Sep 27 (Full Week): Custom2025-09-212025-09-27 (rangeId)
- Sep 1 - Sep 30 (Full Month): Custom2025-09-012025-09-30 (rangeId)
- Jan 1 - Oct 2 (Partial Year): Custom2025-01-012025-10-02 (rangeId)

FYI: I've also compared the before (WCRevenueStatsModel) and after (RevenueStatsEntity) migration data on those tables after making the exact same date selections and everything LGTM, see Images/gif below:

Images/gif

BEFORE `WCRevenueStatsModel`

AFTER `RevenueStatsEntity`

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.

FYI: This is done in preparation to the subsequent WellSQL to Room table migration.

FYI: This change includes but is not limited to the: - Entity Introduction - DAO Introduction (With Test) - WCAndroidDatabase Migration - Code Adjustments (Store) - Test Adjustments & Fixes (Store)

Copilot

Pull Request Overview

This PR migrates the WCRevenueStatsModel from WellSQL to Room database, part of the broader initiative to modernize the app's data persistence layer.

Key changes include:

Converting the WellSQL model to a Room entity with primary key constraints
Implementing a new Room DAO for revenue stats operations
Updating the store to use the new DAO instead of SQL utilities
Converting synchronous database operations to suspend functions

Reviewed Changes

Copilot reviewed 18 out of 19 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`WCRevenueStatsModel.kt`	Converted from WellSQL table to Room entity with composite primary key
`RevenueStatsDao.kt`	New Room DAO implementing database operations for revenue stats
`WCAndroidDatabase.kt`	Added revenue stats entity and DAO to Room database configuration
`WCStatsStore.kt`	Updated to use Room DAO instead of WellSQL utilities, made functions suspend
`WellSqlConfig.kt`	Added migration to drop the old WellSQL table
`WCStatsSqlUtils.kt`	Removed obsolete WellSQL utility functions
Test files	Updated test constructors and mocks to match new data class structure

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-02T13:58:52Z

libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/model/WCRevenueStatsModel.kt

+@Entity(
+    tableName = "RevenueStatsEntity",
+    primaryKeys = ["localSiteId", "rangeId"],
+)
+data class WCRevenueStatsModel(


The composite primary key using rangeId could be problematic since rangeId can be empty string according to the data class. Empty strings in primary keys may cause issues with uniqueness constraints and queries. Consider using a different approach, such as making rangeId nullable or using a single auto-generated primary key.

Suggested change

@Entity(

tableName = "RevenueStatsEntity",

primaryKeys = ["localSiteId", "rangeId"],

)

data class WCRevenueStatsModel(

import androidx.room.PrimaryKey

import androidx.room.Index

@Entity(

tableName = "RevenueStatsEntity",

indices = [Index(value = ["localSiteId", "rangeId"], unique = true)]

)

data class WCRevenueStatsModel(

@PrimaryKey(autoGenerate = true) val id: Long = 0,

Hmmm, you are right but:

During my testing I couldn't find a way to get rangeId come as empty string from the REST API.

Initially, I indeed tried the localSiteId + interval + startDate + endDate combination as primary key. But, then decided in favor of using the localSiteId + rangeId instead. This is because when testing with the former 4 keyed primary key I was getting 8/9 rows and thus couldn't replicate what I was seeing with the previous WellSQL implementation, see below:

BEFORE WCRevenueStatsModel

AFTER RevenueStatsEntity

Now notice that when using the 4 keyed primary key, the 1st and 5th (HOURS) rows completely match, no matter if that's for a non-custom or custom date and thus data is lost, depending which you selected last. Which, to be honest is not a big deal, but nevertheless I wanted to preserve as much parity between the 2 solutions. 💭

The other option would be to stick to artificial primary key (as in WCRevenueStatsModel), but I think rangeId should be... id - a unique value (in context of a site).

I think your choice is correct. But you might want also check this PR (if you haven't yet) that introduces the rangeId and a check if it's empty or not: https://github.com/wordpress-mobile/WordPress-FluxC-Android/pull/2785/files#diff-3142dcb57a32d3e922e5e96b706565af858c76f424d01ecc16a41af257c37649R135-R153

During my testing I couldn't find a way to get rangeId come as empty string from the REST API.

I guess that the only use case for this is when a 3rd party plugin modifies this contract. It's though to assess the risk, though.

Thanks for taking a look at that @wzieba ! 🙇

The other option would be to stick to artificial primary key (as in WCRevenueStatsModel), but I think rangeId should be... id - a unique value (in context of a site).

I too think rangeId should be a unique id value. 🤷

I think your choice is correct. But you might want also check this PR (if you haven't yet) that introduces the rangeId and a check if it's empty or not: https://github.com/wordpress-mobile/WordPress-FluxC-Android/pull/2785/files#diff-3142dcb57a32d3e922e5e96b706565af858c76f424d01ecc16a41af257c37649R135-R153

I think I did, but I don't remember anymore... 🫣 Will check again and come back to you on that! 💯

I guess that the only use case for this is when a 3rd party plugin modifies this contract. It's though to assess the risk, though.

True, good thinking, this is not the first time we see that! 🤔

👋 @wzieba !

I think I did, but I don't remember anymore... 🫣 Will check again and come back to you on that! 💯

So, I did investigate that today and came to the conclusion that:

This FluxC#2785 PR was the one introducing this rangeId. But, I don't see any evidence as to why it could be empty. 🤷

And specific FluxC commit introduces this rangeId != 0 check. Again, I don't see any evidence as to why this rangeId can be 0 or empty. 🤷

I then went and check all the places this gets used and figured out a way to test it. I found that indeed this gets to be empty when it is used in a widget. ✅ (or wearable, not tested)

However, instead of reverting to using an artificial primary key, and make this all that bit more complicated, unnecessarily if you ask me, I chose to keep the migration as is and introduce a bit of client functionality to populate this rangeId with a non-empty value, following the same Custom pattern, using either the Widget or Wear prefix, followed by a Start + End date, the result: 5c457ba

Let me know what you think. 🙏

FYI: Let me also ping a product engineer here as an extra pair of eyes on this specific change. Cc @malinajirka and thanks for helping us out here. 🙇

PS: Btw, this rangeId solution, even without this change here was working as expected without any crashes, because this revenue widget is quite simple, only targeting today (HOURS, without a means to change that (or I don't know how). 🤔

Copilot · 2025-10-02T13:58:53Z

.../fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/persistence/dao/RevenueStatsDao.kt

+    @Query(
+        """
+        SELECT * FROM RevenueStatsEntity
+        WHERE localSiteId = :siteId
+        AND interval = :granularity
+        AND startDate = :startDate
+        AND endDate = :endDate
+        """
+    )


The query uses interval = :granularity but the parameter is of type StatsGranularity enum while the database column interval is a String. Room may not handle this enum-to-string conversion automatically, potentially causing query failures.

Yea, I was thinking of adding a @TypeConverter for this but wasn't too sure if that's too necessary in this case, @wzieba wdty? 🤔

I think a small @TypeConvert could be a good decision to not leave this conversion vulnerable to changes of Kotlin or Room (e.g. upper/lower case etc.).

Thanks for the reply @wzieba , convinced, will go ahead and add that small @TypeConvert. 👍

FYI: This is now done: 1fe4788 Cc @wzieba

libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/store/WCStatsStore.kt

dangermattic · 2025-10-02T13:59:06Z

	1 Warning
⚠️	This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

wpmobilebot · 2025-10-02T14:09:03Z

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

	App Name	WooCommerce-Wear Android
	Platform	⌚️ Wear OS
	Flavor	Jalapeno
	Build Type	Debug
	Commit	`6798713`
	Direct Download	`woocommerce-wear-prototype-build-pr14680-6798713.apk`

wpmobilebot · 2025-10-02T14:17:37Z

📲 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
	Platform	📱 Mobile
	Flavor	Jalapeno
	Build Type	Debug
	Commit	`6798713`
	Direct Download	`woocommerce-prototype-build-pr14680-6798713.apk`

codecov-commenter · 2025-10-02T14:31:00Z

Codecov Report

❌ Patch coverage is 39.34426% with 37 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.45%. Comparing base (4863540) to head (6798713).
⚠️ Report is 12 commits behind head on trunk.

Files with missing lines	Patch %	Lines
.../org/wordpress/android/fluxc/store/WCStatsStore.kt	7.69%	12 Missing ⚠️
...k/rest/wpcom/wc/orderstats/OrderStatsRestClient.kt	0.00%	11 Missing ⚠️
...ndroid/wear/ui/stats/datasource/StatsRepository.kt	0.00%	5 Missing ⚠️
...rdpress/android/fluxc/persistence/WellSqlConfig.kt	0.00%	4 Missing ⚠️
...oocommerce/android/wear/GetWearableMyStoreStats.kt	0.00%	2 Missing ⚠️
...ess/android/fluxc/persistence/WCAndroidDatabase.kt	0.00%	1 Missing ⚠️
...ersistence/converters/StatsGranularityConverter.kt	87.50%	1 Missing ⚠️
...s/android/fluxc/persistence/dao/RevenueStatsDao.kt	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##              trunk   #14680      +/-   ##
============================================
+ Coverage     38.44%   38.45%   +0.01%     
- Complexity     9886     9891       +5     
============================================
  Files          2103     2104       +1     
  Lines        117051   117026      -25     
  Branches      15651    15649       -2     
============================================
+ Hits          44996    45007      +11     
+ Misses        67882    67848      -34     
+ Partials       4173     4171       -2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

wzieba

Looks good, nice job! 👏

I also couldn't make rangeId empty or null. Leaving some comments but nothing blocking.

wzieba · 2025-10-06T17:46:10Z

libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/model/WCRevenueStatsModel.kt

+@Entity(
+    tableName = "RevenueStatsEntity",
+    primaryKeys = ["localSiteId", "rangeId"],
+)
+data class WCRevenueStatsModel(


The other option would be to stick to artificial primary key (as in WCRevenueStatsModel), but I think rangeId should be... id - a unique value (in context of a site).

I think your choice is correct. But you might want also check this PR (if you haven't yet) that introduces the rangeId and a check if it's empty or not: https://github.com/wordpress-mobile/WordPress-FluxC-Android/pull/2785/files#diff-3142dcb57a32d3e922e5e96b706565af858c76f424d01ecc16a41af257c37649R135-R153

During my testing I couldn't find a way to get rangeId come as empty string from the REST API.

I guess that the only use case for this is when a 3rd party plugin modifies this contract. It's though to assess the risk, though.

wzieba · 2025-10-06T17:48:48Z

.../fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/persistence/dao/RevenueStatsDao.kt

+    @Query(
+        """
+        SELECT * FROM RevenueStatsEntity
+        WHERE localSiteId = :siteId
+        AND interval = :granularity
+        AND startDate = :startDate
+        AND endDate = :endDate
+        """
+    )


I think a small @TypeConvert could be a good decision to not leave this conversion vulnerable to changes of Kotlin or Room (e.g. upper/lower case etc.).

wzieba · 2025-10-06T18:08:17Z

libs/fluxc-plugin/src/test/java/org/wordpress/android/fluxc/store/WCStatsStoreTest.kt

+                FetchRevenueStatsResponsePayload(
+                    it,
+                    StatsGranularity.DAYS,
+                    WCRevenueStatsModel(LocalId(1), "", "", "", "", "", "")


nit: I've noticed WCRevenueStatsModel(LocalId(1), "", "", "", "", "", "") is repeated in this PR. WDYT about creating a factory, test-only extension method? I'm thinking about

fun WCRevenueStatsModel.Companion.fake() = WCRevenueStatsModel(LocalId(1), "", "", "", "", "", "")

and then

Suggested change

WCRevenueStatsModel(LocalId(1), "", "", "", "", "", "")

WCRevenueStatsModel.fake()

Or you can even make it more customisable like revenue method from RevenueStatsDaoTest

Thanks for the suggestion @wzieba , will do, I also like how you've named it fake()! 💯

FYI: This is now done: 92d5229 Cc @wzieba

…to migrate/wc-revenue-stats-model-to-room

It seems that 'revenueRangeId' it totally app driven, and not REST API driven, and as such, can be completely manipulated, effectively becoming a non-empty value at all times. With this change, we are making sure that when 'revenueRangeId' comes from places like 'Widget' or 'Wear', we are adding the necessary missing information to uniquely identify this revenue range id. We are using this '<Medium><StartDate><EndDate>' pattern. FYI: This extra 'Medium' data was added to easily identify the medium. With or without 'Medium' this would have worked fine anyway. PS: Where needed, 'Medium' is either 'Widget' or 'Wear'. ------------------------------------------------------------------------ PR Comment: https://github.com/woocommerce/woocommerce-android/pull/ 14680#discussion_r2398949681

PR Comment: https://github.com/woocommerce/woocommerce-android/pull/ 14680#discussion_r2398949700

PR Comment: https://github.com/woocommerce/woocommerce-android/pull/ 14680#discussion_r2407770941

…to migrate/wc-revenue-stats-model-to-room

ParaskP7 added 6 commits October 2, 2025 16:27

Cleanup: Remove unused available flag from revenue response payload

f6a6cdd

Cleanup: Remove unnecessary rows affected from on revenue stats changed

9906fdc

Refactor: Make getXyzRevenueStats suspend functions

e0ec99d

FYI: This is done in preparation to the subsequent WellSQL to Room table migration.

Migrate: 'WCRevenueStatsModel' from wellsql to room

23c1552

FYI: This change includes but is not limited to the: - Entity Introduction - DAO Introduction (With Test) - WCAndroidDatabase Migration - Code Adjustments (Store) - Test Adjustments & Fixes (Store)

Db: Drop 'WCRevenueStatsModel' table from 'wp-fluxc'

97ac561

Analysis: Suppress unit test naming rule closer to revenue stats test

937ac1c

ParaskP7 added this to the 23.5 milestone Oct 2, 2025

ParaskP7 requested a review from Copilot October 2, 2025 13:58

ParaskP7 added the type: technical debt Represents or solves tech debt of the project. label Oct 2, 2025

Copilot AI reviewed Oct 2, 2025

View reviewed changes

ParaskP7 marked this pull request as ready for review October 2, 2025 14:45

ParaskP7 requested a review from wzieba October 2, 2025 14:50

wzieba approved these changes Oct 6, 2025

View reviewed changes

ParaskP7 added 5 commits October 8, 2025 13:40

Merge branch 'trunk' of github.com:woocommerce/woocommerce-android in…

c44e959

…to migrate/wc-revenue-stats-model-to-room

Migration: Introduce type converter for stats granularity

1fe4788

PR Comment: https://github.com/woocommerce/woocommerce-android/pull/ 14680#discussion_r2398949700

Test: Reduce duplicate fakes within wc stats store test

92d5229

PR Comment: https://github.com/woocommerce/woocommerce-android/pull/ 14680#discussion_r2407770941

Merge branch 'trunk' of github.com:woocommerce/woocommerce-android in…

d057e20

…to migrate/wc-revenue-stats-model-to-room

ParaskP7 requested a review from malinajirka October 8, 2025 13:52

Analysis: Fix detect issues on type converter tests

6798713

	WCRevenueStatsModel(LocalId(1), "", "", "", "", "", "")
	WCRevenueStatsModel.fake()

[WellSQL Migration] Migrate WCRevenueStatsModel to Room #14680

Are you sure you want to change the base?

[WellSQL Migration] Migrate WCRevenueStatsModel to Room #14680

Conversation

ParaskP7 commented Oct 2, 2025

Description

Testing information

My store + Performance

Images/gif

BEFORE WCRevenueStatsModel

AFTER RevenueStatsEntity

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

BEFORE WCRevenueStatsModel

AFTER RevenueStatsEntity

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dangermattic commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wpmobilebot commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wpmobilebot commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

wzieba left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

[WellSQL Migration] Migrate `WCRevenueStatsModel` to Room #14680

[WellSQL Migration] Migrate `WCRevenueStatsModel` to Room #14680

`My store` + `Performance`

BEFORE `WCRevenueStatsModel`

AFTER `RevenueStatsEntity`

BEFORE `WCRevenueStatsModel`

AFTER `RevenueStatsEntity`

dangermattic commented Oct 2, 2025 •

edited

Loading

wpmobilebot commented Oct 2, 2025 •

edited

Loading

wpmobilebot commented Oct 2, 2025 •

edited

Loading

codecov-commenter commented Oct 2, 2025 •

edited

Loading