Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package com.woocommerce.android.ui.bookings

import com.woocommerce.android.ui.bookings.compose.BookingAppointmentDetailsModel
import com.woocommerce.android.ui.bookings.compose.BookingAttendanceStatus
import com.woocommerce.android.ui.bookings.compose.BookingStatus
import com.woocommerce.android.ui.bookings.compose.BookingSummaryModel
import com.woocommerce.android.ui.bookings.list.BookingListItem
import com.woocommerce.android.util.CurrencyFormatter
import org.wordpress.android.fluxc.persistence.entity.BookingEntity
import java.time.Duration
import java.time.ZoneOffset
import java.time.format.DateTimeFormatter
import java.time.format.FormatStyle
import javax.inject.Inject

class BookingMapper @Inject constructor(
private val currencyFormatter: CurrencyFormatter,
) {
private val summaryDateFormatter: DateTimeFormatter = DateTimeFormatter.ofLocalizedDateTime(
FormatStyle.MEDIUM,
FormatStyle.SHORT
).withZone(ZoneOffset.UTC)
Comment on lines +17 to +22
Copy link
Contributor Author

Choose a reason for hiding this comment

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

cc @hichamboushaba I've used the UTC based on your discussion.


private val detailsDateFormatter: DateTimeFormatter = DateTimeFormatter.ofPattern("EEEE, dd MMM yyyy")
Copy link
Member

Choose a reason for hiding this comment

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

IMO we should avoid hardcording formatting pattens, I'd use a localized format instead. I know it's used in the app is some spots, but IMO it's not something we should follow.

Suggested change
private val detailsDateFormatter: DateTimeFormatter = DateTimeFormatter.ofPattern("EEEE, dd MMM yyyy")
private val detailsDateFormatter: DateTimeFormatter = DateTimeFormatter.ofLocalizedDate(FormatStyle.FULL)

The above will update the text from: Monday, 06 Oct 2025 to Monday, 6 October 2025 for UK english set as the locale.

Another approach if we are sure we want to keep the shorter month name is to use something like:

    private val detailsDateFormatter: DateTimeFormatter = DateTimeFormatterBuilder()
        .appendPattern("EEEE, ")
        .append(DateTimeFormatter.ofLocalizedDate(FormatStyle.MEDIUM))
        .toFormatter()
        .withZone(ZoneOffset.UTC)

But personally I think using FormatStyle.FULL is better in this context, WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, I did focus on the format I saw in the Figma, but this will work better across different locales. Thanks!

.withZone(ZoneOffset.UTC)
private val timeRangeFormatter: DateTimeFormatter = DateTimeFormatter.ofLocalizedTime(FormatStyle.SHORT)
.withZone(ZoneOffset.UTC)

fun Booking.toBookingSummaryModel(): BookingSummaryModel {
return BookingSummaryModel(
date = summaryDateFormatter.format(start),
// TODO replace mocked values when product and customer data are available
name = "Women’s Haircut",
customerName = "Margarita Nikolaevna",
attendanceStatus = BookingAttendanceStatus.BOOKED,
status = status.toUiModel()
)
}

fun Booking.toUiModel(): BookingListItem {
Copy link
Member

Choose a reason for hiding this comment

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

np, just to make the role a bit clearer, I'd rename this to toListItem(), WDYT?

return BookingListItem(
id = id.value,
summary = toBookingSummaryModel()
)
}

fun Booking.toAppointmentDetailsModel(): BookingAppointmentDetailsModel {
val durationMinutes = Duration.between(start, end).toMinutes()
return BookingAppointmentDetailsModel(
date = detailsDateFormatter.format(start),
time = "${timeRangeFormatter.format(start)} - ${timeRangeFormatter.format(end)}",
// TODO replace mocked values when available from API
staff = "Marianne Renoir",
location = "238 Willow Creek Drive, Montgomery AL 36109",
duration = "$durationMinutes min",
price = currencyFormatter.formatCurrency(cost, currency)
)
}

private fun BookingEntity.Status.toUiModel(): BookingStatus = when (this) {
BookingEntity.Status.Paid -> BookingStatus.Paid
BookingEntity.Status.PendingConfirmation -> BookingStatus.PendingConfirmation
BookingEntity.Status.Cancelled -> BookingStatus.Cancelled
BookingEntity.Status.Complete -> BookingStatus.Complete
BookingEntity.Status.Confirmed -> BookingStatus.Confirmed
BookingEntity.Status.Unpaid -> BookingStatus.Unpaid
is BookingEntity.Status.Unknown -> BookingStatus.Unknown(this.key)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ class BookingsRepository @Inject constructor(
limit = limit,
filters = filters
)

fun observeBooking(bookingId: Long): Flow<Booking?> =
bookingsStore.observeBooking(
site = selectedSite.get(),
bookingId = bookingId
)
}

typealias Booking = BookingEntity
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import com.woocommerce.android.ui.bookings.compose.BookingAttendanceSection
import com.woocommerce.android.ui.bookings.compose.BookingAttendanceStatus
import com.woocommerce.android.ui.bookings.compose.BookingAttendanceStatusBottomSheet
import com.woocommerce.android.ui.bookings.compose.BookingCustomerDetails
import com.woocommerce.android.ui.bookings.compose.BookingCustomerDetailsModel
import com.woocommerce.android.ui.bookings.compose.BookingPaymentDetailsModel
import com.woocommerce.android.ui.bookings.compose.BookingPaymentSection
import com.woocommerce.android.ui.bookings.compose.BookingStatus
import com.woocommerce.android.ui.bookings.compose.BookingSummary
Expand Down Expand Up @@ -76,44 +78,46 @@ fun BookingDetailsScreen(
.verticalScroll(rememberScrollState())
.padding(innerPadding)
) {
BookingSummary(
model = viewState.bookingSummary,
modifier = Modifier.fillMaxWidth()
)
BookingAppointmentDetails(
model = viewState.bookingsAppointmentDetails,
onCancelBooking = viewState.onCancelBooking,
modifier = Modifier.fillMaxWidth()
)
BookingCustomerDetails(
model = viewState.bookingCustomerDetails,
onEmailClick = {},
onPhoneClick = {},
modifier = Modifier.fillMaxWidth()
)
BookingAttendanceSection(
status = viewState.bookingSummary.attendanceStatus,
onClick = { showAttendanceSheet.value = true },
modifier = Modifier.fillMaxWidth()
)
BookingPaymentSection(
model = viewState.bookingPaymentDetails,
status = viewState.bookingSummary.status,
onMarkAsPaid = { onViewOrder(viewState.orderId) },
onViewOrder = { onViewOrder(viewState.orderId) },
onMarkAsRefunded = { onViewOrder(viewState.orderId) },
modifier = Modifier.fillMaxWidth()
)
}
if (showAttendanceSheet.value) {
BookingAttendanceStatusBottomSheet(
onSelect = { status ->
viewState.onAttendanceStatusSelected(status)
},
onDismiss = { showAttendanceSheet.value = false }
)
viewState.bookingUiState?.let {
BookingSummary(
model = viewState.bookingUiState.bookingSummary,
modifier = Modifier.fillMaxWidth()
)
BookingAppointmentDetails(
model = viewState.bookingUiState.bookingsAppointmentDetails,
onCancelBooking = viewState.onCancelBooking,
modifier = Modifier.fillMaxWidth()
)
BookingCustomerDetails(
model = viewState.bookingUiState.bookingCustomerDetails,
onEmailClick = {},
onPhoneClick = {},
modifier = Modifier.fillMaxWidth()
)
BookingAttendanceSection(
status = viewState.bookingUiState.bookingSummary.attendanceStatus,
onClick = { showAttendanceSheet.value = true },
modifier = Modifier.fillMaxWidth()
)
BookingPaymentSection(
model = viewState.bookingUiState.bookingPaymentDetails,
status = viewState.bookingUiState.bookingSummary.status,
onMarkAsPaid = { onViewOrder(viewState.orderId) },
onViewOrder = { onViewOrder(viewState.orderId) },
onMarkAsRefunded = { onViewOrder(viewState.orderId) },
modifier = Modifier.fillMaxWidth()
)
}
}
}
if (showAttendanceSheet.value) {
BookingAttendanceStatusBottomSheet(
onSelect = { status ->
viewState.onAttendanceStatusSelected(status)
},
onDismiss = { showAttendanceSheet.value = false }
)
}
}
}

