Skip to content

Conversation

@wzieba
Copy link
Contributor

@wzieba wzieba commented Nov 21, 2025

Description

This PR migrates WCShippingLabelCreationEligibility from WellSql to Room.

The migration, like always, focuses on actually migrating database entities and not fixing threading issues. All database calls on main thread are now more visible though, because of the runBlocking usage.

Test Steps

Testing this PR requires a site with the old shipping labels flow. See this internal Slack thread for details and website, which has this flow enabled right now: p1764335185877589/1764334560.167769-slack-C03L1NF1EA3

  1. Open the app
  2. Open orders that are eligible for shipping labels
  3. See that eligibility data is cached under ShippingLabelCreationEligibilityEntity table.

Images/gif

Before (trunk)

Screenshot 2025-12-03 at 17 08 21

After

Screenshot 2025-12-03 at 17 00 21
  • 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.

wzieba and others added 4 commits November 21, 2025 11:16
- Convert model to data class with Room annotations
- Use composite primary key [localSiteId, remoteOrderId]
- Replace synthetic primary key with LocalId/RemoteId types
- Add ShippingLabelCreationEligibilityDao with upsert and query methods
- Add comprehensive DAO unit tests covering all operations
- Table name: ShippingLabelCreationEligibilityEntity

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
- Update WCShippingLabelStore to use ShippingLabelCreationEligibilityDao
- Make store constructor internal to allow internal DAO injection
- Use runBlocking for DAO calls outside coroutines
- Remove rowsAffected handling
- Delete WCShippingLabelSqlUtils

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
- Register WCShippingLabelCreationEligibility entity in WCAndroidDatabase
- Add ShippingLabelCreationEligibilityDao to database
- Increment database version from 74 to 75
- Add AutoMigration(from = 74, to = 75)
- Add WellSql migration 234 to drop legacy table

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
- Remove ADDON_WOOCOMMERCE parameter from test configurations
- Update WCDataStoreTest, WCProductStoreTest, WCShippingLabelStoreTest
- Update WooCommerceStoreTest, WCLeaderboardsStoreTest, WCProductLeaderboardsMapperTest
- Remove unused WellSqlConfig imports

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@dangermattic
Copy link
Collaborator

dangermattic commented Nov 21, 2025

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
Copy link
Collaborator

wpmobilebot commented Nov 21, 2025

📲 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 NameWooCommerce-Wear Android
Platform⌚️ Wear OS
FlavorJalapeno
Build TypeDebug
Commit29beb60
Direct Downloadwoocommerce-wear-prototype-build-pr15008-29beb60.apk

Without this provider, Hilt cannot inject the DAO into WCShippingLabelStore,
causing build failures.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Nov 21, 2025

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

App NameWooCommerce Android
Platform📱 Mobile
FlavorJalapeno
Build TypeDebug
Commit29beb60
Direct Downloadwoocommerce-prototype-build-pr15008-29beb60.apk

Base automatically changed from migrate_order_shipment_tracking_model to trunk November 27, 2025 17:36
@wzieba wzieba changed the title Migrate shipping label creation eligibility Migrate WCShippingLabelCreationEligibility to Room Nov 28, 2025
@wzieba wzieba added the type: technical debt Represents or solves tech debt of the project. label Nov 28, 2025
@wzieba wzieba added this to the 23.9 milestone Nov 28, 2025
@codecov-commenter
Copy link

codecov-commenter commented Dec 3, 2025

Codecov Report

❌ Patch coverage is 0% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.61%. Comparing base (d3538d8) to head (29beb60).
⚠️ Report is 40 commits behind head on trunk.

Files with missing lines Patch % Lines
...dpress/android/fluxc/store/WCShippingLabelStore.kt 0.00% 18 Missing ⚠️
...ippinglabels/WCShippingLabelCreationEligibility.kt 0.00% 9 Missing ⚠️
...rdpress/android/fluxc/persistence/WellSqlConfig.kt 0.00% 4 Missing ⚠️
...kotlin/com/woocommerce/android/WooWellSqlConfig.kt 0.00% 1 Missing ⚠️
...istence/dao/ShippingLabelCreationEligibilityDao.kt 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##              trunk   #15008   +/-   ##
=========================================
  Coverage     38.61%   38.61%           
  Complexity    10312    10312           
=========================================
  Files          2163     2163           
  Lines        122674   122654   -20     
  Branches      16934    16930    -4     
=========================================
  Hits          47366    47366           
+ Misses        70503    70483   -20     
  Partials       4805     4805           

☔ 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 wzieba requested a review from irfano December 3, 2025 16:55
@wzieba wzieba marked this pull request as ready for review December 3, 2025 16:55
@wzieba wzieba force-pushed the migrate_shipping_label_creation_eligibility branch from 73d75ee to c19180f Compare December 4, 2025 10:04
@wzieba wzieba requested a review from a team as a code owner December 4, 2025 10:04
@wzieba wzieba marked this pull request as draft December 4, 2025 10:05
@wzieba wzieba force-pushed the migrate_shipping_label_creation_eligibility branch from c19180f to 73d75ee Compare December 4, 2025 10:06
…remote` prefixes

This is redundant, as we use types for this
@wzieba wzieba marked this pull request as ready for review December 4, 2025 11:11
@irfano irfano self-assigned this Dec 4, 2025
Copy link
Contributor

@irfano irfano left a comment

Choose a reason for hiding this comment

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

LGTM and works as expected. I left minor feedback, but I’m approving. 🚢

}

@Test
fun `when eligibility is not eligible with reason, then reason is persisted`() = runTest {
Copy link
Contributor

Choose a reason for hiding this comment

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

This test is actually doing the exact same thing as when eligibility is upserted, then it can be retrieved. Its only benefit is that it states its purpose more explicitly. Feel free to ignore, but I prefer keeping the test suite less crowded.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, I totally agree! I missed this when reviewing AI, fixed in: 29beb60

@wzieba wzieba enabled auto-merge December 4, 2025 16:58
@wzieba wzieba merged commit f174666 into trunk Dec 4, 2025
18 checks passed
@wzieba wzieba deleted the migrate_shipping_label_creation_eligibility branch December 4, 2025 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: technical debt Represents or solves tech debt of the project.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants