[WOOMOB-1722] - Properly clean the Bookings table for Today/Upcoming tabs' first page load #15022

AdamGrzybkowski · 2025-11-26T08:38:51Z

Description

We had a small problem on Today/Upcoming tabs. If the booking displayed on one of those tabs would get deleted on the server, we wouldn't reflect that as the Booking would be cached locally and the app wouldn't delete it.

This PR fixes it by making sure that we clear the table from the bookings that were not received in the first page load for both Today and Upcoming tabs.

To do that, I extracted the creation of the Today/Upcoming DateRange filter so that I can create the same one in BookingStore and check if that's the one used. After that, the app will delete all the bookings that match the given DateRange filter and if the Booking ID is not in the newly loaded list. This was done that way to be the least destructive to the Bookings table, so we only clear the entities that are indeed unnecessary (most likely).

Test Steps

Make sure you have some bookings in the Today and Upcoming tabs
Launch the app
Go to Today and load the bookings
Trash one of the booking from the Today tab via the web
Refresh the Today tab in the app
Verify the booking was removed

Images/gif

soft_clear.mp4

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.

With this change when we fetch the initial page with the DateRange filter matching Today or Upcoming range the Bookings table will be cleared only from the bookings that are not in the initial page load.

dangermattic · 2025-11-26T08:39:43Z

	1 Warning
⚠️	This PR is assigned to the milestone 23.8. This milestone is due in less than 2 days. Please make sure to get it merged by then or assign it to a milestone with a later deadline.

Generated by 🚫 Danger

Copilot

Pull request overview

This PR fixes a caching issue where deleted bookings weren't being removed from the local database when fetched from the server. The solution implements selective deletion of cached bookings for Today/Upcoming tabs on the first page load, ensuring stale data is properly cleaned up.

Key changes:

Introduced BookingsDateRangePresets to provide canonical date range filters shared across app and data layers
Implemented conditional deletion logic in BookingsStore that clears cached bookings matching Today/Upcoming date ranges when fetching page 1
Changed BookingFilters parameter from nullable to non-nullable with EMPTY as the default value

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`BookingsDateRangePresets.kt`	New utility object providing factory methods for today() and upcoming() date range presets with proper UTC handling
`BookingsStore.kt`	Added deletion logic for page 1 fetches with today/upcoming filters, plus `isTodayOrUpcoming()` helper method
`BookingsDao.kt`	Added `deleteForSiteWithDateRangeFilter()` query to selectively delete bookings matching date range and excluding specified IDs
`BookingListFiltersBuilder.kt`	Refactored to use shared `BookingsDateRangePresets` instead of inline date range construction
`BookingListHandler.kt`	Changed filters parameter type from nullable to non-nullable `BookingFilters`
`BookingsRepository.kt`	Updated filters parameter signature to non-nullable `BookingFilters`
`BookingsStoreTest.kt`	Added test cases for new deletion behavior scenarios and updated existing tests with Clock dependency

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

...rc/test/java/org/wordpress/android/fluxc/network/rest/wpcom/wc/bookings/BookingsStoreTest.kt

.../src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/bookings/BookingsStore.kt

...rc/test/java/org/wordpress/android/fluxc/network/rest/wpcom/wc/bookings/BookingsStoreTest.kt

wpmobilebot · 2025-11-26T08:48:48Z

📲 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	`c2cf448`
	Direct Download	`woocommerce-wear-prototype-build-pr15022-c2cf448.apk`

wpmobilebot · 2025-11-26T09:08:14Z

📲 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	`c2cf448`
	Direct Download	`woocommerce-prototype-build-pr15022-c2cf448.apk`

codecov-commenter · 2025-11-26T09:24:50Z

Codecov Report

❌ Patch coverage is 65.95745% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.57%. Comparing base (e9f11a2) to head (c2cf448).
⚠️ Report is 24 commits behind head on trunk.

Files with missing lines	Patch %	Lines
...press/android/fluxc/persistence/dao/BookingsDao.kt	0.00%	14 Missing ⚠️
...xc/network/rest/wpcom/wc/bookings/BookingsStore.kt	85.71%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##              trunk   #15022      +/-   ##
============================================
+ Coverage     38.55%   38.57%   +0.02%     
- Complexity    10289    10297       +8     
============================================
  Files          2161     2162       +1     
  Lines        122614   122647      +33     
  Branches      16899    16904       +5     
============================================
+ Hits          47271    47311      +40     
+ Misses        70543    70527      -16     
- Partials       4800     4809       +9

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

...otlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/bookings/BookingsDateRangePresets.kt

irfano · 2025-11-26T14:24:34Z

.../src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/bookings/BookingsStore.kt

+                    // Clear existing bookings when fetching the first page.
+                    // If filters are applied, only clear entries that match the applied filters,
+                    // otherwise (no filters) clear all entries for the site.


The comment here is not entirely accurate. We say, If filters are applied, only clear entries that match the applied filters, but this is only true for date filters. The issue still exists for other filter types. For example, I filtered “No-show” bookings, trashed one, refreshed the list, and it was still visible.
If we want to address this for all cases, we’ll need to always delete cached entities when any filter is applied. Since it already works for the date filter, we could turn that into a more general solution. WDYT?

Since it already works for the date filter, we could turn that into a more general solution. WDYT?

I initially did that, but then I thought that there was a reason why we added this check in the first place, so I decided to strictly add this logic for Today and Upcoming filter.

I'm open to changing that. I wonder if we had similar problems with Products or Orders, and how they were solved there?

Hmm thinking about it, I'm not sure we need it. We can only apply other filters in the All tab. This tab, without filters applied, will delete or bookings for a selected site on the initial load. So the problems should be less visible and we won't be clearing the data that we would get on next page loads.

