Skip to content

Conversation

@JorgeMucientes
Copy link
Contributor

@JorgeMucientes JorgeMucientes commented Sep 22, 2025

Description

This PR adds a new rest client to fetch Bookings when opening the new Bookings tabs.

Testing information

  1. Log into a CIAB site.
  2. Manually install the bookings plugin from this ZIP file: p1758531103469849/1758101141.913349-slack-C03L1NF1EA3
  3. You'll need to enable bookings v2 on your CIAB testing site. You can do that by installing the Code Snippets plugin to your site and adding the following PHP snippet: define( 'WC_BOOKINGS_NEXT_ENABLED', true );
  4. Place at least one appointment/booking for your site.
  5. Open the Bookings tab and inspect network requests. Check the following request is fired successfully: /wp-json/wc-bookings/v2/bookings/?per_page=25&page=1
[
  {
    "id": 80,
    "start": 1759417200,
    "end": 1759420800,
    "all_day": false,
    "status": "unpaid",
    "cost": "30.00",
    "currency": "USD",
    "customer_id": 0,
    "product_id": 23,
    "resource_id": 22,
    "date_created": 1758531652,
    "date_modified": 1758531652,
    "google_calendar_event_id": "0",
    "order_id": 79,
    "order_item_id": 3,
    "parent_id": 0,
    "person_counts": [],
    "local_timezone": "",
    "_links": { ...
  1. Inspect the DB content and ensure fetched bookings are saved. Check wc-android-database -> Bookings

The tests that have been performed

The above ☝🏼

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

@dangermattic
Copy link
Collaborator

dangermattic commented Sep 22, 2025

4 Warnings
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
⚠️ Class BookingListViewModel is missing tests, but unit-tests-exemption label was set to ignore this.
⚠️ Class BookingsRestClient is missing tests, but unit-tests-exemption label was set to ignore this.
⚠️ Class BookingsStore is missing tests, but unit-tests-exemption label was set to ignore this.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Sep 22, 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 Name WooCommerce-Wear Android
Platform⌚️ Wear OS
FlavorJalapeno
Build TypeDebug
Commitf748b5d
Direct Downloadwoocommerce-wear-prototype-build-pr14645-f748b5d.apk

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Sep 22, 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 Name WooCommerce Android
Platform📱 Mobile
FlavorJalapeno
Build TypeDebug
Commitf748b5d
Direct Downloadwoocommerce-prototype-build-pr14645-f748b5d.apk

@JorgeMucientes JorgeMucientes marked this pull request as ready for review September 23, 2025 15:56
"database": {
"version": 65,
"identityHash": "2d180f4d4c2ccaf6305a746959d1289d",
"identityHash": "2f88094bb00e637769cc5a8bdef5dcc3",
Copy link
Member

Choose a reason for hiding this comment

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

@JorgeMucientes I believe you accidentally commited a change to the schema file of the previous DB version, can you please revert it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great catch @hichamboushaba reverted changes applied to the 65.json schema.

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.

Nice work @JorgeMucientes, code looks good, and works as expected, but there is an issue with the modified schema of DB version 65.

@codecov-commenter
Copy link

codecov-commenter commented Sep 23, 2025

Codecov Report

❌ Patch coverage is 0% with 123 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.48%. Comparing base (2c17432) to head (f748b5d).
⚠️ Report is 15 commits behind head on trunk.

Files with missing lines Patch % Lines
...xc/network/rest/wpcom/wc/bookings/BookingsStore.kt 0.00% 39 Missing ⚠️
...mmerce/android/ui/bookings/BookingListViewModel.kt 0.00% 31 Missing ⚠️
.../android/fluxc/persistence/entity/BookingEntity.kt 0.00% 20 Missing ⚠️
...fluxc/network/rest/wpcom/wc/bookings/BookingDto.kt 0.00% 19 Missing ⚠️
...twork/rest/wpcom/wc/bookings/BookingsRestClient.kt 0.00% 14 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##              trunk   #14645      +/-   ##
============================================
- Coverage     38.52%   38.48%   -0.04%     
  Complexity     9783     9783              
============================================
  Files          2068     2072       +4     
  Lines        115558   115676     +118     
  Branches      15394    15400       +6     
============================================
+ Hits          44522    44523       +1     
- Misses        66899    67016     +117     
  Partials       4137     4137              

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

@JorgeMucientes JorgeMucientes force-pushed the issue/woomob-1357-booking-list-fetch-booking-from-api branch 2 times, most recently from 15d09ef to ec944dd Compare September 24, 2025 09:20
@AdamGrzybkowski
Copy link
Contributor

FYI: I'm getting a crash when updating an already installed app (trunk) with the version from this branch. Fresh install works fine.

FATAL EXCEPTION: main
    Process: com.woocommerce.android.dev, PID: 16126
    java.lang.IllegalStateException: Migration didn't properly handle: Bookings(org.wordpress.android.fluxc.persistence.entity.BookingEntity).

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.

Looks good now, thanks @JorgeMucientes.

FYI: I'm getting a crash when updating an already installed app (trunk) with the version from this branch. Fresh install works fine.

@AdamGrzybkowski I think you tested before the last commit that reverted the changes to the schema of version 65, after the revert, the DB migration works without issues for me.

Copy link
Contributor

@AdamGrzybkowski AdamGrzybkowski left a comment

Choose a reason for hiding this comment

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

@hichamboushaba You're right, I thought I pulled the latest changes...

@JorgeMucientes All good, sorry for the false alert!

@JorgeMucientes JorgeMucientes merged commit 4d98698 into trunk Sep 24, 2025
16 checks passed
@JorgeMucientes JorgeMucientes deleted the issue/woomob-1357-booking-list-fetch-booking-from-api branch September 24, 2025 10:43
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.

7 participants