[CIAB] A minor refactoring to avoid leaking the booking status to UI layer #14667

hichamboushaba · 2025-09-30T15:55:43Z

Description

As per the previous discussion, this PR introduces a model in the DB layer that maps to the API status keys, which avoids leaking this information to the UI layer.
Please feel free to share any thoughts or suggestions for improvement here.

Testing information

Code review should be enough, but small test to confirm there is no regression would be fine.

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.

hichamboushaba · 2025-09-30T15:57:59Z

...fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/persistence/entity/BookingEntity.kt

+            override val key = "complete"
+        }
+
+        data class Unknown(override val key: String) : Status


I know I mentioned I don't like having an Unknown value, but after thinking more about this and knowing how flexible WordPress is, I decided to go for the safest approach here.
We can revisit when next-admin implements logic for displaying badges, and see if we need to modify something to align, WDYT?

Sounds good 👍

hichamboushaba · 2025-09-30T15:58:54Z

...fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/persistence/entity/BookingEntity.kt

    val localTimezone: String
-)
+) {
+    sealed interface Status {


I named this Status and not PaymentStatus as this seems related more to the booking status and not just payment (for example cancelled is not really a payment status).

I wonder if we should rename the UI model too or not, WDYT?

At the time, I thought this was the order payment status, but it looks like it's not so yeah the name change makes sense.

It's still a bit confusing to me, because we can have two cancelled status tags next to each other (attendance and this one) 🤔

It's still a bit confusing to me, because we can have two cancelled status tags next to each other (attendance and this one) 🤔

As discussed yesterday in the call, the cancelled status for the attendance is just a presentation status, it happens only when the booking itself is also cancelled.

but it looks like it's not so yeah the name change makes sense.

Sounds good, I'll rename the UI model too.

hichamboushaba · 2025-09-30T15:59:54Z

@AdamGrzybkowski while working on this, I noticed AttendanceStatusTag doesn't have the Booking prefix, should we add it for the sake of consistency?

wpmobilebot · 2025-09-30T16:07:14Z

📲 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	`41ee907`
	Direct Download	`woocommerce-wear-prototype-build-pr14667-41ee907.apk`

wpmobilebot · 2025-09-30T16:20:42Z

📲 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	`41ee907`
	Direct Download	`woocommerce-prototype-build-pr14667-41ee907.apk`

AdamGrzybkowski

Thanks for the changes!

AdamGrzybkowski · 2025-10-01T08:21:30Z

...fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/persistence/entity/BookingEntity.kt

    val localTimezone: String
-)
+) {
+    sealed interface Status {


At the time, I thought this was the order payment status, but it looks like it's not so yeah the name change makes sense.

It's still a bit confusing to me, because we can have two cancelled status tags next to each other (attendance and this one) 🤔

AdamGrzybkowski · 2025-10-01T08:22:31Z

...fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/persistence/entity/BookingEntity.kt

+            override val key = "complete"
+        }
+
+        data class Unknown(override val key: String) : Status


Sounds good 👍

AdamGrzybkowski · 2025-10-01T08:26:11Z

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/compose/PaymentStatusTag.kt

+        BookingPaymentStatus.Paid -> stringResource(R.string.booking_payment_status_paid)
+        BookingPaymentStatus.Cancelled -> stringResource(R.string.booking_payment_status_cancelled)
+        BookingPaymentStatus.Complete -> stringResource(R.string.booking_payment_status_complete)
+        is BookingPaymentStatus.Unknown -> key


Not sure if there's a better alternative, but just leaving a comment that this caught my attention. Displaying the raw values to the user could be risky, but it would be very handy at spotting the issue for us. One alternative I can think of would be to hide the badge completely.

Displaying the raw values to the user could be risky

Not necessarly risky, for example for orders, WordPress has plugins that adds custom statuses, and for those we display the raw keys when there is no label, as this is what wp-admin does, and that's what the users are used too.

So I said, I think we need to revisit this when next-admin adds the logic for the badges.

AdamGrzybkowski · 2025-10-01T08:27:56Z

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/list/BookingListViewState.kt

    )
 }