True, the issue is less visible for the All tab, but the All filter is saved, and it's possible that the user prefers to always keep a certain filter enabled. In that case, it becomes the same situation as the Today and Upcoming tabs.

I'm open to changing that. I wonder if we had similar problems with Products or Orders, and how they were solved there?

I've just tested the Products tab, and the issue exists there as well.

irfano · 2025-11-26T16:55:29Z

.../src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/bookings/BookingsStore.kt

+                                bookingsDao.deleteForSiteWithDateRangeFilter(
+                                    site.localId(),
+                                    filters.dateRange,
+                                    entities.map { it.id.value }
+                                )
+                                bookingsDao.insertOrReplace(entities)


There is also an alternative way to handle removed bookings. We can compare the new entities with the cached bookings and detect the removed ones instead of removing all cached data. I know replaceAllForSite() is what we already use, but removing only trashed bookings seems like a better approach to me.

Could you clarify? The current implementation only removes the local entities if they are not present in the response from the server. I'm not clearing all cached data here.

cachedFilteredBookings = [A, B, C] remoteFilteredBookings = [B, C, D] (A is removed) // Current implementation runNewDBTransaction { remove(A, B, C) insertOrReplace(B, C, D) } // My suggestion runNewDBTransaction { remove(A) insertOrReplace(B, C, D) }

In the current implementation, we remove the entire cachedFilteredBookings, right?

This is a minor suggestion and it won’t affect the behavior. It will make the DB work a smaller task, but we’ll still need to find the removed booking (A). Still, I think reducing the number of DB transactions is the better approach.

In the current implementation, we remove the entire cachedFilteredBookings, right?

No. We pass the IDs of the fetched bookings and have this SQL query condition AND ((:idsSize = 0) OR id NOT IN (:ids)) to make sure we only delete the ones that are not in the list. It's exactly as in your suggestion.

Oh, I missed that in the SQL. Then it’s okay. Thanks!

My last feedback: can we unify deleteForSiteWithDateRangeFilter() and insertOrReplace() into a single DB transaction? Having two separate transactions might trigger additional observations in the view models. I fixed a similar issue before here: #14800.

In this case, this is not a problem, because we are only removing items that should be deleted. The other issue was created because we were deleting and then inserting the same items.

So in a scenario from #14800 the flickering would still occur, because the item will actually be deleted, no matter if it's wrapped in one transaction or not.

I will wrap them in one transaction because it won't hurt us, but this wouldn't trigger the same issue mentioned above.

@irfano Fixed in c2cf448

Yes, I noticed it currently doesn’t cause any issues, but it could still lead to a hard-to-diagnose problem in the future. Thanks for making the change. 👍🏻

irfano

Good job noticing and addressing this issue. LGTM! This fixed the problem for the Today and Upcoming tabs. My two pieces of feedback were:
~~- Improving the stored cache deletion logic,~~

Fixing the same issue for the All tab as well.

But these can be handled in a separate PR, no need to include them here. Approving! ✅

irfano

I requested changes again. Please check my last comment about unifying the db transactions.

AdamGrzybkowski added 3 commits November 26, 2025 09:26

Prefer BookingFilters.EMPTY over nullable property

b6cd96c

Extract Today/Upcoming DateRange creation for reuse in other places

98c6ec8

Properly clear the Booking table for Today/Upcoming tabs

99c3735

With this change when we fetch the initial page with the DateRange filter matching Today or Upcoming range the Bookings table will be cleared only from the bookings that are not in the initial page load.

AdamGrzybkowski added this to the 23.8 milestone Nov 26, 2025

AdamGrzybkowski requested review from JorgeMucientes, Copilot and irfano November 26, 2025 08:38

AdamGrzybkowski added Bug feature: CIAB Mobile Bookings MVP labels Nov 26, 2025

Copilot started reviewing on behalf of AdamGrzybkowski November 26, 2025 08:39 View session

Copilot finished reviewing on behalf of AdamGrzybkowski November 26, 2025 08:42

Copilot AI reviewed Nov 26, 2025

View reviewed changes

Make sure that only date range matching today/upcoming is applied

5ef2be3

irfano self-assigned this Nov 26, 2025

irfano reviewed Nov 26, 2025

View reviewed changes

Improve BookingsDateRangePresets

e19f6d1

irfano approved these changes Nov 27, 2025

View reviewed changes

irfano requested changes Nov 27, 2025

View reviewed changes

Wrap delete and insert in one DB transaction

c2cf448

AdamGrzybkowski requested a review from irfano November 27, 2025 12:24

irfano approved these changes Nov 27, 2025

View reviewed changes

irfano merged commit 18f870e into trunk Nov 27, 2025
18 checks passed

irfano deleted the issue/WOOMOB-1722 branch November 27, 2025 13:25

[WOOMOB-1722] - Properly clean the Bookings table for Today/Upcoming tabs' first page load #15022

[WOOMOB-1722] - Properly clean the Bookings table for Today/Upcoming tabs' first page load #15022

Conversation

AdamGrzybkowski commented Nov 26, 2025

Description

Test Steps

Images/gif

Uh oh!

dangermattic commented Nov 26, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AdamGrzybkowski Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Choose a reason for hiding this comment

Uh oh!

irfano Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

irfano left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

irfano left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

wpmobilebot commented Nov 26, 2025 •

edited

Loading

wpmobilebot commented Nov 26, 2025 •

edited

Loading

codecov-commenter commented Nov 26, 2025 •

edited

Loading

AdamGrzybkowski Nov 27, 2025 •

edited

Loading

irfano Nov 27, 2025 •

edited

Loading

irfano left a comment •

edited

Loading