Skip to content

Conversation

@AdamGrzybkowski
Copy link
Contributor

@AdamGrzybkowski AdamGrzybkowski commented Sep 16, 2025

Closes WOOMOB-1341

Description

This is a basic skeleton Fragment with a compose screen and a ViewModel for the Booking Details screen.
I've looked into the existing patterns to avoid introducing new ones.

For now, I've added an extra row to the "Developer options" to open the Booking details screen with a fixed ID. Not sure if that's the best place, but this seemed like a good fit to make sure it's only available in debug builds.
I guess I can move it to the Booking List screen once it's merged to the trunk. Thoughts?

Testing information

  1. Go to Settings
  2. Developer options
  3. Tap "Bookind details screen"

The tests that have been performed

Images/gif

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

@AdamGrzybkowski AdamGrzybkowski added this to the 23.3 milestone Sep 16, 2025
@AdamGrzybkowski AdamGrzybkowski changed the title Issue/woomob 1341 booking details fragment [WOOMOB-1341] Booking details fragment skeleton Sep 16, 2025
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 a skeleton booking details screen feature as part of WOOMOB-1341. It creates the basic structure for displaying booking details with a configurable toolbar title.

  • Adds new booking details UI components including Fragment, Screen, ViewModel, and ViewState
  • Implements navigation to the booking details screen from developer options with hardcoded booking ID
  • Adds localized string resource for booking details title formatting

Reviewed Changes

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

Show a summary per file
File Description
BookingDetailsViewModelTest.kt Unit test for ViewModel toolbar title formatting logic
strings.xml Adds localized string resource for booking details title
nav_graph_settings.xml Navigation setup for booking details fragment with bookingId argument
DeveloperOptionsViewModel.kt Adds booking details navigation option to developer menu
DeveloperOptionsFragment.kt Implements navigation action to booking details with hardcoded ID
BookingDetailsViewState.kt Data class defining the view state structure
BookingDetailsViewModel.kt ViewModel handling toolbar title formatting from navigation args
BookingDetailsScreen.kt Composable UI implementation with toolbar and empty content
BookingDetailsFragment.kt Fragment wrapper for the Compose screen
build.gradle Adds lifecycle runtime compose dependency

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

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Sep 16, 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
Commitc9b6987
Direct Downloadwoocommerce-wear-prototype-build-pr14620-c9b6987.apk

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Sep 16, 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
Commitc9b6987
Direct Downloadwoocommerce-prototype-build-pr14620-c9b6987.apk

@codecov-commenter
Copy link

codecov-commenter commented Sep 16, 2025

Codecov Report

❌ Patch coverage is 88.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.51%. Comparing base (dee76d3) to head (c9b6987).
⚠️ Report is 14 commits behind head on trunk.

Files with missing lines Patch % Lines
...id/ui/prefs/developer/DeveloperOptionsViewModel.kt 72.72% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##              trunk   #14620      +/-   ##
============================================
+ Coverage     38.50%   38.51%   +0.01%     
- Complexity     9774     9779       +5     
============================================
  Files          2066     2068       +2     
  Lines        115514   115538      +24     
  Branches      15390    15390              
============================================
+ Hits          44476    44497      +21     
- Misses        66906    66909       +3     
  Partials       4132     4132              

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

