Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions Modules/Sources/Yosemite/Actions/BookingAction.swift
Original file line number Diff line number Diff line change
Expand Up @@ -103,4 +103,12 @@ public enum BookingAction: Action {
bookingID: Int64,
note: String,
onCompletion: (Error?) -> Void)


/// Clears the booking cache.
///
/// - Parameter siteID: The site ID of the booking.
/// - Parameter onCompletion: Called when clear completes.
case clearBookingsCache(siteID: Int64,
onCompletion: () -> Void)
}
26 changes: 17 additions & 9 deletions Modules/Sources/Yosemite/Stores/BookingStore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ public class BookingStore: Store {
note: note,
onCompletion: onCompletion
)
case .clearBookingsCache(siteID: let siteID, onCompletion: let onCompletion):
clearBookingsCache(siteID: siteID, onCompletion: onCompletion)
}
}
}
Expand Down Expand Up @@ -205,12 +207,12 @@ private extension BookingStore {
/// Returns results immediately without saving to storage.
///
func searchBookings(siteID: Int64,
searchQuery: String,
pageNumber: Int,
pageSize: Int,
filters: BookingFilters?,
order: BookingsRemote.Order,
onCompletion: @escaping (Result<[Booking], Error>) -> Void) {
searchQuery: String,
pageNumber: Int,
pageSize: Int,
filters: BookingFilters?,
order: BookingsRemote.Order,
onCompletion: @escaping (Result<[Booking], Error>) -> Void) {
Task { @MainActor in
do {
let bookings = try await remote.loadAllBookings(for: siteID,
Expand Down Expand Up @@ -271,9 +273,9 @@ private extension BookingStore {
/// Synchronizes booking resources for the specified site.
///
func synchronizeResources(siteID: Int64,
pageNumber: Int,
pageSize: Int,
onCompletion: @escaping (Result<Bool, Error>) -> Void) {
pageNumber: Int,
pageSize: Int,
onCompletion: @escaping (Result<Bool, Error>) -> Void) {
Task { @MainActor in
do {
let resources = try await remote.fetchResources(
Expand Down Expand Up @@ -461,6 +463,12 @@ private extension BookingStore {
}
}
}

func clearBookingsCache(siteID: Int64, onCompletion: @escaping () -> Void) {
storageManager.performAndSave({ storage in
storage.deleteBookings(siteID: siteID)
}, completion: onCompletion, on: .main)
}
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ final class BookingListContainerViewModel: ObservableObject {
private var searchQuerySubscription: AnyCancellable?
private var sortBySubscription: AnyCancellable?

private lazy var allTabViewModels: [BookingListViewModel] = [
todayListViewModel,
upcomingListViewModel,
allListViewModel
]

private var filters = BookingFiltersViewModel.Filters()

Expand Down Expand Up @@ -87,16 +92,20 @@ final class BookingListContainerViewModel: ObservableObject {
}

restorePersistedFilters()

todayListViewModel.refreshCoordinator = self
upcomingListViewModel.refreshCoordinator = self
allListViewModel.refreshCoordinator = self
}

func listViewModel(for tab: BookingListTab) -> BookingListViewModel {
switch tab {
case .today:
todayListViewModel
return todayListViewModel
case .upcoming:
upcomingListViewModel
return upcomingListViewModel
case .all:
allListViewModel
return allListViewModel
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious: why is this necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not needed at all! I was just fiddling around with the tabs, and I guess I haven't fully reverted one of my changes. I'll remove the returns.

}
}

Expand Down Expand Up @@ -181,6 +190,29 @@ private extension BookingListContainerViewModel {
}
}

extension BookingListContainerViewModel: BookingListsRefreshCoordinating {
@MainActor
func refreshAllLists() async {
await withCheckedContinuation { continuation in
let action = BookingAction.clearBookingsCache(siteID: siteID) {
continuation.resume()
}
stores.dispatch(action)
}

// Launch all tab refreshes in parallel and wait for all to complete
await withTaskGroup(of: Void.self) { group in
for viewModel in allTabViewModels {
group.addTask { @MainActor in
await viewModel.onRefreshSelfAction(
reason: BookingListViewModel.siblingRefreshReason
)
}
}
}
}
}

private extension BookingListContainerViewModel {
enum Localization {
static let filter = NSLocalizedString(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ import Combine
import protocol Storage.StorageManagerType
import class Networking.BookingsRemote

protocol BookingListsRefreshCoordinating: AnyObject {
func refreshAllLists() async
}

/// View model for `BookingListView`
final class BookingListViewModel: ObservableObject {

Expand All @@ -14,6 +18,8 @@ final class BookingListViewModel: ObservableObject {

@Published private(set) var hasFilters = false

weak var refreshCoordinator: BookingListsRefreshCoordinating?

var emptyStateTitle: String {
type.emptyStateTitle(hasFilters: hasFilters)
}
Expand All @@ -32,6 +38,7 @@ final class BookingListViewModel: ObservableObject {

private static let refreshCacheReason = "refresh-cache"
private static let reorderReason = "reorder"
static let siblingRefreshReason = "sibling-refresh"

/// Keeps track of the current state of the syncing
@Published private(set) var syncState: SyncState = .empty
Expand All @@ -58,6 +65,7 @@ final class BookingListViewModel: ObservableObject {

init(siteID: Int64,
type: BookingListTab,
parent: BookingListContainerViewModel? = nil,
stores: StoresManager = ServiceLocator.stores,
storage: StorageManagerType = ServiceLocator.storageManager,
currentDate: Date = Date()) {
Expand Down Expand Up @@ -96,10 +104,14 @@ final class BookingListViewModel: ObservableObject {
}

/// Called when the user pulls down the list to refresh.
@MainActor
func onRefreshAction() async {
await refreshCoordinator?.refreshAllLists()
}

@MainActor
func onRefreshSelfAction(reason: String? = nil) async {
await withCheckedContinuation { continuation in
paginationTracker.resync(reason: Self.refreshCacheReason) {
paginationTracker.resync(reason: reason ?? Self.refreshCacheReason) {
Copy link
Contributor

@itsmeichigo itsmeichigo Dec 1, 2025

Choose a reason for hiding this comment

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

How about keeping the refresh cache as the default value for simplicity? And should we rename the method to reloadData for clarity?

Suggested change
func onRefreshSelfAction(reason: String? = nil) async {
await withCheckedContinuation { continuation in
paginationTracker.resync(reason: Self.refreshCacheReason) {
paginationTracker.resync(reason: reason ?? Self.refreshCacheReason) {
func reloadData(reason: String = Self.refreshCacheReason) async {
await withCheckedContinuation { continuation in
paginationTracker.resync(reason: reason) {

continuation.resume(returning: ())
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ final class BookingSearchViewModel: ObservableObject {

/// Called when the user pulls down the list to refresh.
@MainActor
func onRefreshAction() async {
func onRefreshAction(reason: String? = nil) async {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this change necessary? Unrelated but looks like the pull to refresh functionality doesn't work on the search view any more.

await withCheckedContinuation { continuation in
searchPaginationTracker.resync(reason: nil) {
continuation.resume(returning: ())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ struct BookingListViewModelTests {
let viewModel = BookingListViewModel(siteID: sampleSiteID, type: .all, stores: stores)

// When
await viewModel.onRefreshAction()
await viewModel.onRefreshSelfAction()

// Then
#expect(skip == 0)
Expand Down Expand Up @@ -441,26 +441,18 @@ struct BookingListViewModelTests {
#expect(actionCallCount == 2, "Should have made two API calls")
}

@Test func on_refresh_action_clears_cache() async {
@Test func on_refresh_action_calls_refreshcoordiantor() async {
// Given
let stores = MockStoresManager(sessionManager: .testingInstance)
var capturedShouldClearCache: Bool?

stores.whenReceivingAction(ofType: BookingAction.self) { action in
guard case let .synchronizeBookings(_, _, _, _, _, shouldClearCache, onCompletion) = action else {
return
}
capturedShouldClearCache = shouldClearCache
onCompletion(.success(false))
}

let mockRefresher = MockBookingListsRefreshCoordinating()
let viewModel = BookingListViewModel(siteID: sampleSiteID, type: .all, stores: stores)
viewModel.refreshCoordinator = mockRefresher

// When
await viewModel.onRefreshAction()

// Then
#expect(capturedShouldClearCache == true, "Refresh action should clear cache")
#expect(mockRefresher.refreshAllListsCalled)
}

// MARK: - Local storage filtering
Expand Down Expand Up @@ -719,3 +711,12 @@ private extension BookingListViewModelTests {
return Booking.fake().copy(siteID: siteID ?? self.sampleSiteID, bookingID: id, startDate: startDate)
}
}


class MockBookingListsRefreshCoordinating: BookingListsRefreshCoordinating {
private(set) var refreshAllListsCalled = false

func refreshAllLists() async {
refreshAllListsCalled = true
}
}