diff --git a/Modules/Sources/Networking/Remote/BookingsRemote.swift b/Modules/Sources/Networking/Remote/BookingsRemote.swift index d8bc1c5fdf7..8fe00809a05 100644 --- a/Modules/Sources/Networking/Remote/BookingsRemote.swift +++ b/Modules/Sources/Networking/Remote/BookingsRemote.swift @@ -8,7 +8,9 @@ import Foundation public protocol BookingsRemoteProtocol { func loadAllBookings(for siteID: Int64, pageNumber: Int, - pageSize: Int) async throws -> [Booking] + pageSize: Int, + startDateBefore: String?, + startDateAfter: String?) async throws -> [Booking] } /// Booking: Remote Endpoints @@ -23,15 +25,27 @@ public final class BookingsRemote: Remote, BookingsRemoteProtocol { /// - siteID: Site for which we'll fetch remote bookings. /// - pageNumber: Number of page that should be retrieved. /// - pageSize: Number of bookings to be retrieved per page. + /// - startDateBefore: Filter bookings with start date before this timestamp. + /// - startDateAfter: Filter bookings with start date after this timestamp. /// public func loadAllBookings(for siteID: Int64, pageNumber: Int = Default.pageNumber, - pageSize: Int = Default.pageSize) async throws -> [Booking] { - let parameters = [ + pageSize: Int = Default.pageSize, + startDateBefore: String? = nil, + startDateAfter: String? = nil) async throws -> [Booking] { + var parameters = [ ParameterKey.page: String(pageNumber), ParameterKey.perPage: String(pageSize) ] + if let startDateBefore = startDateBefore { + parameters[ParameterKey.startDateBefore] = startDateBefore + } + + if let startDateAfter = startDateAfter { + parameters[ParameterKey.startDateAfter] = startDateAfter + } + let path = Path.bookings let request = JetpackRequest(wooApiVersion: .wcBookings, method: .get, siteID: siteID, path: path, parameters: parameters, availableAsRESTRequest: true) let mapper = ListMapper(siteID: siteID) @@ -53,7 +67,9 @@ public extension BookingsRemote { } private enum ParameterKey { - static let page: String = "page" - static let perPage: String = "per_page" + static let page: String = "page" + static let perPage: String = "per_page" + static let startDateBefore: String = "start_date_before" + static let startDateAfter: String = "start_date_after" } } diff --git a/Modules/Sources/Yosemite/Actions/BookingAction.swift b/Modules/Sources/Yosemite/Actions/BookingAction.swift index b9bf4b7e89e..69b84cbe04e 100644 --- a/Modules/Sources/Yosemite/Actions/BookingAction.swift +++ b/Modules/Sources/Yosemite/Actions/BookingAction.swift @@ -13,6 +13,9 @@ public enum BookingAction: Action { case synchronizeBookings(siteID: Int64, pageNumber: Int, pageSize: Int = BookingsRemote.Default.pageSize, + startDateBefore: String? = nil, + startDateAfter: String? = nil, + shouldClearCache: Bool = false, onCompletion: (Result) -> Void) /// Checks if the store already has any bookings. diff --git a/Modules/Sources/Yosemite/Stores/BookingStore.swift b/Modules/Sources/Yosemite/Stores/BookingStore.swift index 72a8c73ddf7..7c9869ea04e 100644 --- a/Modules/Sources/Yosemite/Stores/BookingStore.swift +++ b/Modules/Sources/Yosemite/Stores/BookingStore.swift @@ -35,8 +35,14 @@ public class BookingStore: Store { } switch action { - case let .synchronizeBookings(siteID, pageNumber, pageSize, onCompletion): - synchronizeBookings(siteID: siteID, pageNumber: pageNumber, pageSize: pageSize, onCompletion: onCompletion) + case let .synchronizeBookings(siteID, pageNumber, pageSize, startDateBefore, startDateAfter, shouldClearCache, onCompletion): + synchronizeBookings(siteID: siteID, + pageNumber: pageNumber, + pageSize: pageSize, + startDateBefore: startDateBefore, + startDateAfter: startDateAfter, + shouldClearCache: shouldClearCache, + onCompletion: onCompletion) case let .checkIfStoreHasBookings(siteID, onCompletion): checkIfStoreHasBookings(siteID: siteID, onCompletion: onCompletion) } @@ -53,17 +59,21 @@ private extension BookingStore { func synchronizeBookings(siteID: Int64, pageNumber: Int, pageSize: Int, + startDateBefore: String?, + startDateAfter: String?, + shouldClearCache: Bool, onCompletion: @escaping (Result) -> Void) { Task { @MainActor in do { let bookings = try await remote.loadAllBookings(for: siteID, pageNumber: pageNumber, - pageSize: pageSize) - let shouldDeleteExistingBookings = pageNumber == Default.firstPageNumber + pageSize: pageSize, + startDateBefore: startDateBefore, + startDateAfter: startDateAfter) await upsertStoredBookingsInBackground( readOnlyBookings: bookings, siteID: siteID, - shouldDeleteExistingBookings: shouldDeleteExistingBookings + shouldDeleteExistingBookings: shouldClearCache ) let hasNextPage = bookings.count == pageSize onCompletion(.success(hasNextPage)) @@ -89,7 +99,9 @@ private extension BookingStore { do { let bookings = try await remote.loadAllBookings(for: siteID, pageNumber: 1, - pageSize: 1) + pageSize: 1, + startDateBefore: nil, + startDateAfter: nil) let hasRemoteBookings = !bookings.isEmpty onCompletion(.success(hasRemoteBookings)) } catch { diff --git a/Modules/Tests/NetworkingTests/Remote/BookingsRemoteTests.swift b/Modules/Tests/NetworkingTests/Remote/BookingsRemoteTests.swift index dc9c11801e8..45d4dd475b8 100644 --- a/Modules/Tests/NetworkingTests/Remote/BookingsRemoteTests.swift +++ b/Modules/Tests/NetworkingTests/Remote/BookingsRemoteTests.swift @@ -1,5 +1,6 @@ import Testing @testable import Networking +@testable import NetworkingCore struct BookingsRemoteTests { @@ -35,4 +36,48 @@ struct BookingsRemoteTests { _ = try await remote.loadAllBookings(for: sampleSiteID) } } + + @Test func test_loadAllBookings_sends_correct_parameters() async throws { + // Given + let remote = BookingsRemote(network: network) + let startDateBefore = "2024-12-31T23:59:59" + let startDateAfter = "2024-01-01T00:00:00" + network.simulateResponse(requestUrlSuffix: "bookings", filename: "booking-list") + + // When + _ = try await remote.loadAllBookings(for: sampleSiteID, + pageNumber: 2, + pageSize: 50, + startDateBefore: startDateBefore, + startDateAfter: startDateAfter) + + // Then + let request = try #require(network.requestsForResponseData.first as? JetpackRequest) + let parameters = request.parameters + + #expect((parameters["page"] as? String) == "2") + #expect((parameters["per_page"] as? String) == "50") + #expect((parameters["start_date_before"] as? String) == startDateBefore) + #expect((parameters["start_date_after"] as? String) == startDateAfter) + } + + @Test func test_loadAllBookings_omits_nil_date_parameters() async throws { + // Given + let remote = BookingsRemote(network: network) + network.simulateResponse(requestUrlSuffix: "bookings", filename: "booking-list") + + // When + _ = try await remote.loadAllBookings(for: sampleSiteID, + startDateBefore: nil, + startDateAfter: nil) + + // Then + let request = try #require(network.requestsForResponseData.first as? JetpackRequest) + let parameters = request.parameters + + #expect(parameters["start_date_before"] == nil) + #expect(parameters["start_date_after"] == nil) + #expect(parameters["page"] != nil) + #expect(parameters["per_page"] != nil) + } } diff --git a/Modules/Tests/YosemiteTests/Mocks/MockBookingsRemote.swift b/Modules/Tests/YosemiteTests/Mocks/MockBookingsRemote.swift index 57065c586c6..19004ed5342 100644 --- a/Modules/Tests/YosemiteTests/Mocks/MockBookingsRemote.swift +++ b/Modules/Tests/YosemiteTests/Mocks/MockBookingsRemote.swift @@ -10,7 +10,11 @@ final class MockBookingsRemote: BookingsRemoteProtocol { loadAllBookingsResult = result } - func loadAllBookings(for siteID: Int64, pageNumber: Int, pageSize: Int) async throws -> [Booking] { + func loadAllBookings(for siteID: Int64, + pageNumber: Int, + pageSize: Int, + startDateBefore: String?, + startDateAfter: String?) async throws -> [Booking] { guard let result = loadAllBookingsResult else { throw NetworkError.timeout() } diff --git a/Modules/Tests/YosemiteTests/Stores/BookingStoreTests.swift b/Modules/Tests/YosemiteTests/Stores/BookingStoreTests.swift index fb153d4973e..3dddd900cb6 100644 --- a/Modules/Tests/YosemiteTests/Stores/BookingStoreTests.swift +++ b/Modules/Tests/YosemiteTests/Stores/BookingStoreTests.swift @@ -207,6 +207,77 @@ struct BookingStoreTests { #expect(storedBookingCount == 2) } + @Test func synchronizeBookings_clears_existing_bookings_when_shouldClearCache_is_true() async throws { + // Given + let existingBooking = Booking.fake().copy(siteID: sampleSiteID, bookingID: 999) + storeBooking(existingBooking) + #expect(storedBookingCount == 1) + + let newBooking = Booking.fake().copy(siteID: sampleSiteID, bookingID: 123) + remote.whenLoadingAllBookings(thenReturn: .success([newBooking])) + let store = BookingStore(dispatcher: Dispatcher(), + storageManager: storageManager, + network: network, + remote: remote) + + // When + let result = await withCheckedContinuation { continuation in + store.onAction(BookingAction.synchronizeBookings(siteID: sampleSiteID, + pageNumber: defaultPageNumber, + pageSize: defaultPageSize, + shouldClearCache: true, + onCompletion: { result in + continuation.resume(returning: result) + })) + } + + // Then + #expect(result.isSuccess) + #expect(storedBookingCount == 1) + let storedBooking = try #require(viewStorage.loadBooking(siteID: sampleSiteID, bookingID: 123)) + #expect(storedBooking.bookingID == 123) + + // Verify the existing booking was cleared + let existingStoredBooking = viewStorage.loadBooking(siteID: sampleSiteID, bookingID: 999) + #expect(existingStoredBooking == nil) + } + + @Test func synchronizeBookings_preserves_existing_bookings_when_shouldClearCache_is_false() async throws { + // Given + let existingBooking = Booking.fake().copy(siteID: sampleSiteID, bookingID: 999) + storeBooking(existingBooking) + #expect(storedBookingCount == 1) + + let newBooking = Booking.fake().copy(siteID: sampleSiteID, bookingID: 123) + remote.whenLoadingAllBookings(thenReturn: .success([newBooking])) + let store = BookingStore(dispatcher: Dispatcher(), + storageManager: storageManager, + network: network, + remote: remote) + + // When + let result = await withCheckedContinuation { continuation in + store.onAction(BookingAction.synchronizeBookings(siteID: sampleSiteID, + pageNumber: defaultPageNumber, + pageSize: defaultPageSize, + shouldClearCache: false, + onCompletion: { result in + continuation.resume(returning: result) + })) + } + + // Then + #expect(result.isSuccess) + #expect(storedBookingCount == 2) + + // Verify both bookings exist + let newStoredBooking = try #require(viewStorage.loadBooking(siteID: sampleSiteID, bookingID: 123)) + #expect(newStoredBooking.bookingID == 123) + + let existingStoredBooking = try #require(viewStorage.loadBooking(siteID: sampleSiteID, bookingID: 999)) + #expect(existingStoredBooking.bookingID == 999) + } + // MARK: - checkIfStoreHasBookings @Test func checkIfStoreHasBookings_returns_true_when_bookings_exist_locally() async throws { diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Bookings/BookingListViewModelTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Bookings/BookingListViewModelTests.swift index b0c8a13bdc5..a744cb7b62a 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Bookings/BookingListViewModelTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Bookings/BookingListViewModelTests.swift @@ -92,7 +92,7 @@ struct BookingListViewModelTests { let stores = MockStoresManager(sessionManager: .testingInstance) let booking = Booking.fake().copy(siteID: sampleSiteID) stores.whenReceivingAction(ofType: BookingAction.self) { action in - guard case let .synchronizeBookings(_, _, _, onCompletion) = action else { + guard case let .synchronizeBookings(_, _, _, _, _, _, onCompletion) = action else { return } self.insertBookings([booking]) @@ -130,7 +130,7 @@ struct BookingListViewModelTests { // Given let stores = MockStoresManager(sessionManager: .testingInstance) stores.whenReceivingAction(ofType: BookingAction.self) { action in - guard case let .synchronizeBookings(_, _, _, onCompletion) = action else { + guard case let .synchronizeBookings(_, _, _, _, _, _, onCompletion) = action else { return } onCompletion(.success(false)) @@ -170,7 +170,7 @@ struct BookingListViewModelTests { let firstPageItems = [Booking](repeating: .fake().copy(siteID: sampleSiteID), count: 2) let secondPageItems = [Booking](repeating: .fake().copy(siteID: sampleSiteID), count: 1) stores.whenReceivingAction(ofType: BookingAction.self) { action in - guard case let .synchronizeBookings(_, pageNumber, _, onCompletion) = action else { + guard case let .synchronizeBookings(_, pageNumber, _, _, _, _, onCompletion) = action else { return } invocationCountOfLoadBookings += 1 @@ -218,7 +218,7 @@ struct BookingListViewModelTests { let booking1 = Booking.fake().copy(siteID: sampleSiteID, bookingID: 9) let booking2 = Booking.fake().copy(siteID: sampleSiteID, bookingID: 10) stores.whenReceivingAction(ofType: BookingAction.self) { action in - guard case let .synchronizeBookings(_, _, _, onCompletion) = action else { + guard case let .synchronizeBookings(_, _, _, _, _, _, onCompletion) = action else { return } self.insertBookings([booking1, booking2]) @@ -243,7 +243,7 @@ struct BookingListViewModelTests { // Given let stores = MockStoresManager(sessionManager: .testingInstance) stores.whenReceivingAction(ofType: BookingAction.self) { action in - guard case let .synchronizeBookings(_, _, _, onCompletion) = action else { + guard case let .synchronizeBookings(_, _, _, _, _, _, onCompletion) = action else { return } onCompletion(.success(false)) @@ -266,7 +266,7 @@ struct BookingListViewModelTests { let olderBooking = Booking.fake().copy(siteID: sampleSiteID, bookingID: 1, dateCreated: Date(timeIntervalSince1970: 1000)) let newerBooking = Booking.fake().copy(siteID: sampleSiteID, bookingID: 3, dateCreated: Date(timeIntervalSince1970: 2000)) stores.whenReceivingAction(ofType: BookingAction.self) { action in - guard case let .synchronizeBookings(_, _, _, onCompletion) = action else { + guard case let .synchronizeBookings(_, _, _, _, _, _, onCompletion) = action else { return } let items = [olderBooking, newerBooking] @@ -295,7 +295,7 @@ struct BookingListViewModelTests { var invocationCountOfLoadBookings = 0 var skip: Int? stores.whenReceivingAction(ofType: BookingAction.self) { action in - guard case let .synchronizeBookings(_, pageNumber, pageSize, onCompletion) = action else { + guard case let .synchronizeBookings(_, pageNumber, pageSize, _, _, _, onCompletion) = action else { return } invocationCountOfLoadBookings += 1