Skip to content

Conversation

@AdamGrzybkowski
Copy link
Contributor

WOOMOB-1453

Description

This PR implements the refresh logic on the Booking details screen.
It follows the Booking List logic - so first the booking is retrieved and then the corresponding order.

Additionally, I've added a basic error handling to show a snack when the call fails or when there's no network conneciton.

Steps to reproduce

  1. Monitor network calls via Android Studio or any other tool you like
  2. Launch the app
  3. Go to bookings tab
  4. Tap on the booking
  5. Confirm two endpoints were made to fetch the booking and the order

Testing information

Repeat the above with disabled network.
Repeat the above with network but this time mock the API responses to fail.

The tests that have been performed

The above.

Images/gif

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

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements refresh functionality for the Booking details screen by adding API calls to fetch individual bookings and their associated orders. It follows the same pattern as the Booking List logic and includes basic error handling for network failures.

Key changes:

  • Added fetchBooking API endpoint and store method to retrieve individual bookings
  • Implemented refresh logic in BookingDetailsViewModel with network status checking
  • Added error handling with snackbar notifications for offline and API failure scenarios

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
libs/fluxc-processor/src/main/resources/wc-wp-api-endpoints.txt Added individual booking endpoint pattern
libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/bookings/BookingsStore.kt Implemented fetchBooking method with order fetching logic
libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/bookings/BookingsRestClient.kt Added REST client method for individual booking fetch
WooCommerce/src/main/res/values/strings.xml Added error message string for booking fetch failures
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/details/BookingDetailsViewState.kt Added loading state management for refresh functionality
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/details/BookingDetailsViewModel.kt Implemented refresh logic with network checking and error handling
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/details/BookingDetailsFragment.kt Added event handling for snackbar notifications
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/BookingsRepository.kt Added repository method for booking fetch with error handling
WooCommerce/src/test/kotlin/com/woocommerce/android/ui/bookings/details/BookingDetailsViewModelTest.kt Added comprehensive test coverage for refresh functionality

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

<string name="booking_payment_label_service">Service</string>
<string name="booking_payment_mark_as_paid">Mark as paid</string>
<string name="booking_payment_view_order">View order</string>
<string name="booking_fetch_error">Error fetching booking</string>
Copy link

Copilot AI Oct 14, 2025

Choose a reason for hiding this comment

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

Corrected string name from 'booking_fetch_error' to 'bookings_fetch_error' to match usage in code.

Suggested change
<string name="booking_fetch_error">Error fetching booking</string>
<string name="bookings_fetch_error">Error fetching booking</string>

Copilot uses AI. Check for mistakes.
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Oct 14, 2025

📲 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 NameWooCommerce-Wear Android
Platform⌚️ Wear OS
FlavorJalapeno
Build TypeDebug
Commite2ba38a
Direct Downloadwoocommerce-wear-prototype-build-pr14750-e2ba38a.apk

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Oct 14, 2025

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

App NameWooCommerce Android
Platform📱 Mobile
FlavorJalapeno
Build TypeDebug
Commite2ba38a
Direct Downloadwoocommerce-prototype-build-pr14750-e2ba38a.apk

@codecov-commenter
Copy link

codecov-commenter commented Oct 14, 2025

Codecov Report

❌ Patch coverage is 43.07692% with 37 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.36%. Comparing base (a4105ff) to head (e2ba38a).
⚠️ Report is 36 commits behind head on trunk.

Files with missing lines Patch % Lines
...xc/network/rest/wpcom/wc/bookings/BookingsStore.kt 0.00% 18 Missing ⚠️
...twork/rest/wpcom/wc/bookings/BookingsRestClient.kt 0.00% 10 Missing ⚠️
...commerce/android/ui/bookings/BookingsRepository.kt 0.00% 7 Missing ⚠️
...oid/ui/bookings/details/BookingDetailsViewModel.kt 96.15% 0 Missing and 1 partial ⚠️
...oid/ui/bookings/details/BookingDetailsViewState.kt 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##              trunk   #14750   +/-   ##
=========================================
  Coverage     38.36%   38.36%           
- Complexity    10004    10006    +2     
=========================================
  Files          2115     2115           
  Lines        118014   118074   +60     
  Branches      15758    15766    +8     
=========================================
+ Hits          45280    45304   +24     
- Misses        68535    68570   +35     
- Partials       4199     4200    +1     

☔ 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 hichamboushaba self-assigned this Oct 14, 2025
Copy link
Member

@hichamboushaba hichamboushaba left a comment

Choose a reason for hiding this comment

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

Great work @AdamGrzybkowski, I left one comment about the endpoint we use for fetching orders.

And a remark here, I think adding support for pull-to-refresh would be beneficial here, as it would match the experience we have in order details, and also in case of failures, it will allow the user to retry, WDYT? This can be added in a separate PR.

response.result != null -> {
val dto = response.result
val orderIds = listOf(dto.orderId).filterNot { it == 0L }
val ordersResult = fetchOrders(site, orderIds)
Copy link
Member

@hichamboushaba hichamboushaba Oct 14, 2025

Choose a reason for hiding this comment

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

This will use /wc/v3/orders?include=<orderId>, normally we should use /wc/v3/orders/<orderId> here, as there may be some performance benefits in using the single item endpoint.
To do this, we can use the function WCOrderStore#fetchSingleOrderSync, this returns the fetched order, and handles the caching too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call, done in 5d68d50

Comment on lines 27 to 29
val endpoint = WCWPAPIEndpoint(
WOOCOMMERCE.bookings.id(bookingId).endpoint
).pathV2Bookings
Copy link
Member

Choose a reason for hiding this comment

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

I missed this in my review.

Suggested change
val endpoint = WCWPAPIEndpoint(
WOOCOMMERCE.bookings.id(bookingId).endpoint
).pathV2Bookings
val endpoint = WOOCOMMERCE.bookings.id(bookingId).pathV2Bookings

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 5d68d50

@AdamGrzybkowski
Copy link
Contributor Author

AdamGrzybkowski commented Oct 14, 2025

@hichamboushaba Agree about the PTR - done here 45ff617

Copy link
Member

@hichamboushaba hichamboushaba left a comment

Choose a reason for hiding this comment

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

Thanks @AdamGrzybkowski for all the changes, nice work 👏

@hichamboushaba hichamboushaba merged commit 277fac2 into trunk Oct 14, 2025
16 checks passed
@hichamboushaba hichamboushaba deleted the issue/WOOMOB-1453_booking_details_refresh branch October 14, 2025 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants