-
Notifications
You must be signed in to change notification settings - Fork 121
Fix missing card payment options #16411
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.
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
Changes from all commits
f733077
ad11342
c5ca724
d284cfc
f15876a
dd32b83
4c4771a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,11 +1,15 @@ | ||||||
| import Foundation | ||||||
| import protocol Storage.StorageManagerType | ||||||
| import protocol NetworkingCore.Network | ||||||
| import protocol WooFoundation.CrashLogger | ||||||
| import enum WooFoundation.SeverityLevel | ||||||
|
|
||||||
| /// Determines whether an order is eligible for card present payment or not | ||||||
| /// | ||||||
| public final class OrderCardPresentPaymentEligibilityStore: Store { | ||||||
| private let currentSite: () -> Site? | ||||||
| private let isCIABEnvironmentSupported: () -> Bool | ||||||
| private let crashLogger: CrashLogger | ||||||
| private lazy var siteCIABEligibilityChecker: CIABEligibilityCheckerProtocol = CIABEligibilityChecker( | ||||||
| currentSite: currentSite | ||||||
| ) | ||||||
|
|
@@ -14,9 +18,13 @@ public final class OrderCardPresentPaymentEligibilityStore: Store { | |||||
| dispatcher: Dispatcher, | ||||||
| storageManager: StorageManagerType, | ||||||
| network: Network, | ||||||
| crashLogger: CrashLogger, | ||||||
| isCIABEnvironmentSupported: @escaping () -> Bool, | ||||||
| currentSite: @escaping () -> Site? | ||||||
| ) { | ||||||
| self.currentSite = currentSite | ||||||
| self.isCIABEnvironmentSupported = isCIABEnvironmentSupported | ||||||
| self.crashLogger = crashLogger | ||||||
| super.init( | ||||||
| dispatcher: dispatcher, | ||||||
| storageManager: storageManager, | ||||||
|
|
@@ -56,20 +64,40 @@ private extension OrderCardPresentPaymentEligibilityStore { | |||||
| onCompletion: (Result<Bool, Error>) -> Void) { | ||||||
| let storage = storageManager.viewStorage | ||||||
|
|
||||||
| guard let site = storage.loadSite(siteID: siteID)?.toReadOnly() else { | ||||||
| return onCompletion( | ||||||
| .failure( | ||||||
| OrderIsEligibleForCardPresentPaymentError.siteNotFoundInStorage | ||||||
| /// The following checks are only relevant if CIAB is rolled out. | ||||||
| if isCIABEnvironmentSupported() { | ||||||
| let storageSite = storage.loadSite(siteID: siteID)?.toReadOnly() | ||||||
|
|
||||||
| let site: Site? | ||||||
| if let storageSite { | ||||||
| site = storageSite | ||||||
| } else { | ||||||
| /// Non - fatal fallback to `currentSite` when a storage site is missing | ||||||
| site = currentSite() | ||||||
|
|
||||||
| logFailedStorageSiteRead( | ||||||
| siteID: siteID, | ||||||
| currentSiteFallbackValue: site | ||||||
| ) | ||||||
| ) | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| guard siteCIABEligibilityChecker.isFeatureSupported(.cardReader, for: site) else { | ||||||
| return onCompletion( | ||||||
| .failure( | ||||||
| OrderIsEligibleForCardPresentPaymentError.cardReaderPaymentOptionIsNotSupportedForCIABSites | ||||||
| guard let site else { | ||||||
| logFailedDefaultSiteRead(siteID: siteID) | ||||||
|
|
||||||
| return onCompletion( | ||||||
| .failure( | ||||||
| OrderIsEligibleForCardPresentPaymentError.failedToObtainSite | ||||||
| ) | ||||||
| ) | ||||||
| ) | ||||||
| } | ||||||
|
|
||||||
| guard siteCIABEligibilityChecker.isFeatureSupported(.cardReader, for: site) else { | ||||||
| return onCompletion( | ||||||
| .failure( | ||||||
| OrderIsEligibleForCardPresentPaymentError.cardReaderPaymentOptionIsNotSupportedForCIABSites | ||||||
| ) | ||||||
| ) | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| guard let order = storage.loadOrder(siteID: siteID, orderID: orderID)?.toReadOnly() else { | ||||||
|
|
@@ -83,10 +111,42 @@ private extension OrderCardPresentPaymentEligibilityStore { | |||||
| } | ||||||
| } | ||||||
|
|
||||||
| /// Error logging | ||||||
| private extension OrderCardPresentPaymentEligibilityStore { | ||||||
| func logFailedStorageSiteRead(siteID: Int64, currentSiteFallbackValue: Site?) { | ||||||
| let message = "OrderCardPresentPaymentEligibilityStore: Storage site missing, falling back to currentSite." | ||||||
|
|
||||||
| DDLogError(message) | ||||||
|
|
||||||
| crashLogger.logMessage( | ||||||
| message, | ||||||
| properties: [ | ||||||
| "siteID": siteID, | ||||||
| "currentSiteID": currentSiteFallbackValue?.siteID ?? "empty", | ||||||
| ], | ||||||
| level: .error | ||||||
| ) | ||||||
| } | ||||||
|
|
||||||
| func logFailedDefaultSiteRead(siteID: Int64) { | ||||||
| let message = "OrderCardPresentPaymentEligibilityStore: Current default site missing." | ||||||
|
|
||||||
| DDLogError(message) | ||||||
|
|
||||||
| crashLogger.logMessage( | ||||||
| "OrderCardPresentPaymentEligibilityStore: Current default site missing.", | ||||||
| properties: [ | ||||||
| "requestedSiteID": siteID | ||||||
| ], | ||||||
| level: .error | ||||||
| ) | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| extension OrderCardPresentPaymentEligibilityStore { | ||||||
| enum OrderIsEligibleForCardPresentPaymentError: Error { | ||||||
| case orderNotFoundInStorage | ||||||
| case siteNotFoundInStorage | ||||||
| case failedToObtainSite | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps be slightly more descriptive here? Not essential but may help future debugging/support
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @joshheald Addressed in 8ebbd2e. The flow now proceeds to orders check in case if a site wasn't obtained. |
||||||
| case cardReaderPaymentOptionIsNotSupportedForCIABSites | ||||||
| } | ||||||
| } | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can continue with the eligibility check in this case, making the assumption that it's not a CIAB site and skipping those checks. It can be done later if you prefer.