Expand All @@ -124,21 +128,39 @@ private fun BookingDetailsPreview() {
BookingDetailsScreen(
viewState = BookingDetailsViewState(
toolbarTitle = "Booking #12345",
bookingSummary = BookingSummaryModel(
date = "05/07/2025, 11:00 AM",
name = "Women’s Haircut",
customerName = "Margarita Nikolaevna",
attendanceStatus = BookingAttendanceStatus.CHECKED_IN,
status = BookingStatus.Paid
bookingUiState = BookingUiState(
bookingSummary = BookingSummaryModel(
date = "05/07/2025, 11:00 AM",
name = "Women’s Haircut",
customerName = "Margarita Nikolaevna",
attendanceStatus = BookingAttendanceStatus.CHECKED_IN,
status = BookingStatus.Paid
),
bookingsAppointmentDetails = BookingAppointmentDetailsModel(
date = "Monday, 05 July 2025",
time = "11:00 am - 12:00 pm",
staff = "Marianne Renoir",
location = "238 Willow Creek Drive, Montgomery AL 36109",
duration = "60 min",
price = "$55.00"
),
bookingCustomerDetails = BookingCustomerDetailsModel(
name = "Margarita Nikolaevna",
email = "[email protected]",
phone = "+1 555-123-4567",
billingAddressLines = listOf(
"238 Willow Creek Drive",
"Montgomery AL 36109",
"United States"
)
),
bookingPaymentDetails = BookingPaymentDetailsModel(
service = "$55.00",
tax = "$4.50",
discount = "-",
total = "$59.50"
)
),
bookingsAppointmentDetails = BookingAppointmentDetailsModel(
date = "Monday, 05 July 2025",
time = "11:00 am - 12:00 pm",
staff = "Marianne Renoir",
location = "238 Willow Creek Drive, Montgomery AL 36109",
duration = "60 min",
price = "$55.00"
)
),
onBack = {},
onViewOrder = {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,28 @@ import androidx.lifecycle.LiveData
import androidx.lifecycle.SavedStateHandle
import androidx.lifecycle.asLiveData
import com.woocommerce.android.R
import com.woocommerce.android.ui.bookings.Booking
import com.woocommerce.android.ui.bookings.BookingMapper
import com.woocommerce.android.ui.bookings.BookingsRepository
import com.woocommerce.android.ui.bookings.compose.BookingAttendanceStatus
import com.woocommerce.android.ui.bookings.compose.BookingCustomerDetailsModel
import com.woocommerce.android.ui.bookings.compose.BookingPaymentDetailsModel
import com.woocommerce.android.viewmodel.ResourceProvider
import com.woocommerce.android.viewmodel.ScopedViewModel
import com.woocommerce.android.viewmodel.navArgs
import dagger.hilt.android.lifecycle.HiltViewModel
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.flow.update
import javax.inject.Inject

@HiltViewModel
class BookingDetailsViewModel @Inject constructor(
savedState: SavedStateHandle,
resourceProvider: ResourceProvider,
private val bookingsRepository: BookingsRepository,
private val bookingMapper: BookingMapper,
) : ScopedViewModel(savedState) {

private val navArgs: BookingDetailsFragmentArgs by savedState.navArgs()
Expand All @@ -35,17 +44,59 @@ class BookingDetailsViewModel @Inject constructor(
toolbarTitle = resourceProvider.getString(R.string.booking_details_title, navArgs.bookingId),
)
}
observeBooking(navArgs.bookingId)
Copy link
Member

Choose a reason for hiding this comment

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

Before approving this, I just want to discuss one problem with this approach and decide whether we are OK with it or not.
With this approach, we observe the DB regardless of whether the app is in the foreground or not (this is part of what we discussed before on whether we should expose flows or LiveData).

I think a better approach (IMO 🙂) would be to have a cold flow exposed as LiveData, so the cold flow won't start until the screen reaches the Started lifecycle state, and to achieve it, we can think of something like the following patch:

Index: WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/details/BookingDetailsViewModel.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/details/BookingDetailsViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/details/BookingDetailsViewModel.kt
--- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/details/BookingDetailsViewModel.kt	(revision 509794d7b5731f41a1b4ea3847de4599bcf8a3f5)
+++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/details/BookingDetailsViewModel.kt	(date 1759502430662)
@@ -4,7 +4,6 @@
 import androidx.lifecycle.SavedStateHandle
 import androidx.lifecycle.asLiveData
 import com.woocommerce.android.R
-import com.woocommerce.android.ui.bookings.Booking
 import com.woocommerce.android.ui.bookings.BookingMapper
 import com.woocommerce.android.ui.bookings.BookingsRepository
 import com.woocommerce.android.ui.bookings.compose.BookingAttendanceStatus
@@ -15,69 +14,41 @@
 import com.woocommerce.android.viewmodel.navArgs
 import dagger.hilt.android.lifecycle.HiltViewModel
 import kotlinx.coroutines.flow.MutableStateFlow
-import kotlinx.coroutines.flow.launchIn
-import kotlinx.coroutines.flow.onEach
-import kotlinx.coroutines.flow.update
+import kotlinx.coroutines.flow.combine
+import kotlinx.coroutines.flow.filterNotNull
 import javax.inject.Inject
 
 @HiltViewModel
 class BookingDetailsViewModel @Inject constructor(
     savedState: SavedStateHandle,
     resourceProvider: ResourceProvider,
-    private val bookingsRepository: BookingsRepository,
+    bookingsRepository: BookingsRepository,
     private val bookingMapper: BookingMapper,
 ) : ScopedViewModel(savedState) {
 
     private val navArgs: BookingDetailsFragmentArgs by savedState.navArgs()
 
-    private val _state = MutableStateFlow(
-        BookingDetailsViewState(
-            onCancelBooking = ::onCancelBooking,
-            onAttendanceStatusSelected = ::onAttendanceStatusSelected,
-        )
-    )
-    val state: LiveData<BookingDetailsViewState> = _state.asLiveData()
-
-    init {
-        _state.update {
-            it.copy(
-                toolbarTitle = resourceProvider.getString(R.string.booking_details_title, navArgs.bookingId),
-            )
-        }
-        observeBooking(navArgs.bookingId)
-    }
-
-    private fun onAttendanceStatusSelected(status: BookingAttendanceStatus) {
-        val bookingState = _state.value.bookingUiState
-        if (bookingState != null) {
-            _state.update { current ->
-                current.copy(
-                    bookingUiState = bookingState.copy(
-                        bookingSummary = bookingState.bookingSummary.copy(attendanceStatus = status)
-                    )
-                )
-            }
-        }
-    }
-
-    private fun onCancelBooking() {
-        // TODO Add logic to Cancel booking
-    }
-
-    private fun observeBooking(bookingId: Long) {
-        bookingsRepository.observeBooking(bookingId)
-            .onEach { booking ->
-                booking?.let { updateStateWithBooking(it) }
-            }
-            .launchIn(this)
-    }
-
-    private fun updateStateWithBooking(booking: Booking) = with(bookingMapper) {
-        _state.update { current ->
-            current.copy(
+    private val booking = bookingsRepository.observeBooking(navArgs.bookingId)
+
+    // If we need this, then using `savedState.getStateFlow` would be better here to preserve state across process death,
+    private val bookingAttendanceStatus = MutableStateFlow<BookingAttendanceStatus?>(null)
+
+    val state: LiveData<BookingDetailsViewState> = combine(
+        booking.filterNotNull(),
+        bookingAttendanceStatus
+    ) { booking, attendanceStatus ->
+        with(bookingMapper) {
+            BookingDetailsViewState(
+                toolbarTitle = resourceProvider.getString(R.string.booking_details_title, booking.id),
                 orderId = booking.orderId,
                 bookingUiState = BookingUiState(
-                    bookingSummary = booking.toBookingSummaryModel(),
+                    bookingSummary = booking.toBookingSummaryModel().let {
+                        if (attendanceStatus != null) {
+                            it.copy(attendanceStatus = attendanceStatus)
+                        } else {
+                            it
+                        }
+                    },
                     bookingsAppointmentDetails = booking.toAppointmentDetailsModel(),
                     bookingCustomerDetails = BookingCustomerDetailsModel(
                         name = "Margarita Nikolaevna",
@@ -96,7 +67,24 @@
                         total = "$59.50"
                     )
                 ),
+                onCancelBooking = ::onCancelBooking,
+                onAttendanceStatusSelected = ::onAttendanceStatusSelected
             )
         }
+    }.asLiveData()
+
+    private fun onAttendanceStatusSelected(status: BookingAttendanceStatus) {
+        // Whether we'll need the `bookingAttendanceStatus` property or not depending
+        // on whether we'll have a "save" button for the whole screen or we'll update the status
+        // as soon as the user selects it from the bottom sheet.
+        // Personally I think we should update it immediately, and if we do, then we don't need the property
+        // we just need to manage the loading state, and when the API call is done, the booking will be updated accordingly
+        // via the `booking` flow.
+        // But this is a product decision. And this patch just preserves the current behavior.
+        bookingAttendanceStatus.value = status
+    }
+
+    private fun onCancelBooking() {
+        // TODO Add logic to Cancel booking
     }
 }

The patch also makes the whole screen reactive, so if during the implementation we find that we need the Order (which most probably will happen), then we can chain the Flows to observe also the Order from the DB, and then the DB will be the single source of truth, and drives the screen.

Please check it out, and share your thoughts. While I think observing the DB when the app is in the background should be avoided, if you don't agree with the above approach, I'm OK with accepting the PR as it's.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please check it out, and share your thoughts. While I think observing the DB when the app is in the background should be avoided, if you don't agree with the above approach, I'm OK with accepting the PR as it's.

Thanks @hichamboushaba! You're obviously correct. I think I was working on an autopilot, not being used to LiveData 🤦

I will update the PR on Monday!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in b168757

}

private fun onAttendanceStatusSelected(status: BookingAttendanceStatus) {
_state.update { current ->
current.copy(
bookingSummary = current.bookingSummary.copy(attendanceStatus = status)
)
val bookingState = _state.value.bookingUiState
if (bookingState != null) {
_state.update { current ->
current.copy(
bookingUiState = bookingState.copy(
bookingSummary = bookingState.bookingSummary.copy(attendanceStatus = status)
)
)
}
}
}

private fun onCancelBooking() {
// TODO Add logic to Cancel booking
}

private fun observeBooking(bookingId: Long) {
bookingsRepository.observeBooking(bookingId)
.onEach { booking ->
booking?.let { updateStateWithBooking(it) }
}
.launchIn(this)
}

private fun updateStateWithBooking(booking: Booking) = with(bookingMapper) {
_state.update { current ->
current.copy(
orderId = booking.orderId,
bookingUiState = BookingUiState(
bookingSummary = booking.toBookingSummaryModel(),
bookingsAppointmentDetails = booking.toAppointmentDetailsModel(),
bookingCustomerDetails = BookingCustomerDetailsModel(
name = "Margarita Nikolaevna",
email = "[email protected]",
phone = "+1 555-123-4567",
billingAddressLines = listOf(
"238 Willow Creek Drive",
"Montgomery AL 36109",
"United States"
)
),
bookingPaymentDetails = BookingPaymentDetailsModel(
service = "$55.00",
tax = "$4.50",
discount = "-",
total = "$59.50"
)
),
)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,43 +4,19 @@ import com.woocommerce.android.ui.bookings.compose.BookingAppointmentDetailsMode
import com.woocommerce.android.ui.bookings.compose.BookingAttendanceStatus
import com.woocommerce.android.ui.bookings.compose.BookingCustomerDetailsModel
import com.woocommerce.android.ui.bookings.compose.BookingPaymentDetailsModel
import com.woocommerce.android.ui.bookings.compose.BookingStatus
import com.woocommerce.android.ui.bookings.compose.BookingSummaryModel

data class BookingDetailsViewState(
val toolbarTitle: String = "",
val orderId: Long = 0L,
val bookingSummary: BookingSummaryModel = BookingSummaryModel(
date = "05/07/2025, 11:00 AM",
name = "Women’s Haircut",
customerName = "Margarita Nikolaevna",
attendanceStatus = BookingAttendanceStatus.NO_SHOW,
status = BookingStatus.Paid
),
val bookingsAppointmentDetails: BookingAppointmentDetailsModel = BookingAppointmentDetailsModel(
date = "Monday, 05 July 2025",
time = "11:00 am - 12:00 pm",
staff = "Marianne Renoir",
location = "238 Willow Creek Drive, Montgomery AL 36109",
duration = "60 min",
price = "$55.00"
),
val bookingCustomerDetails: BookingCustomerDetailsModel = BookingCustomerDetailsModel(
name = "Margarita Nikolaevna",
email = "[email protected]",
phone = "+1 555-123-4567",
billingAddressLines = listOf(
"238 Willow Creek Drive",
"Montgomery AL 36109",
"United States"
)
),
val bookingPaymentDetails: BookingPaymentDetailsModel = BookingPaymentDetailsModel(
service = "$55.00",
tax = "$4.50",
discount = "-",
total = "$59.50"
),
val bookingUiState: BookingUiState? = null,
val onCancelBooking: () -> Unit = {},
val onAttendanceStatusSelected: (BookingAttendanceStatus) -> Unit = { _ -> }
)

data class BookingUiState(
val bookingSummary: BookingSummaryModel,
val bookingsAppointmentDetails: BookingAppointmentDetailsModel,
val bookingCustomerDetails: BookingCustomerDetailsModel,
val bookingPaymentDetails: BookingPaymentDetailsModel,
)
Comment on lines +17 to +22
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All those models would be nullable after this PR, so it's handy to wrap them in a single class to avoid multiple null checks.

Loading