diff --git a/Modules/Sources/Networking/Model/Bookings/Booking.swift b/Modules/Sources/Networking/Model/Bookings/Booking.swift index b1588f33b9c..687a23b33ad 100644 --- a/Modules/Sources/Networking/Model/Bookings/Booking.swift +++ b/Modules/Sources/Networking/Model/Bookings/Booking.swift @@ -9,8 +9,8 @@ public struct Booking: Codable, GeneratedCopiable, Hashable, GeneratedFakeable { public let allDay: Bool public let cost: String public let customerID: Int64 - public let dateCreated: Date - public let dateModified: Date + public let dateCreated: Date? + public let dateModified: Date? public let endDate: Date public let googleCalendarEventID: String? public let orderID: Int64 @@ -40,8 +40,8 @@ public struct Booking: Codable, GeneratedCopiable, Hashable, GeneratedFakeable { allDay: Bool, cost: String, customerID: Int64, - dateCreated: Date, - dateModified: Date, + dateCreated: Date?, + dateModified: Date?, endDate: Date, googleCalendarEventID: String?, orderID: Int64, @@ -95,8 +95,27 @@ public struct Booking: Codable, GeneratedCopiable, Hashable, GeneratedFakeable { alternativeTypes: [.decimal(transform: { NSDecimalNumber(decimal: $0).stringValue })]) ?? "" let customerID = try container.decode(Int64.self, forKey: .customerID) - let dateCreated = Date(timeIntervalSince1970: try container.decode(Double.self, forKey: .dateCreated)) - let dateModified = Date(timeIntervalSince1970: try container.decode(Double.self, forKey: .dateModified)) + + let dateCreated: Date? + if let dateCreatedValue = try container.decodeIfPresent( + Double.self, + forKey: .dateCreated + ) { + dateCreated = Date(timeIntervalSince1970: dateCreatedValue) + } else { + dateCreated = nil + } + + let dateModified: Date? + if let dateModifiedValue = try container.decodeIfPresent( + Double.self, + forKey: .dateModified + ) { + dateModified = Date(timeIntervalSince1970: dateModifiedValue) + } else { + dateModified = nil + } + let endDate = Date(timeIntervalSince1970: try container.decode(Double.self, forKey: .endDate)) let googleCalendarEventID = try container.decodeIfPresent(String.self, forKey: .googleCalendarEventID) let orderID = try container.decode(Int64.self, forKey: .orderID) diff --git a/Modules/Sources/Yosemite/Model/Booking/Booking+ReadOnlyConvertible.swift b/Modules/Sources/Yosemite/Model/Booking/Booking+ReadOnlyConvertible.swift index 55a62cb3a6f..db92ed7c8de 100644 --- a/Modules/Sources/Yosemite/Model/Booking/Booking+ReadOnlyConvertible.swift +++ b/Modules/Sources/Yosemite/Model/Booking/Booking+ReadOnlyConvertible.swift @@ -13,8 +13,13 @@ extension Storage.Booking: ReadOnlyConvertible { allDay = booking.allDay cost = booking.cost customerID = booking.customerID - dateCreated = booking.dateCreated - dateModified = booking.dateModified + + /// Falling to back to existing values in case if new values are absent + /// Booking returned when sending a `PUT` request to `bookings/{booking_id}` + /// doesn't contain `date_created` and `date_modified` values. + dateCreated = booking.dateCreated ?? dateCreated + dateModified = booking.dateModified ?? dateModified + endDate = booking.endDate googleCalendarEventID = booking.googleCalendarEventID orderID = booking.orderID diff --git a/Modules/Sources/Yosemite/Stores/BookingStore.swift b/Modules/Sources/Yosemite/Stores/BookingStore.swift index 916087b0a96..ee9454b1eef 100644 --- a/Modules/Sources/Yosemite/Stores/BookingStore.swift +++ b/Modules/Sources/Yosemite/Stores/BookingStore.swift @@ -291,9 +291,39 @@ private extension BookingStore { siteID: siteID, bookingID: bookingID, statusKey: status - ) { _ in - //TODO: - booking status remote update + rollback status in case of error - onCompletion(nil) + ) { [weak self] previousStatusKey in + guard let self else { + return onCompletion(UpdateBookingStatusError.undefinedState) + } + + Task { @MainActor in + do { + if let remoteBooking = try await self.remote.updateBooking( + from: siteID, + bookingID: bookingID, + attendanceStatus: status + ) { + await self.upsertStoredBookingsInBackground( + readOnlyBookings: [remoteBooking], + readOnlyOrders: [], + siteID: siteID + ) + + onCompletion(nil) + } else { + return onCompletion(UpdateBookingStatusError.missingRemoteBooking) + } + } catch { + /// Revert Optimistic Update + self.updateBookingAttendanceStatusLocally( + siteID: siteID, + bookingID: bookingID, + statusKey: previousStatusKey + ) { _ in + onCompletion(error) + } + } + } } } @@ -443,3 +473,11 @@ private extension BookingStore { } } } + + +// MARK: - Errors +// +private enum UpdateBookingStatusError: Error { + case undefinedState + case missingRemoteBooking +} diff --git a/Modules/Tests/NetworkingTests/Remote/BookingsRemoteTests.swift b/Modules/Tests/NetworkingTests/Remote/BookingsRemoteTests.swift index ca2e7946ab2..03e677977fb 100644 --- a/Modules/Tests/NetworkingTests/Remote/BookingsRemoteTests.swift +++ b/Modules/Tests/NetworkingTests/Remote/BookingsRemoteTests.swift @@ -124,6 +124,25 @@ struct BookingsRemoteTests { } } + @Test func test_updateBooking_ignores_nil_dates_in_response() async throws { + // Given + let remote = BookingsRemote(network: network) + let bookingID: Int64 = 206 + network.simulateResponse(requestUrlSuffix: "bookings/\(bookingID)", filename: "booking-no-create-update-dates") + + // When + let booking = try await remote.updateBooking( + from: sampleSiteID, + bookingID: bookingID, + attendanceStatus: .noShow, + ) + + // Then + #expect(booking?.dateCreated == nil) + #expect(booking?.dateModified == nil) + #expect(booking?.id == bookingID) + } + @Test func test_fetchResources_properly_returns_parsed_resources() async throws { // Given let remote = BookingsRemote(network: network) diff --git a/Modules/Tests/NetworkingTests/Responses/booking-no-create-update-dates.json b/Modules/Tests/NetworkingTests/Responses/booking-no-create-update-dates.json new file mode 100644 index 00000000000..8fab8cc0e36 --- /dev/null +++ b/Modules/Tests/NetworkingTests/Responses/booking-no-create-update-dates.json @@ -0,0 +1,22 @@ +{ + "data": { + "id": 206, + "start": 1761238800, + "end": 1761242400, + "all_day": false, + "status": "cancelled", + "attendance_status": "no-show", + "cost": "35.00", + "currency": "USD", + "customer_id": 5, + "product_id": 23, + "resource_id": 19, + "google_calendar_event_id": "0", + "order_id": 205, + "order_item_id": 21, + "parent_id": 0, + "person_counts": [], + "local_timezone": "", + "note": "edited note" + } +} diff --git a/Modules/Tests/YosemiteTests/Mocks/MockBookingsRemote.swift b/Modules/Tests/YosemiteTests/Mocks/MockBookingsRemote.swift index 76ee035d4e7..ca704607dd7 100644 --- a/Modules/Tests/YosemiteTests/Mocks/MockBookingsRemote.swift +++ b/Modules/Tests/YosemiteTests/Mocks/MockBookingsRemote.swift @@ -7,6 +7,7 @@ final class MockBookingsRemote: BookingsRemoteProtocol { private var loadAllBookingsResult: Result<[Booking], Error>? private var loadBookingResult: Result? private var fetchResourceResult: Result? + private var updateBookingResult: Result? private var fetchResourcesResult: Result<[BookingResource], Error>? func whenLoadingAllBookings(thenReturn result: Result<[Booking], Error>) { @@ -17,6 +18,10 @@ final class MockBookingsRemote: BookingsRemoteProtocol { loadBookingResult = result } + func whenUpdatingBooking(thenReturn result: Result) { + updateBookingResult = result + } + func whenFetchingResource(thenReturn result: Result) { fetchResourceResult = result } @@ -53,7 +58,10 @@ final class MockBookingsRemote: BookingsRemoteProtocol { } func updateBooking(from siteID: Int64, bookingID: Int64, attendanceStatus: Networking.BookingAttendanceStatus) async throws -> Networking.Booking? { - return nil + guard let result = updateBookingResult else { + throw NetworkError.timeout() + } + return try result.get() } func fetchResources(for siteID: Int64, pageNumber: Int, pageSize: Int) async throws -> [Networking.BookingResource] { diff --git a/Modules/Tests/YosemiteTests/Stores/BookingStoreTests.swift b/Modules/Tests/YosemiteTests/Stores/BookingStoreTests.swift index 153b911a3e1..7127e1455e1 100644 --- a/Modules/Tests/YosemiteTests/Stores/BookingStoreTests.swift +++ b/Modules/Tests/YosemiteTests/Stores/BookingStoreTests.swift @@ -1,3 +1,4 @@ +import Foundation import Testing @testable import Networking @testable import Storage @@ -624,6 +625,124 @@ struct BookingStoreTests { #expect(orderInfo.statusKey == "processing") } + // MARK: - performUpdateBookingAttendanceStatus + + @Test func performUpdateBookingAttendanceStatus_updates_localBooking() async throws { + // Given + let booking = Booking.fake().copy( + siteID: sampleSiteID, + bookingID: 1, + attendanceStatusKey: BookingAttendanceStatus.booked.rawValue + ) + storeBooking(booking) + + let remoteBooking = booking.copy(attendanceStatusKey: BookingAttendanceStatus.checkedIn.rawValue) + remote.whenUpdatingBooking(thenReturn: .success(remoteBooking)) + let store = BookingStore(dispatcher: Dispatcher(), + storageManager: storageManager, + network: network, + remote: remote, + ordersRemote: ordersRemote) + + // When + let error = await withCheckedContinuation { continuation in + store.onAction( + BookingAction.updateBookingAttendanceStatus( + siteID: sampleSiteID, + bookingID: 1, + status: .checkedIn, + onCompletion: { error in + continuation.resume(returning: error) + } + ) + ) + } + + // Then + #expect(error == nil) + let storedBooking = try #require(viewStorage.loadBooking(siteID: sampleSiteID, bookingID: 1)) + #expect(storedBooking.attendanceStatusKey == BookingAttendanceStatus.checkedIn.rawValue) + } + + @Test func performUpdateBookingAttendanceStatus_keeps_existing_create_and_update_dates() async throws { + // Given + let date = Date(timeIntervalSince1970: 0) + let booking = Booking.fake().copy( + siteID: sampleSiteID, + bookingID: 1, + dateCreated: date, + dateModified: date + ) + storeBooking(booking) + + let remoteBooking = booking.copy( + dateCreated: nil, + dateModified: nil + ) + remote.whenUpdatingBooking(thenReturn: .success(remoteBooking)) + let store = BookingStore(dispatcher: Dispatcher(), + storageManager: storageManager, + network: network, + remote: remote, + ordersRemote: ordersRemote) + + // When + let error = await withCheckedContinuation { continuation in + store.onAction( + BookingAction.updateBookingAttendanceStatus( + siteID: sampleSiteID, + bookingID: 1, + status: .checkedIn, + onCompletion: { error in + continuation.resume(returning: error) + } + ) + ) + } + + // Then + #expect(error == nil) + let storedBooking = try #require(viewStorage.loadBooking(siteID: sampleSiteID, bookingID: 1)) + #expect(storedBooking.dateCreated == date) + #expect(storedBooking.dateModified == date) + } + + @Test func performUpdateBookingAttendanceStatus_reverts_old_status_on_error() async throws { + // Given + let booking = Booking.fake().copy( + siteID: sampleSiteID, + bookingID: 1, + attendanceStatusKey: BookingAttendanceStatus.booked.rawValue + ) + storeBooking(booking) + + remote.whenUpdatingBooking(thenReturn: .failure(NetworkError.timeout())) + let store = BookingStore(dispatcher: Dispatcher(), + storageManager: storageManager, + network: network, + remote: remote, + ordersRemote: ordersRemote) + + // When + let error = await withCheckedContinuation { continuation in + store.onAction( + BookingAction.updateBookingAttendanceStatus( + siteID: sampleSiteID, + bookingID: 1, + status: .checkedIn, + onCompletion: { error in + continuation.resume(returning: error) + } + ) + ) + } + + // Then + #expect(error != nil) + let storedBooking = try #require(viewStorage.loadBooking(siteID: sampleSiteID, bookingID: 1)) + #expect(storedBooking.attendanceStatusKey == BookingAttendanceStatus.booked.rawValue) + } + // MARK: - synchronizeResources @Test func synchronizeResources_returns_false_for_hasNextPage_when_number_of_retrieved_results_is_zero() async throws { diff --git a/WooCommerce/Classes/ViewModels/Booking Details/BookingDetailsViewModel.swift b/WooCommerce/Classes/ViewModels/Booking Details/BookingDetailsViewModel.swift index b373493f6da..c4a71953e57 100644 --- a/WooCommerce/Classes/ViewModels/Booking Details/BookingDetailsViewModel.swift +++ b/WooCommerce/Classes/ViewModels/Booking Details/BookingDetailsViewModel.swift @@ -62,12 +62,6 @@ private extension BookingDetailsViewModel { content: .appointmentDetails(appointmentDetailsContent) ) - let attendanceSection = Section( - header: .title(Localization.attendanceSectionHeaderTitle.uppercased()), - footerText: Localization.attendanceSectionFooterText, - content: .attendance(attendanceContent) - ) - let paymentSection = Section( header: .title(Localization.paymentSectionHeaderTitle.uppercased()), content: .payment(paymentContent) @@ -81,7 +75,6 @@ private extension BookingDetailsViewModel { sections = [ headerSection, appointmentDetailsSection, - attendanceSection, paymentSection, bookingNotes ] @@ -90,36 +83,126 @@ private extension BookingDetailsViewModel { func updateDisplayProperties(from booking: Booking) { navigationTitle = Self.navigationTitle(for: booking) + headerContent.update(with: booking) + + setupCustomerSectionVisibility() if let billingAddress = booking.orderInfo?.customerInfo?.billingAddress, !billingAddress.isEmpty { customerContent.update(with: billingAddress) - insertCustomerSectionIfAbsent() } - headerContent.update(with: booking) + appointmentDetailsContent.update(with: booking, resource: bookingResource) + + setupAttendanceSectionVisibility() attendanceContent.update(with: booking) + paymentContent.update(with: booking) } - func insertCustomerSectionIfAbsent() { - // Avoid adding if it already exists - let customerSectionExists = sections.contains { + func setupCustomerSectionVisibility() { + if let billingAddress = booking.orderInfo?.customerInfo?.billingAddress, !billingAddress.isEmpty { + insertCustomerSectionIfAbsent() + } else { + deleteCustomerSectionIfPresent() + } + } + + func setupAttendanceSectionVisibility() { + if booking.attendanceStatus == .cancelled || booking.bookingStatus == .cancelled { + deleteAttendanceSectionIfPresent() + } else { + insertAttendanceSectionIfAbsent() + } + } + + func insertAttendanceSectionIfAbsent() { + guard let insertAfterIndex = sections.firstIndex(where: { if case .customer = $0.content { return true } - return false + }) ?? sections.firstIndex(where: { + if case .appointmentDetails = $0.content { + return true + } + return false + }) else { + return } - guard !customerSectionExists else { + insertSectionIfAbsent( + section: Section( + header: .title(Localization.attendanceSectionHeaderTitle.uppercased()), + footerText: Localization.attendanceSectionFooterText, + content: .attendance(attendanceContent) + ), + at: insertAfterIndex + 1 + ) + } + + func insertCustomerSectionIfAbsent() { + guard let insertAfterIndex = sections.firstIndex(where: { + if case .appointmentDetails = $0.content { + return true + } + return false + }) else { return } - let customerSection = Section( - header: .title(Localization.customerSectionHeaderTitle.uppercased()), - content: .customer(customerContent) + insertSectionIfAbsent( + section: Section( + header: .title(Localization.customerSectionHeaderTitle.uppercased()), + content: .customer(customerContent) + ), + at: insertAfterIndex + 1 ) + } + + func insertSectionIfAbsent(section: Section, at index: Int) { + let sectionExists = sections.contains { + if section.content.id == $0.content.id { + return true + } + + return false + } + + guard !sectionExists else { + return + } + + withAnimation { + sections.insert(section, at: index) + } + } + + func deleteAttendanceSectionIfPresent() { + guard let attendanceSectionIndex = sections.firstIndex(where: { + if case .attendance = $0.content { + return true + } + return false + }) else { + return + } + + withAnimation { + _ = sections.remove(at: attendanceSectionIndex) + } + } + + func deleteCustomerSectionIfPresent() { + guard let customerSectionIndex = sections.firstIndex(where: { + if case .customer = $0.content { + return true + } + return false + }) else { + return + } + withAnimation { - sections.insert(customerSection, at: 2) + _ = sections.remove(at: customerSectionIndex) } } diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Bookings/BookingDetailsViewModelTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Bookings/BookingDetailsViewModelTests.swift index 58970f3bb84..5766abba1cb 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Bookings/BookingDetailsViewModelTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Bookings/BookingDetailsViewModelTests.swift @@ -298,4 +298,25 @@ final class BookingDetailsViewModelTests: XCTestCase { XCTAssertEqual(attendanceContent.value, "No Show") } + + func test_attendance_section_is_hidden_when_booking_is_cancelled() { + // Given + let booking = Booking.fake().copy( + statusKey: "cancelled", + attendanceStatusKey: "cancelled" + ) + + // When + let viewModel = BookingDetailsViewModel(booking: booking, stores: storesManager) + + // Then + let containsAttendanceSection = viewModel.sections.contains { section in + if case .attendance = section.content { + return true + } + return false + } + + XCTAssertFalse(containsAttendanceSection) + } }