+
+private fun BookingEntity.Status.toUiModel(): BookingPaymentStatus = when (this) {


You don't have to do this now, but we will need the same mappings in the details screen, so we could start putting those in a separate file, so they are available for reuse.

Yes, this is something we need, a proper mapper class would be useful for reusability.

AdamGrzybkowski · 2025-10-01T08:40:27Z

@AdamGrzybkowski while working on this, I noticed AttendanceStatusTag doesn't have the Booking prefix, should we add it for the sake of consistency?

Makes sense, I guess. Feel free to rename in this PR, or we can do it later.

dangermattic · 2025-10-01T08:53:30Z

	1 Warning
⚠️	This PR is assigned to the milestone 23.4. 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

codecov-commenter · 2025-10-01T10:07:58Z

Codecov Report

❌ Patch coverage is 14.73684% with 81 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.42%. Comparing base (0154fe1) to head (41ee907).
⚠️ Report is 31 commits behind head on trunk.

Files with missing lines	Patch %	Lines
...ce/android/ui/bookings/compose/BookingStatusTag.kt	0.00%	33 Missing ⚠️
.../android/fluxc/persistence/entity/BookingEntity.kt	33.33%	12 Missing ⚠️
...ings/compose/BookingAttendanceStatusBottomSheet.kt	0.00%	11 Missing ⚠️
.../ui/bookings/compose/BookingAttendanceStatusTag.kt	0.00%	9 Missing ⚠️
...erce/android/ui/bookings/compose/BookingSummary.kt	22.22%	7 Missing ⚠️
...e/android/ui/bookings/list/BookingListViewState.kt	36.36%	2 Missing and 5 partials ⚠️
...id/ui/bookings/compose/BookingAttendanceSection.kt	0.00%	1 Missing ⚠️
...xc/network/rest/wpcom/wc/bookings/BookingsStore.kt	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##              trunk   #14667      +/-   ##
============================================
- Coverage     38.42%   38.42%   -0.01%     
  Complexity     9818     9818              
============================================
  Files          2089     2089              
  Lines        116528   116546      +18     
  Branches      15575    15592      +17     
============================================
- Hits          44778    44777       -1     
- Misses        67597    67612      +15     
- Partials       4153     4157       +4

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

hichamboushaba added 3 commits September 30, 2025 16:48

Introduce a DB model for the booking status

acf455f

Update UI layer to align with the new BookingEntity.Status model

2079d0d

Rename file

6ffb8dd

hichamboushaba added type: task An internally driven task. feature: CIAB Mobile Bookings MVP labels Sep 30, 2025

hichamboushaba commented Sep 30, 2025

View reviewed changes

hichamboushaba requested a review from AdamGrzybkowski September 30, 2025 15:59

hichamboushaba marked this pull request as ready for review September 30, 2025 15:59

hichamboushaba added this to the 23.4 milestone Sep 30, 2025

AdamGrzybkowski self-assigned this Oct 1, 2025

AdamGrzybkowski approved these changes Oct 1, 2025

View reviewed changes

Rename BookingPaymentStatus to BookingStatus

b1ff9de

Rename AttendanceStatus to BookingAttendanceStatus

fdd2d59

hichamboushaba force-pushed the ciab-bookings-payment-status-refactor branch from 62ea539 to fdd2d59 Compare October 1, 2025 08:53

hichamboushaba enabled auto-merge October 1, 2025 08:54

Fix unit tests

41ee907

hichamboushaba merged commit d666a23 into trunk Oct 1, 2025
15 checks passed

hichamboushaba deleted the ciab-bookings-payment-status-refactor branch October 1, 2025 10:02

[CIAB] A minor refactoring to avoid leaking the booking status to UI layer #14667

[CIAB] A minor refactoring to avoid leaking the booking status to UI layer #14667

Uh oh!

Conversation

hichamboushaba commented Sep 30, 2025

Description

Testing information

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!

hichamboushaba commented Sep 30, 2025

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

AdamGrzybkowski left a comment

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!

AdamGrzybkowski Oct 1, 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!

AdamGrzybkowski commented Oct 1, 2025

Uh oh!

dangermattic commented Oct 1, 2025

Uh oh!

Uh oh!

codecov-commenter commented Oct 1, 2025

Codecov Report

Uh oh!

Uh oh!

wpmobilebot commented Sep 30, 2025 •

edited

Loading

wpmobilebot commented Sep 30, 2025 •

edited

Loading

AdamGrzybkowski Oct 1, 2025 •

edited

Loading