Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 11 additions & 2 deletions WooCommerce/Classes/Blaze/BlazeEligibilityChecker.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,14 @@ protocol BlazeEligibilityCheckerProtocol {
/// Checks for Blaze eligibility for a site and its products.
final class BlazeEligibilityChecker: BlazeEligibilityCheckerProtocol {
private let stores: StoresManager
private let siteCIABEligibilityChecker: CIABEligibilityCheckerProtocol

init(stores: StoresManager = ServiceLocator.stores) {
init(
stores: StoresManager = ServiceLocator.stores,
siteCIABEligibilityChecker: CIABEligibilityCheckerProtocol = CIABEligibilityChecker()
) {
self.stores = stores
self.siteCIABEligibilityChecker = siteCIABEligibilityChecker
}

/// Checks if the site is eligible for Blaze.
Expand All @@ -39,7 +44,11 @@ final class BlazeEligibilityChecker: BlazeEligibilityCheckerProtocol {
private extension BlazeEligibilityChecker {
@MainActor
func checkSiteEligibility(_ site: Site) async -> Bool {
guard site.isAdmin && site.canBlaze else {
guard
site.isAdmin,
site.canBlaze,
siteCIABEligibilityChecker.isFeatureSupported(.blaze, for: site)
Copy link
Contributor

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

Choose a reason for hiding this comment

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

Done in c337898

else {
return false
}

Expand Down
69 changes: 49 additions & 20 deletions WooCommerce/Classes/ViewRelated/Hub Menu/HubMenuViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,13 @@ final class HubMenuViewModel: ObservableObject {
private let inboxEligibilityChecker: InboxEligibilityChecker
private let blazeEligibilityChecker: BlazeEligibilityCheckerProtocol
private let googleAdsEligibilityChecker: GoogleAdsEligibilityChecker
private let siteCIABEligibilityChecker: CIABEligibilityCheckerProtocol

private(set) lazy var inboxViewModel = InboxViewModel(siteID: siteID)

@Published private(set) var shouldShowNewFeatureBadgeOnPayments: Bool = false

@Published private var isSiteEligibleForPayments = false
@Published private var isSiteEligibleForBlaze = false
@Published private var isSiteEligibleForGoogleAds = false
@Published private var isSiteEligibleForInbox = false
Expand Down Expand Up @@ -125,6 +127,7 @@ final class HubMenuViewModel: ObservableObject {
inboxEligibilityChecker: InboxEligibilityChecker = InboxEligibilityUseCase(),
blazeEligibilityChecker: BlazeEligibilityCheckerProtocol = BlazeEligibilityChecker(),
Copy link
Contributor

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

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.

googleAdsEligibilityChecker: GoogleAdsEligibilityChecker = DefaultGoogleAdsEligibilityChecker(),
siteCIABEligibilityChecker: CIABEligibilityCheckerProtocol = CIABEligibilityChecker(),
analytics: Analytics = ServiceLocator.analytics) {
self.siteID = siteID
self.credentials = stores.sessionManager.defaultCredentials
Expand All @@ -136,6 +139,7 @@ final class HubMenuViewModel: ObservableObject {
self.inboxEligibilityChecker = inboxEligibilityChecker
self.blazeEligibilityChecker = blazeEligibilityChecker
self.googleAdsEligibilityChecker = googleAdsEligibilityChecker
self.siteCIABEligibilityChecker = siteCIABEligibilityChecker
self.cardPresentPaymentsOnboarding = CardPresentPaymentsOnboardingUseCase()
self.analytics = analytics
observeSiteForUIUpdates()
Expand Down Expand Up @@ -251,30 +255,55 @@ private extension HubMenuViewModel {
}

func setupGeneralElements() {
$shouldShowNewFeatureBadgeOnPayments
.combineLatest($isSiteEligibleForInbox,
$isSiteEligibleForBlaze,
$isSiteEligibleForGoogleAds)
.map { [weak self] combinedResult -> [HubMenuItem] in
guard let self else { return [] }
let (shouldShowBadgeOnPayments, eligibleForInbox, eligibleForBlaze, eligibleForGoogleAds) = combinedResult
return createGeneralElements(
shouldShowBadgeOnPayments: shouldShowBadgeOnPayments,
eligibleForGoogleAds: eligibleForGoogleAds,
eligibleForBlaze: eligibleForBlaze,
eligibleForInbox: eligibleForInbox
)
}
.assign(to: &$generalElements)
Publishers.CombineLatest(
$shouldShowNewFeatureBadgeOnPayments,
$isSiteEligibleForPayments
)
.combineLatest(
Publishers.CombineLatest3(
$isSiteEligibleForInbox,
$isSiteEligibleForBlaze,
$isSiteEligibleForGoogleAds
)
)
.map { [weak self] combinedResults -> [HubMenuItem] in
guard let self else { return [] }

let ((shouldShowBadgeOnPayments, eligibleForPayments), (eligibleForInbox, eligibleForBlaze, eligibleForGoogleAds)) = combinedResults

let paymentsEligibility: PaymentsFeatureEligibility = eligibleForPayments ?
.eligible(shouldShowBadgeOnPayments: shouldShowBadgeOnPayments) :
.ineligible

return createGeneralElements(
paymentsEligibility: paymentsEligibility,
eligibleForGoogleAds: eligibleForGoogleAds,
eligibleForBlaze: eligibleForBlaze,
eligibleForInbox: eligibleForInbox
)
}
.assign(to: &$generalElements)
}

enum PaymentsFeatureEligibility {
case ineligible
case eligible(shouldShowBadgeOnPayments: Bool)
}

func createGeneralElements(shouldShowBadgeOnPayments: Bool,
func createGeneralElements(paymentsEligibility: PaymentsFeatureEligibility,
eligibleForGoogleAds: Bool,
eligibleForBlaze: Bool,
eligibleForInbox: Bool) -> [HubMenuItem] {
var items: [HubMenuItem] = [
Payments(iconBadge: shouldShowBadgeOnPayments ? .dot : nil)
]
var items: [HubMenuItem] = []

switch paymentsEligibility {
case .ineligible:
break
case .eligible(let shouldShowBadgeOnPayments):
items.append(
Payments(iconBadge: shouldShowBadgeOnPayments ? .dot : nil)
)
}

if shouldShowAISettings {
items.append(AISettings())
Expand Down Expand Up @@ -354,7 +383,7 @@ private extension HubMenuViewModel {
}

func updateMenuItemEligibility(with site: Yosemite.Site) {

isSiteEligibleForPayments = siteCIABEligibilityChecker.isFeatureSupported(.payments, for: site)
isSiteEligibleForInbox = inboxEligibilityChecker.isEligibleForInbox(siteID: site.siteID)

Task { @MainActor in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,6 @@ final class MockCIABEligibilityChecker: CIABEligibilityCheckerProtocol {
}

func isFeatureSupported(_ feature: CIABAffectedFeature, for site: Site) -> Bool {
return !mockedCIABDisabledFeatures.contains(feature) && mockedCIABSites.contains(site)
return !mockedCIABDisabledFeatures.contains(feature) || !mockedCIABSites.contains(site)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,53 @@ final class BlazeEligibilityCheckerTests: XCTestCase {
XCTAssertTrue(isEligible)
}

@MainActor
func test_isEligible_is_true_when_site_is_non_ciab_and_other_requirements_met() async {
// Given
stores.authenticate(credentials: .wpcom(username: "", authToken: "", siteAddress: ""))
let site = mockSite(isEligibleForBlaze: true,
isJetpackThePluginInstalled: false,
isJetpackConnected: true)
let siteIsCIABChecker = MockCIABEligibilityChecker(
mockedIsCurrentSiteCIAB: false,
)
let blazeEligibilityChecker = BlazeEligibilityChecker(
stores: stores,
siteCIABEligibilityChecker: siteIsCIABChecker
)
mockPluginFetch(remotePlugin: .fake().copy(plugin: Self.pluginSlug, active: true))

// When
let isEligible = await blazeEligibilityChecker.isSiteEligible(site)

// Then
XCTAssertTrue(isEligible)
}

@MainActor
func test_isEligible_is_false_when_site_is_ciab_and_other_requirements_met() async {
// Given
stores.authenticate(credentials: .wpcom(username: "", authToken: "", siteAddress: ""))
let site = mockSite(isEligibleForBlaze: true,
isJetpackThePluginInstalled: false,
isJetpackConnected: true)
let siteIsCIABChecker = MockCIABEligibilityChecker(
mockedIsCurrentSiteCIAB: true,
mockedCIABSites: [site]
)
let blazeEligibilityChecker = BlazeEligibilityChecker(
stores: stores,
siteCIABEligibilityChecker: siteIsCIABChecker
)
mockPluginFetch(remotePlugin: .fake().copy(plugin: Self.pluginSlug, active: true))

// When
let isEligible = await blazeEligibilityChecker.isSiteEligible(site)

// Then
XCTAssertFalse(isEligible)
}

// MARK: - `isProductEligible`

@MainActor
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,64 @@ final class HubMenuViewModelTests: XCTestCase {
XCTAssertTrue(expectedElementsIDs.isSubset(of: generalElementsIds))
}

@MainActor
func test_generalElements_does_not_include_blaze_when_site_is_CIAB_site() {
// Given
let stores = MockStoresManager(sessionManager: .makeForTesting())
// Setting site ID is required before setting `Site`.
stores.updateDefaultStore(storeID: sampleSiteID)
stores.updateDefaultStore(.fake().copy(siteID: sampleSiteID))

let blazeEligibilityChecker = MockBlazeEligibilityChecker(isSiteEligible: false)
let mockCIABEligibilityChecker = MockCIABEligibilityChecker(
mockedIsCurrentSiteCIAB: true,
mockedCIABSites: [stores.sessionManager.defaultSite ?? .fake()]
)

// When
let viewModel = HubMenuViewModel(
siteID: sampleSiteID,
tapToPayBadgePromotionChecker: TapToPayBadgePromotionChecker(),
stores: stores,
blazeEligibilityChecker: blazeEligibilityChecker,
siteCIABEligibilityChecker: mockCIABEligibilityChecker
)

viewModel.setupMenuElements()

// Then
XCTAssertNil(viewModel.generalElements.firstIndex(where: { $0.id == HubMenuViewModel.Blaze.id }))
}

@MainActor
func test_generalElements_does_not_include_payments_when_site_is_CIAB_site() {
// Given
let stores = MockStoresManager(sessionManager: .makeForTesting())
// Setting site ID is required before setting `Site`.
stores.updateDefaultStore(storeID: sampleSiteID)
stores.updateDefaultStore(.fake().copy(siteID: sampleSiteID))

let blazeEligibilityChecker = MockBlazeEligibilityChecker(isSiteEligible: false)
let mockCIABEligibilityChecker = MockCIABEligibilityChecker(
mockedIsCurrentSiteCIAB: true,
mockedCIABSites: [stores.sessionManager.defaultSite ?? .fake()]
)

// When
let viewModel = HubMenuViewModel(
siteID: sampleSiteID,
tapToPayBadgePromotionChecker: TapToPayBadgePromotionChecker(),
stores: stores,
blazeEligibilityChecker: blazeEligibilityChecker,
siteCIABEligibilityChecker: mockCIABEligibilityChecker
)

viewModel.setupMenuElements()

// Then
XCTAssertNil(viewModel.generalElements.firstIndex(where: { $0.id == HubMenuViewModel.Payments.id }))
}

@MainActor
func test_generalElements_does_not_include_blaze_when_default_site_is_not_set() {
// When
Expand Down