-
Notifications
You must be signed in to change notification settings - Fork 133
[WOOMOB-1516] - Booking attendance status #14778
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Conversation
Generated by 🚫 Danger |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Implements booking attendance status end-to-end: persistence, mapping, network update, UI display, and update-in-progress skeleton states.
- Adds attendanceStatus to BookingEntity with Room converters and DB auto-migration.
- Maps API field attendance_status to domain/UI, exposes updateAttendanceStatus via repository/store/client.
- Updates UI to display attendance status, handle update via bottom sheet, and show SkeletonView while updating.
Reviewed Changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/persistence/entity/BookingEntity.kt | Adds AttendanceStatus sealed type, field on entity, and type converters. |
libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/persistence/WCAndroidDatabase.kt | Bumps DB version and adds auto-migration for new column. |
libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/bookings/BookingsRestClient.kt | Adds endpoint to update attendance status. |
libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/bookings/BookingsStore.kt | Adds flow to call REST client, map DTO, and persist updated booking. |
libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/bookings/BookingDto.kt | Adds attendance_status field to DTO. |
libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/bookings/BookingDtoMapper.kt | Maps attendance_status to entity. |
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/BookingsRepository.kt | Exposes updateAttendanceStatus to app layer. |
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/BookingMapper.kt | Maps entity attendance to UI, threads update-in-progress state. |
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/details/* | Wires selection to repository, tracks AttendanceUpdateStatus, updates state. |
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/compose/* | Renders attendance tag/section with Skeleton during updates; updates enums → sealed types. |
WooCommerce/src/main/res/values/strings.xml | Adds error string for attendance status updates. |
WooCommerce/src/test/* | Adjusts tests for new API and verifies update call. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
BookingAttendanceStatus.CheckedIn -> stringResource(R.string.booking_attendance_status_checked_in) | ||
BookingAttendanceStatus.Cancelled -> stringResource(R.string.booking_attendance_status_cancelled) | ||
BookingAttendanceStatus.NoShow -> stringResource(R.string.booking_attendance_status_no_show) | ||
is BookingAttendanceStatus.Unknown -> key |
Copilot
AI
Oct 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unresolved reference 'key' in the Unknown branch; use the smart-cast receiver. Replace with 'this.key' so the code compiles.
is BookingAttendanceStatus.Unknown -> key | |
is BookingAttendanceStatus.Unknown -> this.key |
Copilot uses AI. Check for mistakes.
when (attendanceUpdateStatus) { | ||
AttendanceUpdateStatus.InProgress -> { | ||
SkeletonView( | ||
modifier = Modifier | ||
.clip(RoundedCornerShape(4.dp)) | ||
.sizeIn( | ||
minHeight = 20.dp, | ||
maxWidth = 80.dp | ||
) | ||
.size(skeletonSize) | ||
) | ||
} | ||
|
||
AttendanceUpdateStatus.Idle -> { | ||
WCTag( | ||
text = state.text(), | ||
backgroundColor = state.backgroundColor(), | ||
textColor = colorResource(R.color.tagView_text), | ||
fontWeight = FontWeight.Normal, | ||
modifier = modifier | ||
.onSizeChanged { | ||
with(density) { | ||
skeletonSize = DpSize(it.width.toDp(), it.height.toDp()) | ||
} | ||
} | ||
) | ||
} | ||
} |
Copilot
AI
Oct 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The skeleton uses size(skeletonSize) without a minimum width; if the first render is InProgress, skeletonSize is 0.dp and nothing is shown. Add a minWidth fallback (e.g., via sizeIn(minWidth = 40.dp)) or use a default size when skeletonSize == DpSize.Zero. Also, the passed 'modifier' isn't applied to the skeleton path, which causes layout inconsistency during loading; apply 'modifier' to the outer container or skeleton as well.
Copilot uses AI. Check for mistakes.
when (attendanceUpdateStatus) { | ||
AttendanceUpdateStatus.InProgress -> { | ||
SkeletonView( | ||
modifier = Modifier | ||
.size(skeletonSize) | ||
) | ||
} | ||
|
||
AttendanceUpdateStatus.Idle -> { | ||
Row( | ||
verticalAlignment = Alignment.CenterVertically, | ||
modifier = Modifier | ||
.onSizeChanged { | ||
with(density) { | ||
skeletonSize = DpSize(it.width.toDp(), it.height.toDp()) | ||
} | ||
} | ||
) { | ||
Text( | ||
text = value, | ||
style = MaterialTheme.typography.labelLarge, |
Copilot
AI
Oct 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Skeleton uses size(skeletonSize) with no minimums; on first render in InProgress state, it will render at 0×0. Add a fallback size (e.g., sizeIn(minWidth = 80.dp, minHeight = 20.dp)) or conditionally use a default when skeletonSize == DpSize.Zero so loading is visible.
Copilot uses AI. Check for mistakes.
companion object Companion { | ||
fun fromKey(key: String): AttendanceStatus { | ||
return when (key) { | ||
Booked.key -> Booked | ||
NoShow.key -> NoShow | ||
CheckedIn.key -> CheckedIn | ||
else -> Unknown(key) | ||
} | ||
} | ||
} |
Copilot
AI
Oct 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Naming the companion object explicitly 'Companion' is redundant and unusual. Prefer 'companion object' without a name for clarity.
Copilot uses AI. Check for mistakes.
val orderResult = bookingDto.orderId.takeIf { it != 0L }?.let { | ||
orderStore.fetchSingleOrderSync(site, bookingDto.orderId) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this is needed - asked here p1760707694187709/1760706907.922799-slack-CNA89KY6M
val parentId: Long, | ||
val personCounts: List<Long>?, | ||
val localTimezone: String, | ||
@ColumnInfo(defaultValue = "") val attendanceStatus: AttendanceStatus, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this, as a default okay? All new bookings start with booked
so we could do the same 🤔
BookingEntity.AttendanceStatus.Booked -> BookingAttendanceStatus.Booked | ||
BookingEntity.AttendanceStatus.CheckedIn -> BookingAttendanceStatus.CheckedIn | ||
BookingEntity.AttendanceStatus.NoShow -> BookingAttendanceStatus.NoShow |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As you can see, there's no Cancelled
here - not sure yet how we will determine such attendance status.
📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
|
📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## trunk #14778 +/- ##
============================================
- Coverage 38.08% 38.05% -0.04%
- Complexity 9998 10001 +3
============================================
Files 2119 2119
Lines 119170 119321 +151
Branches 16228 16273 +45
============================================
+ Hits 45385 45402 +17
- Misses 69206 69330 +124
- Partials 4579 4589 +10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
WOOMOB-1516
Description
This PR implements the
attendance_status
we get from the API. This value is now displayed and also update after selecting an option from the bottom sheet.For the loading progress indicator, I've decided to use the SkeletonView (see the recording below).
Only the bookings created after installing the latest plugin will have the status set. The old one will have an empty string. I didn't handle this in any way as this should be an issue for us during the development phase. But if you think we should, let me know.
Steps to reproduce
booked
status shownTesting information
Test network connection issues and response errors.
The tests that have been performed
The above
Images/gif
Screen_recording_20251017_151920.mp4
RELEASE-NOTES.txt
if necessary. Use the "[Internal]" label for non-user-facing changes.