Merge pull request #5341 from woocommerce/issue/5269-systemplugin-active

[Mobile Payments] Use active (not networkActivated) to determine plugin activation state

Author: allendav

Fakes/Fakes/Networking.generated.swift

@@ -1457,7 +1457,8 @@ extension SystemPlugin {
             url: .fake(),
             authorName: .fake(),
             authorUrl: .fake(),
-            networkActivated: .fake()
+            networkActivated: .fake(),
+            active: .fake()
         )
     }
 }

Networking/Networking/Mapper/SystemStatusMapper.swift

@@ -19,14 +19,16 @@ struct SystemStatusMapper: Mapper {
 
         let systemStatus = try decoder.decode(SystemStatusEnvelope.self, from: response).systemStatus
 
-        /// For now, we're going to override the networkActivated Bool in each plugin to convey active or inactive -- in order to
-        /// avoid a core data change to add a Bool for activated
-        /// This will be undone in #5269
+        /// Active and in-active plugins share identical structure, but are stored in separate parts of the remote response
+        /// (and without an active attribute in the response). So... we use the same decoder for active and in-active plugins
+        /// and here we apply the correct value for active (or not)
+        ///
         let activePlugins = systemStatus.activePlugins.map {
-            $0.overrideNetworkActivated(isNetworkActivated: true)
+            $0.copy(active: true)
         }
+
         let inactivePlugins = systemStatus.inactivePlugins.map {
-            $0.overrideNetworkActivated(isNetworkActivated: false)
+            $0.copy(active: false)
         }
 
         return activePlugins + inactivePlugins

Networking/Networking/Model/Copiable/Models+Copiable.generated.swift

@@ -1262,7 +1262,8 @@ extension SystemPlugin {
         url: CopiableProp<String> = .copy,
         authorName: CopiableProp<String> = .copy,
         authorUrl: CopiableProp<String> = .copy,
-        networkActivated: CopiableProp<Bool> = .copy
+        networkActivated: CopiableProp<Bool> = .copy,
+        active: CopiableProp<Bool> = .copy
     ) -> SystemPlugin {
         let siteID = siteID ?? self.siteID
         let plugin = plugin ?? self.plugin
@@ -1273,6 +1274,7 @@ extension SystemPlugin {
         let authorName = authorName ?? self.authorName
         let authorUrl = authorUrl ?? self.authorUrl
         let networkActivated = networkActivated ?? self.networkActivated
+        let active = active ?? self.active
 
         return SystemPlugin(
             siteID: siteID,
@@ -1283,7 +1285,8 @@ extension SystemPlugin {
             url: url,
             authorName: authorName,
             authorUrl: authorUrl,
-            networkActivated: networkActivated
+            networkActivated: networkActivated,
+            active: active
         )
     }
 }

Networking/Networking/Model/SystemPlugin.swift

@@ -39,6 +39,10 @@ public struct SystemPlugin: Decodable, GeneratedFakeable, GeneratedCopiable {
     ///
     public let networkActivated: Bool
 
+    /// Is the plugin active, i.e. false | true
+    ///
+    public let active: Bool
+
     /// Struct initializer.
     ///
     public init(siteID: Int64,
@@ -49,7 +53,8 @@ public struct SystemPlugin: Decodable, GeneratedFakeable, GeneratedCopiable {
                 url: String,
                 authorName: String,
                 authorUrl: String,
-                networkActivated: Bool) {
+                networkActivated: Bool,
+                active: Bool) {
         self.siteID = siteID
         self.plugin = plugin
         self.name = name
@@ -59,6 +64,7 @@ public struct SystemPlugin: Decodable, GeneratedFakeable, GeneratedCopiable {
         self.authorName = authorName
         self.authorUrl = authorUrl
         self.networkActivated = networkActivated
+        self.active = active
     }
     /// The public initializer for SystemPlugin.
     ///
@@ -76,6 +82,12 @@ public struct SystemPlugin: Decodable, GeneratedFakeable, GeneratedCopiable {
         let authorUrl = try container.decode(String.self, forKey: .authorUrl)
         let networkActivated = try container.decode(Bool.self, forKey: .networkActivated)
 
+        /// Active and in-active plugins share identical structure, but are stored in separate parts of the remote response
+        /// (and without an active attribute in the response). So... we use the same decoder for active and in-active plugins
+        /// and in SystemStatusMapper we apply the correct value for active (which here is defaulted to true)
+        ///
+        let active = true
+
         self.init(siteID: siteID,
                   plugin: plugin,
                   name: name,
@@ -84,23 +96,8 @@ public struct SystemPlugin: Decodable, GeneratedFakeable, GeneratedCopiable {
                   url: url,
                   authorName: authorName,
                   authorUrl: authorUrl,
-                  networkActivated: networkActivated)
-    }
-}
-
-extension SystemPlugin {
-    func overrideNetworkActivated(isNetworkActivated: Bool) -> SystemPlugin {
-        SystemPlugin(
-            siteID: self.siteID,
-            plugin: self.plugin,
-            name: self.name,
-            version: self.version,
-            versionLatest: self.versionLatest,
-            url: self.url,
-            authorName: self.authorName,
-            authorUrl: self.authorUrl,
-            networkActivated: isNetworkActivated
-        )
+                  networkActivated: networkActivated,
+                  active: active)
     }
 }
 

Networking/NetworkingTests/Mapper/SystemStatusMapperTests.swift

@@ -21,10 +21,8 @@ class SystemStatusMapperTests: XCTestCase {
         let expectedVersionLatest = "5.8.0"
         let expectedAuthorName = "Automattic"
         let expectedAuthorUrl = "https://woocommerce.com"
-        /// TODO - The mapper is overriding networkActivated to be true for active plugins in general
-        /// When we fix #5269 this test will need to be updated to properly test the
-        /// new `activated` attribute that will be added to SystemPlugin instead
-        let expectedNetworkActivated = true
+        let expectedNetworkActivated = false
+        let expectedActive = true
 
         // When
         let systemPlugins = try mapLoadSystemStatusResponse()
@@ -43,6 +41,7 @@ class SystemStatusMapperTests: XCTestCase {
         XCTAssertEqual(systemPlugin.authorName, expectedAuthorName)
         XCTAssertEqual(systemPlugin.authorUrl, expectedAuthorUrl)
         XCTAssertEqual(systemPlugin.networkActivated, expectedNetworkActivated)
+        XCTAssertEqual(systemPlugin.active, expectedActive)
     }
 
     /// Verifies the SystemPlugin fields are parsed correctly for an inactive plugin
@@ -57,10 +56,8 @@ class SystemStatusMapperTests: XCTestCase {
         let expectedVersionLatest = "1.7.2"
         let expectedAuthorName = "Matt Mullenweg"
         let expectedAuthorUrl = "http://ma.tt/"
-        /// TODO - The mapper is overriding networkActivated to be true for active plugins in general
-        /// When we fix #5269 this test will need to be updated to properly test the
-        /// new `activated` attribute that will be added to SystemPlugin instead
         let expectedNetworkActivated = false
+        let expectedActive = false
 
         // When
         let systemPlugins = try mapLoadSystemStatusResponse()
@@ -79,6 +76,7 @@ class SystemStatusMapperTests: XCTestCase {
         XCTAssertEqual(systemPlugin.authorName, expectedAuthorName)
         XCTAssertEqual(systemPlugin.authorUrl, expectedAuthorUrl)
         XCTAssertEqual(systemPlugin.networkActivated, expectedNetworkActivated)
+        XCTAssertEqual(systemPlugin.active, expectedActive)
     }
 }
 

WooCommerce/Classes/ViewRelated/Dashboard/Settings/In-Person Payments/CardPresentPaymentsOnboardingUseCase.swift

@@ -201,10 +201,7 @@ private extension CardPresentPaymentsOnboardingUseCase {
     }
 
     func isWCPayActivated(plugin: SystemPlugin) -> Bool {
-        // For now we are overriding networkActivated in SystemStatusMapper
-        // to convey active / not active for a plugin.
-        // TODO - replace with simply `activated` as part of #5269
-        return plugin.networkActivated
+        return plugin.active
     }
 
     func getWCPayAccount() -> PaymentGatewayAccount? {

WooCommerce/WooCommerceTests/UnitTests.xctestplan

@@ -27,7 +27,6 @@
   "testTargets" : [
     {
       "skippedTests" : [
-        "CardPresentPaymentsOnboardingUseCaseTests\/test_onboarding_returns_wcpay_not_activated_when_wcpay_installed_but_not_active()",
         "StripeCardReaderIntegrationTests"
       ],
       "target" : {

WooCommerce/WooCommerceTests/ViewRelated/CardPresentPayments/CardPresentPaymentsOnboardingUseCaseTests.swift

@@ -69,7 +69,6 @@ class CardPresentPaymentsOnboardingUseCaseTests: XCTestCase {
 
     }
 
-    // TODO: Unskip in #5269
     func test_onboarding_returns_wcpay_not_activated_when_wcpay_installed_but_not_active() {
         // Given
         setupCountry(country: .us)
@@ -359,15 +358,17 @@ private extension CardPresentPaymentsOnboardingUseCaseTests {
 // MARK: - Plugin helpers
 private extension CardPresentPaymentsOnboardingUseCaseTests {
     func setupPlugin(status: SitePluginStatusEnum, version: PluginVersion) {
+        let active = status == .active || status == .networkActive
+        let networkActivated = status == .networkActive
         let plugin = SystemPlugin
             .fake()
             .copy(
                 siteID: sampleSiteID,
                 plugin: "woocommerce-payments",
-                // status: status, // TODO fix in #5269
                 name: "WooCommerce Payments",
                 version: version.rawValue,
-                networkActivated: true // TODO remove in #5269
+                networkActivated: networkActivated,
+                active: active
             )
         storageManager.insertSampleSystemPlugin(readOnlySystemPlugin: plugin)
     }

Yosemite/Yosemite/Model/Storage/SystemPlugin+ReadOnlyConvertible.swift

@@ -17,6 +17,7 @@ extension Storage.SystemPlugin: ReadOnlyConvertible {
         version = entity.version
         versionLatest = entity.versionLatest
         networkActivated = entity.networkActivated
+        active = entity.active
     }
 
     /// Returns a readonly version of the Storage.SystemPlugin
@@ -30,6 +31,7 @@ extension Storage.SystemPlugin: ReadOnlyConvertible {
               url: url,
               authorName: authorName,
               authorUrl: authorUrl,
-              networkActivated: networkActivated)
+              networkActivated: networkActivated,
+              active: active)
     }
 }