Skip to content

Bookings: Show bookings tab when site is eligible #16153

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.

Sign up for GitHub

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

Merged
merged 10 commits into from
Sep 25, 2025
Merged

Bookings: Show bookings tab when site is eligible #16153

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
077d33f
Show bookings tab when site is eligible
itsmeichigo Sep 19, 2025
9e81f47
Remove unnecessary caching
itsmeichigo Sep 19, 2025
6f6c479
Add unit tests
itsmeichigo Sep 19, 2025
f460ecd
Create bookingsEligibilityChecker upon observing
itsmeichigo Sep 19, 2025
2115397
Ignore periphery
itsmeichigo Sep 19, 2025
349779f
Merge branch 'WOOMOB-1363-hide-POS-tab-for-CIAB-sites' into woomob-12…
itsmeichigo Sep 22, 2025
ecfc2b3
Disable optional tabs when site ID change
itsmeichigo Sep 23, 2025
76c5b0d
Update site ID for POSTabCoordinator upon switching stores
itsmeichigo Sep 24, 2025
35c4988
Ignore unused method to be updated later
itsmeichigo Sep 24, 2025
7198db0
Merge branch 'trunk' into woomob-1231-ios-bookings-tab
itsmeichigo Sep 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions WooCommerce/Classes/Bookings/BookingsTabView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ final class BookingsTabViewHostingController: UIHostingController<BookingsTabVie
override var shouldShowOfflineBanner: Bool {
return true
}

func didSwitchStore(id: Int64) {
// TODO: update view
}
}

private extension BookingsTabViewHostingController {
Expand Down
49 changes: 46 additions & 3 deletions WooCommerce/Classes/POS/TabBar/POSTabCoordinator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ final class POSTabViewController: UIViewController {
/// Coordinator for the Point of Sale tab.
///
final class POSTabCoordinator {
private let siteID: Int64
private(set) var siteID: Int64
private let tabContainerController: TabContainerController
private let viewControllerToPresent: UIViewController
private let storesManager: StoresManager
Expand Down Expand Up @@ -107,12 +107,55 @@ final class POSTabCoordinator {
}

func onTabSelected() {
presentPOSView()
presentPOSView(siteID: siteID)
}

func didSwitchStore(id: Int64) {
self.siteID = id

// Resets lazy properties so they get recreated with new siteID
posItemFetchStrategyFactory = PointOfSaleItemFetchStrategyFactory(
siteID: siteID,
credentials: credentials,
selectedSite: defaultSitePublisher,
appPasswordSupportState: isAppPasswordSupported
)

posPopularItemFetchStrategyFactory =
PointOfSaleFixedItemFetchStrategyFactory(
fixedStrategy: posItemFetchStrategyFactory.popularStrategy()
)

posCouponFetchStrategyFactory = PointOfSaleCouponFetchStrategyFactory(
siteID: siteID,
currencySettings: currencySettings,
credentials: credentials,
selectedSite: defaultSitePublisher,
appPasswordSupportState: isAppPasswordSupported,
storage: storageManager
)

posCouponProvider = PointOfSaleCouponService(
siteID: siteID,
currencySettings: currencySettings,
credentials: credentials,
selectedSite: defaultSitePublisher,
appPasswordSupportState: isAppPasswordSupported,
storage: storageManager
)

barcodeScanService = PointOfSaleBarcodeScanService(
siteID: siteID,
credentials: credentials,
selectedSite: defaultSitePublisher,
appPasswordSupportState: isAppPasswordSupported,
currencySettings: currencySettings
)
}
}

private extension POSTabCoordinator {
func presentPOSView() {
func presentPOSView(siteID: Int64) {
Task { @MainActor [weak self] in
guard let self else { return }
let serviceAdaptor = POSServiceLocatorAdaptor()
Expand Down
14 changes: 9 additions & 5 deletions WooCommerce/Classes/ViewRelated/MainTabBarController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -842,11 +842,15 @@ private extension MainTabBarController {
}
hubMenuTabCoordinator?.activate(siteID: siteID)

// Set dashboard to be the default tab - disable optional tabs by default.
selectedIndex = WooTab.myStore.visibleIndex(isPOSTabVisible: false,
isBookingsTabVisible: false)
updateTabViewControllers(isPOSTabVisible: false,
isBookingsTabVisible: false)
// Sets dashboard to be the default tab.
selectedIndex = WooTab.myStore.visibleIndex(isPOSTabVisible: isPOSTabVisible,
isBookingsTabVisible: isBookingsTabVisible)

// Updates site ID for the POS coordinator to ensure correct data
posTabCoordinator?.didSwitchStore(id: siteID)
Copy link
Contributor

@jaclync jaclync Sep 25, 2025

Choose a reason for hiding this comment

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

I'm hoping the siteID can remain constant in POSTabCoordinator. How about reinitializing the POSTabCoordinator here instead of updating the siteID? Would it cause any issues?

Copy link
Contributor Author

@itsmeichigo itsmeichigo Sep 25, 2025

Choose a reason for hiding this comment

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

@jaclync POSTabCoordinator requires the eligibility checker, which now requires a full site model to check if the site is CIAB. Upon changes of site ID, we don't yet have the full site model so it's not possible to reinitialize the coordinator.

Copy link
Contributor Author

@itsmeichigo itsmeichigo Sep 25, 2025

Choose a reason for hiding this comment

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

It would be great if the coordinator can be separated from the eligibility checker, but with the current design I don't have a better solution. Please feel free to suggest a solution if you have one in mind, thanks!

Copy link
Contributor

@jaclync jaclync Sep 25, 2025

Choose a reason for hiding this comment

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

I'd consider separating the tab visibility logic in POSEntryPointEligibilityCheckerProtocol to a new protocol that stays in the app, so that POSEntryPointEligibilityCheckerProtocol just has checkEligibility and refreshEligibility that should not depend on the full Site (the CIAB check is performed on the tab visibility).

We're also in the middle of moving POS code into a separate module, and I'm refactoring the dependencies of PointOfSaleEntryPointView passed in POSTabCoordinator. Please go ahead with merging this PR and I'll look into the refactoring as part of the POS module refactoring work.

Copy link
Contributor Author

@itsmeichigo itsmeichigo Sep 25, 2025

Choose a reason for hiding this comment

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

Thank you!


// Updates site ID for the bookings tab to display correct bookings
(bookingsContainerController.wrappedController as? BookingsTabViewHostingController)?.didSwitchStore(id: siteID)
}

func createDashboardViewController(siteID: Int64) -> UIViewController {
Expand Down
2 changes: 1 addition & 1 deletion WooCommerce/Classes/ViewRelated/NotificationsBadgeController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ final class NotificationsBadgeController {
///
private func hideDotOn(with input: NotificationsBadgeInput) {
let tag = dotTag(for: input.tab)
if let subviews = input.tabBar.orderedTabBarActionableViews[input.tabIndex].subviews.first?.subviews {
if let subviews = input.tabBar.orderedTabBarActionableViews[safe: input.tabIndex]?.subviews.first?.subviews {
Copy link
Contributor Author

@itsmeichigo itsmeichigo Sep 24, 2025

Choose a reason for hiding this comment

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

While testing I encountered a crash due to index out of bound here, so I added this for safety.

for subview in subviews where subview.tag == tag {
subview.fadeOut() { _ in
subview.removeFromSuperview()
Expand Down