Issue/woomob 1351 add logic to show or hide the booking tab dynamically #14640

JorgeMucientes · 2025-09-19T16:08:51Z

Closes: WOOMOB-1351

Description

This is a follow-up PR to handle the visibility of the Booking tab dynamically based on the following criteria:

Site is CIAB
Site has at least 1 bookable product published
Feature flag is enabled locally

I've added a Flow to keep any changes around those 3 conditions reactive.

Testing information

To make tests quicker I recommed logging into the app with a WP.com account that has at least access to:

A WP.com/Jetpack connected site non CIAB
A CIAB site with Bookings plugin installed

Log into a CIAB site with Bookings plugin enabled. Have at least one Bookable product created. You can create such sites from matticspace tools/garden/index.php
Check the new bookings tab is shown.
Edit the bookable product and make it a draft. Notice how the bookings tab is gone
Publish the bookable product again and see how tab is visible again
Delete the product completely and check tab is gone.
Change to a non CIAB site with bookable products published and check the Bookings tab is still gone.

The tests that have been performed

The above ☝🏼

Images/gif

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

wpmobilebot · 2025-09-19T16:16:31Z

📲 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	`34a3bb1`
	Direct Download	`woocommerce-wear-prototype-build-pr14640-34a3bb1.apk`

wpmobilebot · 2025-09-19T16:33:33Z

📲 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	`34a3bb1`
	Direct Download	`woocommerce-prototype-build-pr14640-34a3bb1.apk`

JorgeMucientes · 2025-09-19T16:13:23Z

WooCommerce/src/main/kotlin/com/woocommerce/android/ciab/CIABExtensions.kt

+import com.woocommerce.android.ciab.CIABSiteGateKeeper.Companion.CIAB_GARDEN_NAME
+import org.wordpress.android.fluxc.model.SiteModel
+
+fun SiteModel.isCIABSite() = isGardenSite && gardenName == CIAB_GARDEN_NAME


Unsure this is the best place for such extension. Open to suggestions

We already have SiteModelExt for similar extensions, WDYT?

JorgeMucientes · 2025-09-19T16:45:57Z

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/tab/BookingsTabController.kt

