From 7124c49d7d15b568ba597eb8e4725e1e635671f5 Mon Sep 17 00:00:00 2001 From: Juan Manuel Pereira Date: Wed, 3 Apr 2024 08:51:35 -0300 Subject: [PATCH] Add Web UI loading state pixels (#2531) --- DuckDuckGo/DBP/DBPHomeViewController.swift | 5 +- .../Pixels/DataBrokerProtectionPixels.swift | 21 ++- .../DataBrokerProtectionWebUIPixels.swift | 69 +++++++ .../DataBrokerProtectionViewController.swift | 36 ++++ ...DataBrokerProtectionWebUIPixelsTests.swift | 168 ++++++++++++++++++ 5 files changed, 297 insertions(+), 2 deletions(-) create mode 100644 LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Pixels/DataBrokerProtectionWebUIPixels.swift create mode 100644 LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionWebUIPixelsTests.swift diff --git a/DuckDuckGo/DBP/DBPHomeViewController.swift b/DuckDuckGo/DBP/DBPHomeViewController.swift index f3b2cf95d0..fdfca10e97 100644 --- a/DuckDuckGo/DBP/DBPHomeViewController.swift +++ b/DuckDuckGo/DBP/DBPHomeViewController.swift @@ -195,7 +195,10 @@ public class DataBrokerProtectionPixelsHandler: EventMapping + private var wasHTTPErrorPixelFired = false + + init(pixelHandler: EventMapping) { + self.pixelHandler = pixelHandler + } + + func firePixel(for error: Error) { + if wasHTTPErrorPixelFired { + wasHTTPErrorPixelFired = false // We reset the flag + return + } + + let nsError = error as NSError + + if nsError.domain == NSURLErrorDomain { + let statusCode = nsError.code + if statusCode >= 400 && statusCode < 600 { + pixelHandler.fire(.webUILoadingFailed(errorCategory: "httpError-\(statusCode)")) + wasHTTPErrorPixelFired = true + } else { + pixelHandler.fire(.webUILoadingFailed(errorCategory: "other-\(nsError.code)")) + } + } else { + pixelHandler.fire(.webUILoadingFailed(errorCategory: "other-\(nsError.code)")) + } + } + + func firePixel(for selectedURL: DataBrokerProtectionWebUIURLType, type: PixelType) { + let environment = selectedURL == .custom ? "staging" : "production" + + switch type { + case .loading: + pixelHandler.fire(.webUILoadingStarted(environment: environment)) + case .success: + pixelHandler.fire(.webUILoadingSuccess(environment: environment)) + } + } +} diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/UI/DataBrokerProtectionViewController.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/UI/DataBrokerProtectionViewController.swift index 9ac49e395a..688de0476e 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/UI/DataBrokerProtectionViewController.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/UI/DataBrokerProtectionViewController.swift @@ -18,7 +18,9 @@ import Cocoa import SwiftUI +import Common import BrowserServicesKit +import PixelKit import WebKit import Combine @@ -29,6 +31,8 @@ final public class DataBrokerProtectionViewController: NSViewController { private var loader: NSProgressIndicator! private let webUISettings: DataBrokerProtectionWebUIURLSettingsRepresentable private let webUIViewModel: DBPUIViewModel + private let pixelHandler: EventMapping + private let webUIPixel: DataBrokerProtectionWebUIPixels private let openURLHandler: (URL?) -> Void private var reloadObserver: NSObjectProtocol? @@ -43,6 +47,8 @@ final public class DataBrokerProtectionViewController: NSViewController { self.dataManager = dataManager self.openURLHandler = openURLHandler self.webUISettings = webUISettings + self.pixelHandler = DataBrokerProtectionPixelsHandler() + self.webUIPixel = DataBrokerProtectionWebUIPixels(pixelHandler: pixelHandler) self.webUIViewModel = DBPUIViewModel(dataManager: dataManager, scheduler: scheduler, webUISettings: webUISettings, @@ -82,6 +88,7 @@ final public class DataBrokerProtectionViewController: NSViewController { addLoadingIndicator() if let url = URL(string: webUISettings.selectedURL) { + webUIPixel.firePixel(for: webUISettings.selectedURLType, type: .loading) webView?.load(url) } else { removeLoadingIndicator() @@ -126,11 +133,40 @@ extension DataBrokerProtectionViewController: WKUIDelegate { extension DataBrokerProtectionViewController: WKNavigationDelegate { + public func webView(_ webView: WKWebView, didFail navigation: WKNavigation!, withError error: any Error) { + fireWebViewError(error) + } + + public func webView(_ webView: WKWebView, didFailProvisionalNavigation navigation: WKNavigation!, withError error: any Error) { + fireWebViewError(error) + } + + private func fireWebViewError(_ error: Error) { + webUIPixel.firePixel(for: error) + removeLoadingIndicator() + } + + public func webView(_ webView: WKWebView, decidePolicyFor navigationResponse: WKNavigationResponse) async -> WKNavigationResponsePolicy { + guard let statusCode = (navigationResponse.response as? HTTPURLResponse)?.statusCode else { + // if there's no http status code to act on, exit and allow navigation + return .allow + } + + if statusCode >= 400 { + webUIPixel.firePixel(for: NSError(domain: NSURLErrorDomain, code: statusCode)) + return .cancel + } + + return .allow + } + public func webView(_ webView: WKWebView, didStartProvisionalNavigation navigation: WKNavigation!) { loader.startAnimation(nil) } public func webView(_ webView: WKWebView, didFinish navigation: WKNavigation!) { removeLoadingIndicator() + + webUIPixel.firePixel(for: webUISettings.selectedURLType, type: .success) } } diff --git a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionWebUIPixelsTests.swift b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionWebUIPixelsTests.swift new file mode 100644 index 0000000000..f42517fb5a --- /dev/null +++ b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionWebUIPixelsTests.swift @@ -0,0 +1,168 @@ +// +// DataBrokerProtectionWebUIPixelsTests.swift +// +// Copyright © 2023 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import XCTest +import Foundation +@testable import DataBrokerProtection + +final class DataBrokerProtectionWebUIPixelsTests: XCTestCase { + + let handler = MockDataBrokerProtectionPixelsHandler() + + override func tearDown() { + handler.clear() + } + + func testWhenURLErrorIsHttp_thenCorrectPixelIsFired() { + let sut = DataBrokerProtectionWebUIPixels(pixelHandler: handler) + + sut.firePixel(for: NSError(domain: NSURLErrorDomain, code: 404)) + + let lastPixelFired = MockDataBrokerProtectionPixelsHandler.lastPixelsFired.first! + + XCTAssertEqual( + lastPixelFired.params!["error_category"], + "httpError-404" + ) + } + + func testWhenURLErrorIsNotHttp_thenCorrectPixelIsFired() { + let sut = DataBrokerProtectionWebUIPixels(pixelHandler: handler) + + sut.firePixel(for: NSError(domain: NSURLErrorDomain, code: 100)) + + let lastPixelFired = MockDataBrokerProtectionPixelsHandler.lastPixelsFired.first! + + XCTAssertEqual( + lastPixelFired.params!["error_category"], + "other-100" + ) + } + + func testWhenErrorIsNotURL_thenCorrectPixelIsFired() { + let sut = DataBrokerProtectionWebUIPixels(pixelHandler: handler) + + sut.firePixel(for: NSError(domain: NSCocoaErrorDomain, code: 500)) + + let lastPixelFired = MockDataBrokerProtectionPixelsHandler.lastPixelsFired.first! + + XCTAssertEqual( + lastPixelFired.params!["error_category"], + "other-500" + ) + } + + func testWhenSelectedURLisCustomAndLoading_thenStagingParamIsSent() { + let sut = DataBrokerProtectionWebUIPixels(pixelHandler: handler) + + sut.firePixel(for: .custom, type: .loading) + + let lastPixelFired = MockDataBrokerProtectionPixelsHandler.lastPixelsFired.first! + + XCTAssertEqual( + lastPixelFired.name, + DataBrokerProtectionPixels.webUILoadingStarted(environment: "staging").name + ) + XCTAssertEqual( + lastPixelFired.params!["environment"], + "staging" + ) + } + + func testWhenSelectedURLisProductionAndLoading_thenProductionParamIsSent() { + let sut = DataBrokerProtectionWebUIPixels(pixelHandler: handler) + + sut.firePixel(for: .production, type: .loading) + + let lastPixelFired = MockDataBrokerProtectionPixelsHandler.lastPixelsFired.first! + + XCTAssertEqual( + lastPixelFired.name, + DataBrokerProtectionPixels.webUILoadingStarted(environment: "staging").name + ) + XCTAssertEqual( + lastPixelFired.params!["environment"], + "production" + ) + } + + func testWhenSelectedURLisCustomAndSuccess_thenStagingParamIsSent() { + let sut = DataBrokerProtectionWebUIPixels(pixelHandler: handler) + + sut.firePixel(for: .custom, type: .success) + + let lastPixelFired = MockDataBrokerProtectionPixelsHandler.lastPixelsFired.first! + + XCTAssertEqual( + lastPixelFired.name, + DataBrokerProtectionPixels.webUILoadingSuccess(environment: "staging").name + ) + XCTAssertEqual( + lastPixelFired.params!["environment"], + "staging" + ) + } + + func testWhenSelectedURLisProductionAndSuccess_thenProductionParamIsSent() { + let sut = DataBrokerProtectionWebUIPixels(pixelHandler: handler) + + sut.firePixel(for: .production, type: .success) + + let lastPixelFired = MockDataBrokerProtectionPixelsHandler.lastPixelsFired.first! + + XCTAssertEqual( + lastPixelFired.name, + DataBrokerProtectionPixels.webUILoadingSuccess(environment: "staging").name + ) + XCTAssertEqual( + lastPixelFired.params!["environment"], + "production" + ) + } + + func testWhenHTTPPixelIsFired_weDoNotFireAnotherPixelRightAway() { + let sut = DataBrokerProtectionWebUIPixels(pixelHandler: handler) + + sut.firePixel(for: NSError(domain: NSURLErrorDomain, code: 404)) + sut.firePixel(for: NSError(domain: NSCocoaErrorDomain, code: 500)) + + let httpPixel = MockDataBrokerProtectionPixelsHandler.lastPixelsFired.first! + + XCTAssertEqual( + httpPixel.params!["error_category"], + "httpError-404" + ) + XCTAssertEqual(MockDataBrokerProtectionPixelsHandler.lastPixelsFired.count, 1) // We only fire one pixel + } + + func testWhenHTTPPixelIsFired_weFireTheNextErrorPixelOnTheSecondTry() { + let sut = DataBrokerProtectionWebUIPixels(pixelHandler: handler) + + sut.firePixel(for: NSError(domain: NSURLErrorDomain, code: 404)) + sut.firePixel(for: NSError(domain: NSCocoaErrorDomain, code: 500)) + sut.firePixel(for: NSError(domain: NSCocoaErrorDomain, code: 500)) + + let httpPixel = MockDataBrokerProtectionPixelsHandler.lastPixelsFired.first! + + XCTAssertEqual( + httpPixel.params!["error_category"], + "httpError-404" + ) + XCTAssertEqual(MockDataBrokerProtectionPixelsHandler.lastPixelsFired.count, 2) // We fire the HTTP pixel and the second cocoa error pixel + } +}