@AdamGrzybkowski AdamGrzybkowski force-pushed the issue/WOOMOB-1341_booking_details_fragment branch from 0092062 to 30e8917 Compare September 17, 2025 07:25
@AdamGrzybkowski AdamGrzybkowski marked this pull request as ready for review September 17, 2025 07:36
@JorgeMucientes JorgeMucientes self-assigned this Sep 17, 2025
@AdamGrzybkowski AdamGrzybkowski modified the milestones: 23.3, 23.4 Sep 17, 2025
Comment on lines 25 to 28
return ComposeView(requireContext()).apply {
setViewCompositionStrategy(ViewCompositionStrategy.DisposeOnViewTreeLifecycleDestroyed)
setContent {
WooThemeWithBackground {
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor np. We have a composeView() extension that provides these code for you.

PS: I've noticed in my prior PR when adding BookingListFragment that I was guilty of not using this extension either 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ohh, thanks! I will update it!

implementation(libs.androidx.fragment.ktx)
implementation(libs.androidx.lifecycle.viewmodel.savedstate)
implementation(libs.androidx.lifecycle.process)
implementation(libs.androidx.lifecycle.runtime.compose)
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor np. I'm not sure this dependency is needed. It is already part of androidx.lifecycle:lifecycle-viewmodel-compose.
Is there a reason why you added this specifically?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was added to get access to .collectAsStateWithLifecycle(), but since we won't be using it, I will remove it!

@wpmobilebot
Copy link
Collaborator

Project manifest changes for WooCommerce

The following changes in the WooCommerce's merged AndroidManifest.xml file were detected (build variant: vanillaRelease):

--- ./build/reports/diff_manifest/WooCommerce/vanillaRelease/base_manifest.txt	2025-09-22 09:36:05.369677713 +0000
+++ ./build/reports/diff_manifest/WooCommerce/vanillaRelease/head_manifest.txt	2025-09-22 09:36:08.499700251 +0000
@@ -622,17 +622,6 @@
             android:exported="false"
             android:theme="@android:style/Theme.Translucent.NoTitleBar" />
 
-        <provider
-            android:name="zendesk.belvedere.BelvedereFileProvider"
-            android:authorities="com.woocommerce.android.belvedere2.attachments"
-            android:exported="false"
-            android:grantUriPermissions="true"
-            android:readPermission="true" >
-            <meta-data
-                android:name="android.support.FILE_PROVIDER_PATHS"
-                android:resource="@xml/belvedere_attachment_storage_v2" />
-        </provider>
-
         <service
             android:name="androidx.work.impl.background.systemalarm.SystemAlarmService"
             android:directBootAware="false"
@@ -771,6 +760,16 @@
             android:required="false" />
 
         <provider
+            android:name="zendesk.belvedere.BelvedereFileProvider"
+            android:authorities="com.woocommerce.android.belvedere2.attachments"
+            android:exported="false"
+            android:grantUriPermissions="true"
+            android:readPermission="true" >
+            <meta-data
+                android:name="android.support.FILE_PROVIDER_PATHS"
+                android:resource="@xml/belvedere_attachment_storage_v2" />
+        </provider>
+        <provider
             android:name="com.google.firebase.provider.FirebaseInitProvider"
             android:authorities="com.woocommerce.android.firebaseinitprovider"
             android:directBootAware="true"

Go to https://buildkite.com/automattic/woocommerce-android/builds/32240/canvas?sid=019970c4-14f4-4b9a-a0dd-da99e747c014, click on the Artifacts tab and audit the files.

@AdamGrzybkowski
Copy link
Contributor Author

@JorgeMucientes I applied the changes.

Any thoghts on this:

For now, I've added an extra row to the "Developer options" to open the Booking details screen with a fixed ID. Not sure if that's the best place, but this seemed like a good fit to make sure it's only available in debug builds.
I guess I can move it to the Booking List screen once it's merged to the trunk. Thoughts?

Copy link
Contributor

@JorgeMucientes JorgeMucientes left a comment

Choose a reason for hiding this comment

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

Thanks for addressing the changes @AdamGrzybkowski. Everything LGTM 👏🏼

Any thoghts on this:
For now, I've added an extra row to the "Developer options" to open the Booking details screen with a fixed ID. Not sure if that's the best place, but this seemed like a good fit to make sure it's only available in debug builds.
I guess I can move it to the Booking List screen once it's merged to the trunk. Thoughts?

About that, I think it's fine the approach you followed as it will take a few days to have the booking list ready, so its a good place to provide an access point to booking detail in the meantime. Good idea!

@AdamGrzybkowski AdamGrzybkowski merged commit f526bad into trunk Sep 22, 2025
15 checks passed
@AdamGrzybkowski AdamGrzybkowski deleted the issue/WOOMOB-1341_booking_details_fragment branch September 22, 2025 11:52
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.

6 participants