Skip to content

Commit

Permalink
Inbox: Remove woo plugin check for eligibility checker (#13325)
Browse files Browse the repository at this point in the history
  • Loading branch information
itsmeichigo authored Jul 16, 2024
2 parents 6308bcf + a8b3076 commit 13c483b
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 233 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ final class DashboardViewModel: ObservableObject {
@MainActor
func reloadAllData() async {
isReloadingAllData = true
checkInboxEligibility()
await withTaskGroup(of: Void.self) { group in
group.addTask { [weak self] in
await self?.syncDashboardEssentialData()
Expand All @@ -194,9 +195,6 @@ final class DashboardViewModel: ObservableObject {
await reloadCards(showOnDashboardCards)
}
}
group.addTask { [weak self] in
await self?.checkInboxEligibility()
}
group.addTask { [weak self] in
await self?.loadDashboardCardsFromStorage()
}
Expand Down Expand Up @@ -482,12 +480,11 @@ private extension DashboardViewModel {
localAnnouncementViewModel = viewModel
}

@MainActor
func checkInboxEligibility() async {
func checkInboxEligibility() {
guard featureFlagService.isFeatureFlagEnabled(.dynamicDashboardM2) else {
return
}
isEligibleForInbox = await inboxEligibilityChecker.isEligibleForInbox(siteID: siteID)
isEligibleForInbox = inboxEligibilityChecker.isEligibleForInbox(siteID: siteID)
}

func configureOrdersResultController() {
Expand Down
12 changes: 2 additions & 10 deletions WooCommerce/Classes/ViewRelated/Hub Menu/HubMenuViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -341,20 +341,12 @@ private extension HubMenuViewModel {

func updateMenuItemEligibility(with site: Yosemite.Site) {

/// We're dispatching 3 separate tasks because using task group to
/// asynchronously update variables in the main thread is considered unsafe
/// when enabling concurrency checks.
/// Using task group would require more effort like this:
/// https://www.hackingwithswift.com/quick-start/concurrency/how-to-handle-different-result-types-in-a-task-group

isSiteEligibleForBlaze = blazeEligibilityChecker.isSiteEligible(site)

Task { @MainActor in
isSiteEligibleForGoogleAds = await googleAdsEligibilityChecker.isSiteEligible(siteID: site.siteID)
}
isSiteEligibleForInbox = inboxEligibilityChecker.isEligibleForInbox(siteID: site.siteID)

Task { @MainActor in
isSiteEligibleForInbox = await inboxEligibilityChecker.isEligibleForInbox(siteID: site.siteID)
isSiteEligibleForGoogleAds = await googleAdsEligibilityChecker.isSiteEligible(siteID: site.siteID)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,94 +8,25 @@ import Experiments
/// Since mobile requires API support for filtering, only stores with a minimum WC plugin version are eligible.
///
protocol InboxEligibilityChecker {
/// Determines whether the store is eligible for inbox feature.
/// - Parameters:
/// - siteID: the ID of the site to check for Inbox eligibility.
/// - completion: called when the Inbox eligibility is determined.
///
func isEligibleForInbox(siteID: Int64, completion: @escaping (Bool) -> Void)

/// Asynchronously determines whether the store is eligible for inbox feature.
/// - Parameters:
/// - siteID: the ID of the site to check for Inbox eligibility.
///
func isEligibleForInbox(siteID: Int64) async -> Bool
func isEligibleForInbox(siteID: Int64) -> Bool

}

/// Default implementation to check whether a store is eligible for Inbox feature.
///
final class InboxEligibilityUseCase: InboxEligibilityChecker {
private let stores: StoresManager
private let featureFlagService: FeatureFlagService

init(stores: StoresManager = ServiceLocator.stores, featureFlagService: FeatureFlagService = ServiceLocator.featureFlagService) {
self.stores = stores
init(featureFlagService: FeatureFlagService = ServiceLocator.featureFlagService) {
self.featureFlagService = featureFlagService
}

/// Determines whether the store is eligible for inbox feature.
/// - Parameters:
/// - siteID: the ID of the site to check for Inbox eligibility.
/// - completion: called when the Inbox eligibility is determined.
func isEligibleForInbox(siteID: Int64, completion: @escaping (Bool) -> Void) {
Task { @MainActor in
let result = await isEligibleForInbox(siteID: siteID)
completion(result)
}
}

@MainActor
func isEligibleForInbox(siteID: Int64) async -> Bool {
guard featureFlagService.isFeatureFlagEnabled(.inbox) else {
return false
}

if let savedPlugin = await fetchWooPluginFromStorage(siteID: siteID) {
return checkIfInboxIsSupported(wcPlugin: savedPlugin)
} else {
let remotePlugin = await fetchWooPluginFromRemote(siteID: siteID)
return checkIfInboxIsSupported(wcPlugin: remotePlugin)
}
}
}

private extension InboxEligibilityUseCase {
@MainActor
func fetchWooPluginFromStorage(siteID: Int64) async -> SystemPlugin? {
await withCheckedContinuation { continuation in
stores.dispatch(SystemStatusAction.fetchSystemPlugin(siteID: siteID, systemPluginName: Constants.wcPluginName) { plugin in
continuation.resume(returning: plugin)
})
}
}

@MainActor
func fetchWooPluginFromRemote(siteID: Int64) async -> SystemPlugin? {
await withCheckedContinuation { continuation in
stores.dispatch(SystemStatusAction.synchronizeSystemInformation(siteID: siteID) { result in
switch result {
case .success(let info):
let wcPlugin = info.systemPlugins.first(where: { $0.name == Constants.wcPluginName })
continuation.resume(returning: wcPlugin)
case .failure:
continuation.resume(returning: nil)
}
})
}
}

func checkIfInboxIsSupported(wcPlugin: SystemPlugin?) -> Bool {
guard let wcPlugin = wcPlugin, wcPlugin.active else {
return false
}
return VersionHelpers.isVersionSupported(version: wcPlugin.version,
minimumRequired: Constants.wcPluginMinimumVersion)
}

enum Constants {
static let wcPluginName = "WooCommerce"
// TODO: 6148 - Update the minimum WC version with inbox filtering.
static let wcPluginMinimumVersion = "5.0.0"
func isEligibleForInbox(siteID: Int64) -> Bool {
featureFlagService.isFeatureFlagEnabled(.inbox)
}
}
47 changes: 21 additions & 26 deletions WooCommerce/Classes/ViewRelated/Inbox/InboxNoteRow.swift
Original file line number Diff line number Diff line change
Expand Up @@ -105,33 +105,28 @@ struct InboxNoteRow: View {
}
}

@ViewBuilder
private func webView(url: URL) -> some View {
let isWPComStore = ServiceLocator.stores.sessionManager.defaultSite?.isWordPressComStore ?? false

if isWPComStore {
NavigationView {
AuthenticatedWebView(isPresented: .constant(tappedAction != nil),
url: url,
urlToTriggerExit: nil) { _ in

func webView(url: URL) -> some View {
NavigationStack {
Group {
if viewModel.shouldAuthenticateAdminPage {
AuthenticatedWebView(isPresented: .constant(tappedAction != nil),
url: url)
} else {
WebView(isPresented: .constant(tappedAction != nil),
url: url)
}
}
.navigationTitle(Localization.inboxWebViewTitle)
.navigationBarTitleDisplayMode(.inline)
.toolbar {
ToolbarItem(placement: .confirmationAction) {
Button(action: {
tappedAction = nil
}, label: {
Text(Localization.doneButtonWebview)
})
}
}
.navigationTitle(Localization.inboxWebViewTitle)
.navigationBarTitleDisplayMode(.inline)
.toolbar {
ToolbarItem(placement: .confirmationAction) {
Button(action: {
tappedAction = nil
}, label: {
Text(Localization.doneButtonWebview)
})
}
}
}
.wooNavigationBarStyle()
}
else {
SafariSheetView(url: url)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@ struct InboxNoteRowViewModel: Identifiable, Equatable {
featureFlagService.isFeatureFlagEnabled(.showInboxCTA)
}

var shouldAuthenticateAdminPage: Bool {
guard let site = stores.sessionManager.defaultSite else {
return false
}
return stores.shouldAuthenticateAdminPage(for: site)
}

init(note: InboxNote,
today: Date = .init(),
locale: Locale = .current,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,8 @@ import Foundation

final class MockInboxEligibilityChecker: InboxEligibilityChecker {
var isEligible = false
private(set) var isSiteEligibleInvoked = false

func isEligibleForInbox(siteID: Int64, completion: @escaping (Bool) -> Void) {
completion(isEligible)
isSiteEligibleInvoked = true
}

func isEligibleForInbox(siteID: Int64) async -> Bool {
isSiteEligibleInvoked = true
return isEligible
func isEligibleForInbox(siteID: Int64) -> Bool {
isEligible
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,7 @@ final class HubMenuViewModelTests: XCTestCase {
tapToPayBadgePromotionChecker: TapToPayBadgePromotionChecker(),
stores: stores,
inboxEligibilityChecker: inboxEligibilityChecker)
waitUntil {
inboxEligibilityChecker.isSiteEligibleInvoked
}

viewModel.setupMenuElements()

// Then inbox is in the menu
Expand Down
Original file line number Diff line number Diff line change
@@ -1,121 +1,18 @@
import XCTest
@testable import WooCommerce
import Yosemite

final class InboxEligibilityUseCaseTests: XCTestCase {

@MainActor
func test_async_isEligibleForInbox_returns_true_if_stored_woo_plugin_is_found_and_version_is_eligible_and_feature_flag_is_on() async {
// Given
let siteID: Int64 = 132
let featureFlagService = MockFeatureFlagService(isInboxOn: true)
let stores = MockStoresManager(sessionManager: .makeForTesting())
let useCase = InboxEligibilityUseCase(stores: stores, featureFlagService: featureFlagService)

var triggeredSyncingSystemInformation = false

// When
stores.whenReceivingAction(ofType: SystemStatusAction.self) { action in
switch action {
case let .fetchSystemPlugin(_, _, onCompletion):
onCompletion(SystemPlugin.fake().copy(siteID: siteID, name: "WooCommerce", version: "6.0.0", active: true))
case .synchronizeSystemInformation:
triggeredSyncingSystemInformation = true
default:
break
}
}
let result = await useCase.isEligibleForInbox(siteID: siteID)

// Then
XCTAssertTrue(result)
XCTAssertFalse(triggeredSyncingSystemInformation)
}

@MainActor
func test_async_isEligibleForInbox_returns_true_if_remote_woo_plugin_is_found_and_version_is_eligible_and_feature_flag_is_on() async {
// Given
let siteID: Int64 = 132
let featureFlagService = MockFeatureFlagService(isInboxOn: true)
let stores = MockStoresManager(sessionManager: .makeForTesting())
let useCase = InboxEligibilityUseCase(stores: stores, featureFlagService: featureFlagService)

var triggeredSyncingSystemInformation = false

// When
stores.whenReceivingAction(ofType: SystemStatusAction.self) { action in
switch action {
case let .fetchSystemPlugin(_, _, onCompletion):
onCompletion(nil)
case let .synchronizeSystemInformation(_, onCompletion):
let wooPlugin = SystemPlugin.fake().copy(siteID: siteID, name: "WooCommerce", version: "6.0.0", active: true)
let systemInfo = SystemInformation.fake().copy(systemPlugins: [wooPlugin])
triggeredSyncingSystemInformation = true
onCompletion(.success(systemInfo))
default:
break
}
}
let result = await useCase.isEligibleForInbox(siteID: siteID)

// Then
XCTAssertTrue(result)
XCTAssertTrue(triggeredSyncingSystemInformation)
}

@MainActor
func test_async_isEligibleForInbox_returns_false_if_stored_woo_plugin_is_found_and_version_is_eligible_and_feature_flag_is_off() async {
func test_async_isEligibleForInbox_returns_false_when_feature_flag_is_off() {
// Given
let siteID: Int64 = 132
let featureFlagService = MockFeatureFlagService(isInboxOn: false)
let stores = MockStoresManager(sessionManager: .makeForTesting())
let useCase = InboxEligibilityUseCase(stores: stores, featureFlagService: featureFlagService)

var triggeredSyncingSystemInformation = false

// When
stores.whenReceivingAction(ofType: SystemStatusAction.self) { action in
switch action {
case let .fetchSystemPlugin(_, _, onCompletion):
onCompletion(SystemPlugin.fake().copy(siteID: siteID, name: "WooCommerce", version: "6.0.0"))
case .synchronizeSystemInformation:
triggeredSyncingSystemInformation = true
default:
break
}
}
let result = await useCase.isEligibleForInbox(siteID: siteID)

// Then
XCTAssertFalse(result)
XCTAssertFalse(triggeredSyncingSystemInformation)
}

@MainActor
func test_async_isEligibleForInbox_returns_false_if_stored_woo_plugin_is_found_and_version_is_ineligible_and_feature_flag_is_on() async {
// Given
let siteID: Int64 = 132
let featureFlagService = MockFeatureFlagService(isInboxOn: true)
let stores = MockStoresManager(sessionManager: .makeForTesting())
let useCase = InboxEligibilityUseCase(stores: stores, featureFlagService: featureFlagService)

var triggeredSyncingSystemInformation = false
let useCase = InboxEligibilityUseCase(featureFlagService: featureFlagService)

// When
stores.whenReceivingAction(ofType: SystemStatusAction.self) { action in
switch action {
case let .fetchSystemPlugin(_, _, onCompletion):
onCompletion(SystemPlugin.fake().copy(siteID: siteID, name: "WooCommerce", version: "4.5.0"))
case .synchronizeSystemInformation:
triggeredSyncingSystemInformation = true
default:
break
}
}
let result = await useCase.isEligibleForInbox(siteID: siteID)
let result = useCase.isEligibleForInbox(siteID: siteID)

// Then
XCTAssertFalse(result)
XCTAssertFalse(triggeredSyncingSystemInformation)
}
}

0 comments on commit 13c483b

Please sign in to comment.