Skip to content

[CIAB] Hide Blaze and Payments from hub menu #16096

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

Jump to bottom
Merged
merged 6 commits into from
Sep 11, 2025
Merged

[CIAB] Hide Blaze and Payments from hub menu #16096

merged 6 commits into from
Sep 11, 2025

Conversation

@RafaelKayumov
Copy link
Contributor

@RafaelKayumov RafaelKayumov commented Sep 5, 2025 •
edited
Loading

WOOMOB-1272

Description

Utilizes CIABEligibilityChecker and conditionally removes "Payments" and "Blaze" options from Hub Menu. Also Injects CIABEligibilityChecker into BlazeEligibilityChecker. BlazeEligibilityChecker now takes into account if the site is CIAB.

Non-CIAB site testing case (default behaviour)

  • Use testing site that doesn't contain "garden" or "ciab" in name (this is the temp CIAB / non-CIAB site distinguishing logic).
  • Site should be eligible for Blaze.
  • Navigate to "Menu" tab.
  • Make sure "Payments" and "Blaze" items are presented.
Снимок экрана 2025-09-10 в 15 48 07
  • Navigate to a product on product tab.
  • Make sure the "Blaze" item is presented.
Снимок экрана 2025-09-10 в 15 52 14

CIAB site testing case

  • Add "garden" or "ciab" into site name in admin dashboard
  • Restart the app and make sure the name change took place
  • Navigate to "Menu" tab.
  • Make sure "Payments" and "Blaze" items are NOT presented.
Снимок экрана 2025-09-10 в 15 41 44
  • Navigate to a product on product tab.
  • Make sure the "Blaze" item is NOT presented.
Снимок экрана 2025-09-10 в 15 53 27

Testing Info

Tested the above steps on iPhone Simulator 18.4

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@RafaelKayumov RafaelKayumov added this to the 23.3 milestone Sep 5, 2025
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Sep 5, 2025 •
edited
Loading

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

App NameWooCommerce iOS Prototype
Build Numberpr16096-c5dc307
Version23.2
Bundle IDcom.automattic.alpha.woocommerce
Commitc5dc307
Installation URL3hgc0dgu4i7hg
 Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@RafaelKayumov RafaelKayumov force-pushed the WOOMOB-1272-hide-blaze-and-payments-from-hub-menu branch from 8240073 to 97b5f1d Compare September 9, 2025 12:30
Base automatically changed from WOOMOB-1268-indentify-CIAB-sites to trunk September 10, 2025 09:41
@RafaelKayumov RafaelKayumov force-pushed the WOOMOB-1272-hide-blaze-and-payments-from-hub-menu branch from 97b5f1d to c85ccdf Compare September 10, 2025 12:22
@RafaelKayumov RafaelKayumov changed the base branch from trunk to WOOMOB-1271-hide-split-shipments-for-ciab September 10, 2025 12:23
@RafaelKayumov RafaelKayumov marked this pull request as ready for review September 10, 2025 13:18
@itsmeichigo itsmeichigo self-assigned this Sep 11, 2025
itsmeichigo
itsmeichigo approved these changes Sep 11, 2025
View reviewed changes
Copy link
Contributor

@itsmeichigo itsmeichigo left a comment

Choose a reason for hiding this comment

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

Tested and confirmed that Blaze and Payments are only available for non-CIAB sites.
I'm confused that Blaze is also hidden from dashboard, while that seems to be part of a draft PR. Could this be an issue with splitting code from a bigger change?

I left some nit-picking in the comments.

WooCommerce/Classes/Blaze/BlazeEligibilityChecker.swift
guard
site.isAdmin,
site.canBlaze,
siteCIABEligibilityChecker.isFeatureSupported(.blaze, for: site)
Copy link
Contributor

@itsmeichigo itsmeichigo Sep 11, 2025

Choose a reason for hiding this comment

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

Nit: Please consider adding unit tests to check for this condition.

Copy link
Contributor Author

@RafaelKayumov RafaelKayumov Sep 11, 2025

Choose a reason for hiding this comment

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

Done in c337898

WooCommerce/Classes/ViewRelated/Hub Menu/HubMenuViewModel.swift
stores: StoresManager = ServiceLocator.stores,
generalAppSettings: GeneralAppSettingsStorage = ServiceLocator.generalAppSettings,
inboxEligibilityChecker: InboxEligibilityChecker = InboxEligibilityUseCase(),
blazeEligibilityChecker: BlazeEligibilityCheckerProtocol = BlazeEligibilityChecker(),
Copy link
Contributor

@itsmeichigo itsmeichigo Sep 11, 2025

Choose a reason for hiding this comment

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

Should we inject CIABEligibilityChecker to this checker too?

Suggested change
blazeEligibilityChecker: BlazeEligibilityCheckerProtocol = BlazeEligibilityChecker(),
blazeEligibilityChecker: BlazeEligibilityCheckerProtocol? = nil,

then inside init:

self.blazeEligibilityChecker = blazeEligibilityChecker ?? BlazeEligibilityChecker(siteCIABEligibilityChecker: siteCIABEligibilityChecker)

Copy link
Contributor Author

@RafaelKayumov RafaelKayumov Sep 11, 2025

Choose a reason for hiding this comment

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

Thx for the attention here.
I'd still prefer to leave it. If we make a fallback to BlazeEligibilityChecker(...) for the nil blazeEligibilityChecker argument and them purposely (for whatever reason) pass nil - it will be neglected and overridden by the ?? operator.
Currently the BlazeEligibilityChecker() initializer already has a default value for the siteCIABEligibilityChecker so we should be good. Even if there are different CIABEligibilityChecker instances - they can work independently and show correct results.

Base automatically changed from WOOMOB-1271-hide-split-shipments-for-ciab to trunk September 11, 2025 08:38
@RafaelKayumov
Copy link
Contributor Author

RafaelKayumov commented Sep 11, 2025

I'm confused that Blaze is also hidden from dashboard, while that seems to be part of a draft PR. Could this be an issue with splitting code from a bigger change?

@itsmeichigo Thx for the review. That's true. After starting the work on the dashboard cards hiding I also got confused because there was no Blaze card. Turned out to be a bug where the default site was nil upon check in BlazeCampaignDashboardViewModel:

func checkSiteEligibility() async -> Bool {
        guard let site = stores.sessionManager.defaultSite else {
            return false
        }
        return await blazeEligibilityChecker.isSiteEligible(site)
    }

In that draft PR I reworked it and bound to reactive stores.site instead.

@RafaelKayumov RafaelKayumov merged commit 1365091 into trunk Sep 11, 2025
14 checks passed
@RafaelKayumov RafaelKayumov deleted the WOOMOB-1272-hide-blaze-and-payments-from-hub-menu branch September 11, 2025 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@itsmeichigo itsmeichigo itsmeichigo approved these changes

Assignees

@itsmeichigo itsmeichigo

Labels

feature: CIAB Mobile Experience type: task An internally driven task.

Projects

None yet

Milestone

23.3

Development

Successfully merging this pull request may close these issues.

4 participants

@RafaelKayumov @wpmobilebot @itsmeichigo