Skip to content

Conversation

@irfano
Copy link
Contributor

@irfano irfano commented Dec 4, 2025

Description

This PR addresses a data consistency issue where stale booking data could persist in the local cache when applying filters.

Previously, when fetching filtered bookings, we used a simple insertOrReplace strategy. This updated existing bookings but failed to remove bookings that were no longer present in the server response (e.g., deleted bookings or bookings that no longer matched the filter).

While we had previously addressed this issue specifically for Today and Upcoming tabs (#15022), this fix extends that same robust cleanup logic to all filter types. It uses a single optimized SQL query to:

  1. Identify local bookings that match the current filter criteria.
  2. Delete those that are missing from the fresh server response.
  3. Insert/Update the new bookings.

Pagination & Data Sync Strategy
To ensure data integrity, we clear the entire cache for the active filter when refreshing the list (page 1).
Why clear pages 2, 3, etc.? Without backend support for deleted items, we can't reliably sync a shifting list client-side.

  • The Risk: If a new booking is added, items shift from Page 1 to Page 2. If we only update Page 1, our logic would see the shifted item as "missing" from Page 1 and delete it, causing accidental data loss.
  • The Solution: We prioritize data accuracy over retaining offline data for older pages.

Test Steps

  1. Make sure you have some bookings in the Today and Upcoming tabs
  2. Launch the app
  3. Go to Today and load the bookings
  4. Trash one of the booking from the Today tab via the web
  5. Refresh the Today tab in the app
  6. Verify the booking was removed
  7. Repeat 3-6 for the All tab.

Images/gif

Before

before-bug.mov

After

after.mov
  • 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.

Update `cleanAndUpsertBookings` to use the new comprehensive filtering when deleting stale booking records.
The new implementation uses a single `cleanAndUpsertBookings` call when filters are applied, removing the special handling for "Today" or "Upcoming" filters.
}
page == 1 && query.isNullOrEmpty() -> {
// Refreshing with filters applied: selectively remove only the stale entries.
bookingsDao.cleanAndUpsertBookings(site.localId(), filters, entities)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the main change: cleanAndUpsertBookings() previously received only the date filters, but now it receives the entire filter object.

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Dec 4, 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
Commit6e45b92
Direct Downloadwoocommerce-wear-prototype-build-pr15061-6e45b92.apk

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Dec 4, 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
Commit6e45b92
Direct Downloadwoocommerce-prototype-build-pr15061-6e45b92.apk

@codecov-commenter
Copy link

codecov-commenter commented Dec 4, 2025

Codecov Report

❌ Patch coverage is 21.87500% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.69%. Comparing base (8257c85) to head (6e45b92).
⚠️ Report is 216 commits behind head on trunk.

Files with missing lines Patch % Lines
...press/android/fluxc/persistence/dao/BookingsDao.kt 0.00% 21 Missing ⚠️
...xc/network/rest/wpcom/wc/bookings/BookingsStore.kt 55.55% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##              trunk   #15061      +/-   ##
============================================
- Coverage     38.70%   38.69%   -0.01%     
+ Complexity    10326    10322       -4     
============================================
  Files          2162     2162              
  Lines        122532   122533       +1     
  Branches      16918    16918              
============================================
- Hits          47424    47414      -10     
- Misses        70307    70317      +10     
- Partials       4801     4802       +1     

☔ 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.

@JorgeMucientes JorgeMucientes self-assigned this Dec 12, 2025
@JorgeMucientes
Copy link
Contributor

Hey @irfano because of this very recent API change where attendance_status field is missing from Booking entity reponse. Reported it in Slack: p1765542693182419-slack-C09224W4HAA
I've added a slight change in this PR (hope that is all right 🙏🏼 ) to avoid the app crashing when this happens. That way I can test these changes and we also fix the issue for CIAB sites.

Btw, I learned that when you set a default value to a non-nullable field in a DTO model class Gson might still parse it as null when the field is missing.

Gson uses reflection and Unsafe.allocateInstance to create objects, so Kotlin’s constructor (where default values and null checks live) is never executed during deserialization.​
As a result, a property declared as val name: String = "default" can end up being null if the JSON has name: null, or it can stay uninitialized if the field is missing and Gson writes null directly into it.

Copy link
Contributor

@JorgeMucientes JorgeMucientes left a comment

Choose a reason for hiding this comment

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

Thanks for addressing this improvement @irfano . The code changes look good and everything worked as expected in my tests

@irfano irfano enabled auto-merge December 15, 2025 17:11
@irfano irfano merged commit 84412ad into trunk Dec 15, 2025
16 of 17 checks passed
@irfano irfano deleted the issue/WOOMOB-1462-fix-cached-removed-booking-issue branch December 15, 2025 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants