From d9fce6c304e7a18f3e49913aa117cc3bb13f9426 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Thu, 25 May 2023 13:59:14 +0100 Subject: [PATCH 1/3] 9773 Prevent TTP reconnection if no location authn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We reconnect to the reader automatically on foregrounding the app, if the device and store support it and TTP has been used before. For users which grant `allow once` permissions, this means that in the next app session, they’ll be asked to provide location permission without the context of a payment attempt, which is strange – the system permission alert pops up over whatever screen they happen to be on, interrupting their flow. With this change, we only attempt to reconnect if we already have location permission – this means we no longer show a location permission request out of context. --- .../TapToPayReconnectionController.swift | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/Settings/In-Person Payments/TapToPayReconnectionController.swift b/WooCommerce/Classes/ViewRelated/Dashboard/Settings/In-Person Payments/TapToPayReconnectionController.swift index bfaf93b5610..c44cd288f8e 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/Settings/In-Person Payments/TapToPayReconnectionController.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/Settings/In-Person Payments/TapToPayReconnectionController.swift @@ -1,5 +1,6 @@ import Foundation import Yosemite +import CoreLocation protocol BuiltInCardReaderConnectionControllerBuilding { func createConnectionController(forSiteID: Int64, @@ -47,6 +48,8 @@ final class TapToPayReconnectionController { private var adoptedConnectionCompletionHandler: ((Result) -> Void)? = nil + private var locationManager: CLLocationManager = CLLocationManager() + init(stores: StoresManager = ServiceLocator.stores, connectionControllerFactory: BuiltInCardReaderConnectionControllerBuilding = BuiltInCardReaderConnectionControllerFactory(), onboardingCache: CardPresentPaymentOnboardingStateCache = .shared) { @@ -70,7 +73,8 @@ final class TapToPayReconnectionController { isReconnecting = true let supportDeterminer = supportDeterminer ?? CardReaderSupportDeterminer(siteID: siteID) Task { @MainActor in - guard supportDeterminer.siteSupportsLocalMobileReader(), + guard locationIsAuthorized, + supportDeterminer.siteSupportsLocalMobileReader(), await supportDeterminer.deviceSupportsLocalMobileReader(), await supportDeterminer.hasPreviousTapToPayUsage(), await supportDeterminer.connectedReader() == nil, @@ -130,6 +134,17 @@ private extension TapToPayReconnectionController { connectionController = nil isReconnecting = false } + + var locationIsAuthorized: Bool { + switch locationManager.authorizationStatus { + case .notDetermined, .restricted, .denied: + return false + case .authorizedAlways, .authorizedWhenInUse: + return true + @unknown default: + return false + } + } } enum TapToPayReconnectionError: Error { From 94e0f9abe0639495c7546bb61d16d764be9aba32 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Thu, 25 May 2023 14:17:31 +0100 Subject: [PATCH 2/3] 9773 Add location request removal to 13.9 release --- RELEASE-NOTES.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index b2d34e29d5e..c3663b488eb 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -1,5 +1,9 @@ *** PLEASE FOLLOW THIS FORMAT: [] [] +13.9 +----- +- [*] Payments: Location permissions request is not shown to TTP users who grant "Allow once" permission on first foregrounding the app any more [https://github.com/woocommerce/woocommerce-ios/pull/9821] + 13.8 ----- - [Internal] Orders: Bundled products (within a product bundle) are now indented, to show their relationship to the parent bundle. [https://github.com/woocommerce/woocommerce-ios/pull/9778] From ad439264c1be5934604de3db7681f0d58065b2fa Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Thu, 25 May 2023 15:42:17 +0100 Subject: [PATCH 3/3] 9773 Update tests to include location authorization --- .../CardReaderSupportDeterminer.swift | 14 ++++++++++++++ .../TapToPayReconnectionController.swift | 16 +--------------- .../Mocks/MockCardReaderSupportDeterminer.swift | 6 ++++++ .../TapToPayReconnectionControllerTests.swift | 2 ++ 4 files changed, 23 insertions(+), 15 deletions(-) diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/Settings/In-Person Payments/CardReaderSupportDeterminer.swift b/WooCommerce/Classes/ViewRelated/Dashboard/Settings/In-Person Payments/CardReaderSupportDeterminer.swift index 6173fe24a4c..ef7019bab98 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/Settings/In-Person Payments/CardReaderSupportDeterminer.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/Settings/In-Person Payments/CardReaderSupportDeterminer.swift @@ -1,17 +1,20 @@ import Foundation import Yosemite +import CoreLocation protocol CardReaderSupportDetermining { func connectedReader() async -> CardReader? func hasPreviousTapToPayUsage() async -> Bool func siteSupportsLocalMobileReader() -> Bool func deviceSupportsLocalMobileReader() async -> Bool + var locationIsAuthorized: Bool { get } } final class CardReaderSupportDeterminer: CardReaderSupportDetermining { private let stores: StoresManager private let configuration: CardPresentPaymentsConfiguration private let siteID: Int64 + private var locationManager: CLLocationManager = CLLocationManager() init(siteID: Int64, configuration: CardPresentPaymentsConfiguration = CardPresentConfigurationLoader().configuration, @@ -21,6 +24,17 @@ final class CardReaderSupportDeterminer: CardReaderSupportDetermining { self.stores = stores } + var locationIsAuthorized: Bool { + switch locationManager.authorizationStatus { + case .notDetermined, .restricted, .denied: + return false + case .authorizedAlways, .authorizedWhenInUse: + return true + @unknown default: + return false + } + } + @MainActor func connectedReader() async -> CardReader? { await withCheckedContinuation { continuation in diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/Settings/In-Person Payments/TapToPayReconnectionController.swift b/WooCommerce/Classes/ViewRelated/Dashboard/Settings/In-Person Payments/TapToPayReconnectionController.swift index c44cd288f8e..7885320b9d3 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/Settings/In-Person Payments/TapToPayReconnectionController.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/Settings/In-Person Payments/TapToPayReconnectionController.swift @@ -1,6 +1,5 @@ import Foundation import Yosemite -import CoreLocation protocol BuiltInCardReaderConnectionControllerBuilding { func createConnectionController(forSiteID: Int64, @@ -48,8 +47,6 @@ final class TapToPayReconnectionController { private var adoptedConnectionCompletionHandler: ((Result) -> Void)? = nil - private var locationManager: CLLocationManager = CLLocationManager() - init(stores: StoresManager = ServiceLocator.stores, connectionControllerFactory: BuiltInCardReaderConnectionControllerBuilding = BuiltInCardReaderConnectionControllerFactory(), onboardingCache: CardPresentPaymentOnboardingStateCache = .shared) { @@ -73,7 +70,7 @@ final class TapToPayReconnectionController { isReconnecting = true let supportDeterminer = supportDeterminer ?? CardReaderSupportDeterminer(siteID: siteID) Task { @MainActor in - guard locationIsAuthorized, + guard supportDeterminer.locationIsAuthorized, supportDeterminer.siteSupportsLocalMobileReader(), await supportDeterminer.deviceSupportsLocalMobileReader(), await supportDeterminer.hasPreviousTapToPayUsage(), @@ -134,17 +131,6 @@ private extension TapToPayReconnectionController { connectionController = nil isReconnecting = false } - - var locationIsAuthorized: Bool { - switch locationManager.authorizationStatus { - case .notDetermined, .restricted, .denied: - return false - case .authorizedAlways, .authorizedWhenInUse: - return true - @unknown default: - return false - } - } } enum TapToPayReconnectionError: Error { diff --git a/WooCommerce/WooCommerceTests/Mocks/MockCardReaderSupportDeterminer.swift b/WooCommerce/WooCommerceTests/Mocks/MockCardReaderSupportDeterminer.swift index d040dd94700..869baf3febb 100644 --- a/WooCommerce/WooCommerceTests/Mocks/MockCardReaderSupportDeterminer.swift +++ b/WooCommerce/WooCommerceTests/Mocks/MockCardReaderSupportDeterminer.swift @@ -3,6 +3,12 @@ import Yosemite @testable import WooCommerce final class MockCardReaderSupportDeterminer: CardReaderSupportDetermining { + + var shouldReturnLocationIsAuthorized = false + var locationIsAuthorized: Bool { + return shouldReturnLocationIsAuthorized + } + var shouldReturnConnectedReader: CardReader? = nil func connectedReader() async -> CardReader? { return shouldReturnConnectedReader diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Settings/In-Person Payments/TapToPayReconnectionControllerTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Settings/In-Person Payments/TapToPayReconnectionControllerTests.swift index fbcaa45dd01..c6cb3d1a111 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Settings/In-Person Payments/TapToPayReconnectionControllerTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Settings/In-Person Payments/TapToPayReconnectionControllerTests.swift @@ -48,6 +48,7 @@ final class TapToPayReconnectionControllerTests: XCTestCase { func test_reconnectIfNeeded_calls_searchAndConnect_if_no_reader_connected_and_site_and_device_meet_requirements() throws { // Given let supportDeterminer = MockCardReaderSupportDeterminer() + supportDeterminer.shouldReturnLocationIsAuthorized = true supportDeterminer.shouldReturnConnectedReader = nil supportDeterminer.shouldReturnSiteSupportsLocalMobileReader = true supportDeterminer.shouldReturnDeviceSupportsLocalMobileReader = true @@ -69,6 +70,7 @@ final class TapToPayReconnectionControllerTests: XCTestCase { func test_reconnectIfNeeded_creates_a_new_connection_controller_with_expected_parameters() throws { // Given let supportDeterminer = MockCardReaderSupportDeterminer() + supportDeterminer.shouldReturnLocationIsAuthorized = true supportDeterminer.shouldReturnConnectedReader = nil supportDeterminer.shouldReturnSiteSupportsLocalMobileReader = true supportDeterminer.shouldReturnDeviceSupportsLocalMobileReader = true