+            selectedSite.observe()
+                .filter { it != null }
+                .collect { siteModel ->


We start observing bookable products only when we know for sure a site has been selected. Otherwise the app will crash. For example when loggin into the app for the first time, MainActivity is resumed before the app has finished selecting a site, leading to a crash.

codecov-commenter · 2025-09-19T17:29:51Z

Codecov Report

❌ Patch coverage is 68.57143% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.50%. Comparing base (8e165c0) to head (34a3bb1).
⚠️ Report is 145 commits behind head on trunk.

Files with missing lines	Patch %	Lines
...id/ui/bookings/tab/ObserveBookingsTabVisibility.kt	79.31%	2 Missing and 4 partials ⚠️
...e/android/ui/bookings/tab/BookingsTabController.kt	0.00%	4 Missing ⚠️
...com/woocommerce/android/ciab/CIABSiteGateKeeper.kt	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##              trunk   #14640      +/-   ##
============================================
+ Coverage     38.49%   38.50%   +0.01%     
- Complexity     9767     9771       +4     
============================================
  Files          2064     2064              
  Lines        115429   115448      +19     
  Branches      15370    15372       +2     
============================================
+ Hits          44430    44459      +29     
+ Misses        66883    66872      -11     
- Partials       4116     4117       +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

Nice work @JorgeMucientes, the logic is working as expected, but I think we can make some improvements to fix some issues that I shared below, please check the comments.

hichamboushaba · 2025-09-22T10:13:56Z

WooCommerce/src/main/kotlin/com/woocommerce/android/ciab/CIABExtensions.kt

+import com.woocommerce.android.ciab.CIABSiteGateKeeper.Companion.CIAB_GARDEN_NAME
+import org.wordpress.android.fluxc.model.SiteModel
+
+fun SiteModel.isCIABSite() = isGardenSite && gardenName == CIAB_GARDEN_NAME


We already have SiteModelExt for similar extensions, WDYT?

hichamboushaba · 2025-09-22T10:18:11Z

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/tab/BookingsTabController.kt

-                .onFailure {
-                    // TODO log error or track errors?
+            selectedSite.observe()
+                .filter { it != null }


I suggest using filterNotNull here, it will automatically cast the variable and allow you to get rid of the double-bang operator below.

hichamboushaba · 2025-09-22T11:51:05Z

...erce/src/main/kotlin/com/woocommerce/android/ui/bookings/tab/ObserveBookingsTabVisibility.kt

+    }
+        .map { productsCount ->
+            productsCount > 0 &&
+                siteModel?.isCIABSite() == true &&


siteModel is non-nullable here

Suggested change

siteModel?.isCIABSite() == true &&

siteModel.isCIABSite() == true &&

hichamboushaba · 2025-09-22T12:00:04Z

...erce/src/main/kotlin/com/woocommerce/android/ui/bookings/tab/ObserveBookingsTabVisibility.kt

+        ),
+        excludeSampleProducts = true
+    ).onStart {
+        productListRepository.fetchProductList(


With this logic, we'll make an API request to fetch bookable products for all sites, WDYT about changing the order of things slightly to make sure we return an early false when the site is not CIAB, soemthing like the following:

operator fun invoke(siteModel: SiteModel): Flow<Boolean> = flow { val isCIABSite = FeatureFlag.BOOKINGS_MVP.isEnabled() && siteModel.isCIABSite() if (!isCIABSite) { emit(false) } else { emitAll( productListRepository.observeProductsCount( filterOptions = mapOf( ProductFilterOption.STATUS to ProductStatus.PUBLISH.value, ProductFilterOption.TYPE to BOOKING_PRODUCT_TYPE ), excludeSampleProducts = true ).map { count -> count > 0 }.onStart { productListRepository.fetchProductList( productFilterOptions = mapOf(ProductFilterOption.TYPE to BOOKING_PRODUCT_TYPE) ).onFailure { WooLog.w(WooLog.T.BOOKINGS, "Failed to fetch bookable products") } }.distinctUntilChanged() ) } }

Another point related to this, with the fetch happening in onStart, we don't emit any value until it's done, and this results in showing the bookings tab before hiding it when switching to any site that doesn't have the requirements:

Screen_recording_20250922_142723.mp4

This issue affects both your version and the above suggested version (for CIAB sites that don't have bookable products), we can avoid this by making the fetch async:

.onStart { appCoroutineScope.launch { productListRepository.fetchProductList( productFilterOptions = mapOf(ProductFilterOption.TYPE to BOOKING_PRODUCT_TYPE) ).onFailure { WooLog.w(WooLog.T.BOOKINGS, "Failed to fetch bookable products") } } }

hichamboushaba · 2025-09-22T13:22:13Z

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/tab/BookingsTabController.kt

        owner.lifecycle.removeObserver(this)
    }

    private fun checkBookingsTabVisibility() {


Not directly related to this PR, but just sharing an observation here, I think that given we use activity.lifecycleScope.launch in this function, it's safe to call it directly from init without making this class a LifecycleObserver, lifecycleScope already takes care of this, and won't start the Coroutine until the Activity is started.

We just need to ensure BookingsTabController subscribes to ObserveBookingsTabVisibility when the app starts, and unsubscribes when destroyed.activity.lifecycleScope takes care of this already so no need to make the class lifecycle aware.

AdamGrzybkowski · 2025-09-23T08:23:33Z

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/tab/BookingsTabController.kt

-    private val showBookingsTab: ShowBookingsTab
-) : DefaultLifecycleObserver {
+    private val observeBookingsTabVisibility: ObserveBookingsTabVisibility,
+    private val selectedSite: SelectedSite


❓ Is there a reason, we can't inject the selectedSite in ObserveBookingsTabVisibility and keep all the logic there?

I didn't gave much though to it tbh. Any advantages I'm not seeing from having everything in ObserveBookingsTabVisibility?
I feel the current implementation is readable/clean enough, but I'm open to move the selected site to ObserveBookingsTabVisibility.

Two things I see:

Keeping the whole tab visibility logic in one place.

ObserveBookingsTabVisibilityTest - the BookingsTabController doesn't have tests, and because of Android dependencies, it's a bit trickier to add them. Keeping everything in ObserveBookingsTabVisibility makes it possible to test the whole flow, including the suspending issue mentioned with the previous code.

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/tab/BookingsTabController.kt

JorgeMucientes · 2025-09-23T13:26:26Z

Excellent feedback @hichamboushaba @AdamGrzybkowski thank you. Great eye for detail! I've addressed all your suggested feedback. Ready for another round :)

hichamboushaba

Nice work @JorgeMucientes, thanks for taking care of this.

I won't merge this to give you a chance to check Adam's last remark about moving SelectedSite to ObserveBookingsTabVisibility.

AdamGrzybkowski

Tested and looks good 👍

The other comment is non-blocking so approving!

JorgeMucientes · 2025-09-24T14:44:47Z

Thanks for all the feedback folks. I've moved the logic around observing selected site to ObserveBookingsTabVisibility and kept all the logic there as suggested. The provided reasons where good enough to apply the refactor.

JorgeMucientes added 9 commits September 18, 2025 18:07

Add logs to error case when failing to check Bookings tab visibility

c38a541

Add new extension function to check site is CIAB

c89ba2d

Check if current site is CIAB before displaying bookings tab

84ff7b1

Refactor extension function to apply only to SiteModel

f99c836

Link Bookings tab visibility to Bookable published bookable products

838ea0f

Add logs when refreshing bookable products fails

aed415a

Rename class

ccf36cd

Add tests

cab6f52

Add unit tests

f3b0973

JorgeMucientes added this to the 23.4 milestone Sep 19, 2025

JorgeMucientes added feature: CIAB Mobile Bookings MVP New Feature labels Sep 19, 2025

JorgeMucientes added 2 commits September 19, 2025 18:35

Ensure site is selected before observing bookings count

802ffd6

Fix unit tests after refactor

a120eed

JorgeMucientes commented Sep 19, 2025

View reviewed changes

JorgeMucientes requested review from AdamGrzybkowski and hichamboushaba September 19, 2025 16:46

JorgeMucientes marked this pull request as ready for review September 19, 2025 16:46

hichamboushaba self-assigned this Sep 22, 2025

hichamboushaba reviewed Sep 22, 2025

View reviewed changes

JorgeMucientes added 7 commits September 22, 2025 18:12

Move new is isCIABSite to the correct file

eb13d9e

Use filterNotNull function

6c84c24

Remove unneeded nullable expression and simplified boolean condition

5610a84

Avoid fetching bookings over the network if site is not CIAB

2ad102a

Make onStart logic async to avoid delaying the emission of first event

41a3fc6

Fix unit tests

9e03aae

AdamGrzybkowski reviewed Sep 23, 2025

View reviewed changes

Remove nesting flow collection to avoid potential issues

f05fba9

JorgeMucientes requested review from AdamGrzybkowski and hichamboushaba September 23, 2025 13:26

Fix detekt issue

7a5b2c0

hichamboushaba approved these changes Sep 23, 2025

View reviewed changes

AdamGrzybkowski approved these changes Sep 24, 2025

View reviewed changes

JorgeMucientes added 2 commits September 24, 2025 16:23

Moves logic to observe site changes to ObserveBookingsTabVisibility

e1227bc

Fix unit tests

81a856c

JorgeMucientes enabled auto-merge September 24, 2025 14:44

Fix detekt indentation issue

34a3bb1

JorgeMucientes merged commit add243f into trunk Sep 24, 2025
15 checks passed

JorgeMucientes deleted the issue/woomob-1351-add-logic-to-show-or-hide-the-booking-tab-dynamically branch September 24, 2025 15:27

	siteModel?.isCIABSite() == true &&
	siteModel.isCIABSite() == true &&

Issue/woomob 1351 add logic to show or hide the booking tab dynamically #14640

Issue/woomob 1351 add logic to show or hide the booking tab dynamically #14640

Uh oh!

Conversation

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

Uh oh!

Description

Testing information

The tests that have been performed

Images/gif

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

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!

codecov-commenter commented Sep 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

hichamboushaba 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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

hichamboushaba left a comment

Choose a reason for hiding this comment

Uh oh!

AdamGrzybkowski left a comment

Choose a reason for hiding this comment

Uh oh!

JorgeMucientes commented Sep 24, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

JorgeMucientes commented Sep 19, 2025 •

edited

Loading

wpmobilebot commented Sep 19, 2025 •

edited

Loading

wpmobilebot commented Sep 19, 2025 •

edited

Loading

codecov-commenter commented Sep 19, 2025 •

edited

Loading

JorgeMucientes commented Sep 23, 2025 •

edited

Loading