From f783437aa5c8e0380e62dfbd9f4c91f34d662f91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mariusz=20=C5=9Apiewak?= Date: Fri, 15 Nov 2024 13:14:57 +0100 Subject: [PATCH 1/3] Prevent sending duplicate home tab pixel --- DuckDuckGo.xcodeproj/project.pbxproj | 4 + DuckDuckGo/NewTabPageViewController.swift | 18 +++- .../NewTabPageControllerPixelTests.swift | 93 +++++++++++++++++++ .../NewTabPageMessagesModelTests.swift | 2 +- 4 files changed, 112 insertions(+), 5 deletions(-) create mode 100644 DuckDuckGoTests/NewTabPageControllerPixelTests.swift diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 6f8db18707..13a6703bd6 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -366,6 +366,7 @@ 6FEC0B852C999352006B4F6E /* FavoriteItem.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6FEC0B842C999352006B4F6E /* FavoriteItem.swift */; }; 6FEC0B882C999961006B4F6E /* FavoritesListInteractingAdapter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6FEC0B872C999961006B4F6E /* FavoritesListInteractingAdapter.swift */; }; 6FF915822B88E07A0042AC87 /* AdAttributionFetcherTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6FF915802B88E0750042AC87 /* AdAttributionFetcherTests.swift */; }; + 6FF9AD452CE766F700C5A406 /* NewTabPageControllerPixelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6FF9AD442CE766F700C5A406 /* NewTabPageControllerPixelTests.swift */; }; 7B1604E82CB685B400A44EC6 /* Logger+TipKit.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7B1604E72CB685B400A44EC6 /* Logger+TipKit.swift */; }; 7B1604EC2CB68BDA00A44EC6 /* TipKitController+ConvenienceInitializers.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7B1604EB2CB68BDA00A44EC6 /* TipKitController+ConvenienceInitializers.swift */; }; 7B1604EE2CB68D2600A44EC6 /* TipKitDebugOptionsUIActionHandling.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7B1604ED2CB68D2600A44EC6 /* TipKitDebugOptionsUIActionHandling.swift */; }; @@ -1666,6 +1667,7 @@ 6FEC0B842C999352006B4F6E /* FavoriteItem.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FavoriteItem.swift; sourceTree = ""; }; 6FEC0B872C999961006B4F6E /* FavoritesListInteractingAdapter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FavoritesListInteractingAdapter.swift; sourceTree = ""; }; 6FF915802B88E0750042AC87 /* AdAttributionFetcherTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AdAttributionFetcherTests.swift; sourceTree = ""; }; + 6FF9AD442CE766F700C5A406 /* NewTabPageControllerPixelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NewTabPageControllerPixelTests.swift; sourceTree = ""; }; 7B1604E72CB685B400A44EC6 /* Logger+TipKit.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Logger+TipKit.swift"; sourceTree = ""; }; 7B1604EB2CB68BDA00A44EC6 /* TipKitController+ConvenienceInitializers.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TipKitController+ConvenienceInitializers.swift"; sourceTree = ""; }; 7B1604ED2CB68D2600A44EC6 /* TipKitDebugOptionsUIActionHandling.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TipKitDebugOptionsUIActionHandling.swift; sourceTree = ""; }; @@ -3834,6 +3836,7 @@ 6F03CAFF2C32ED22004179A8 /* NewTabPage */ = { isa = PBXGroup; children = ( + 6FF9AD442CE766F700C5A406 /* NewTabPageControllerPixelTests.swift */, 6F03CB002C32ED42004179A8 /* NewTabPageMessagesModelTests.swift */, 6F40D15C2C34436200BF22F0 /* HomePageDisplayDailyPixelBucketTests.swift */, 6F934F852C58DB00008364E4 /* NewTabPageSettingsPersistentStorageTests.swift */, @@ -8090,6 +8093,7 @@ 987130C8294AAB9F00AB05E0 /* BookmarksTestHelpers.swift in Sources */, 9F4CC51D2C48D240006A96EB /* CoreDataDatabaseTestUtilities.swift in Sources */, C185ED672BD43DA100BAE9DC /* ImportPasswordsStatusHandlerTests.swift in Sources */, + 6FF9AD452CE766F700C5A406 /* NewTabPageControllerPixelTests.swift in Sources */, F198D7981E3A45D90088DA8A /* WKWebViewConfigurationExtensionTests.swift in Sources */, 564DE45E2C45218500D23241 /* OnboardingNavigationDelegateTests.swift in Sources */, C14E2F7729DE14EA002AC515 /* AutofillInterfaceUsernameTruncatorTests.swift in Sources */, diff --git a/DuckDuckGo/NewTabPageViewController.swift b/DuckDuckGo/NewTabPageViewController.swift index b67eb9ac0f..02c0009e75 100644 --- a/DuckDuckGo/NewTabPageViewController.swift +++ b/DuckDuckGo/NewTabPageViewController.swift @@ -42,6 +42,8 @@ final class NewTabPageViewController: UIHostingController, NewTabPage { private weak var daxDialogViewController: DaxDialogViewController? private var daxDialogHeightConstraint: NSLayoutConstraint? + private let pixelFiring: PixelFiring.Type + var isDaxDialogVisible: Bool { daxDialogViewController?.view.isHidden == false } @@ -54,12 +56,14 @@ final class NewTabPageViewController: UIHostingController, NewTabPage { variantManager: VariantManager, newTabDialogFactory: any NewTabDaxDialogProvider, newTabDialogTypeProvider: NewTabDialogSpecProvider, - faviconLoader: FavoritesFaviconLoading) { + faviconLoader: FavoritesFaviconLoading, + pixelFiring: PixelFiring.Type = Pixel.self) { self.associatedTab = tab self.variantManager = variantManager self.newTabDialogFactory = newTabDialogFactory self.newTabDialogTypeProvider = newTabDialogTypeProvider + self.pixelFiring = pixelFiring newTabPageViewModel = NewTabPageViewModel() shortcutsSettingsModel = NewTabPageShortcutsSettingsModel() @@ -96,14 +100,20 @@ final class NewTabPageViewController: UIHostingController, NewTabPage { override func viewDidAppear(_ animated: Bool) { super.viewDidAppear(animated) + view.backgroundColor = UIColor(designSystemColor: .background) + + // If there's no tab switcher then this will be true, if there is a tabswitcher then only allow the + // stuff below to happen if it's being dismissed + guard presentedViewController?.isBeingDismissed ?? true else { + return + } + associatedTab.viewed = true presentNextDaxDialog() - Pixel.fire(pixel: .homeScreenShown) + pixelFiring.fire(.homeScreenShown, withAdditionalParameters: [:]) sendDailyDisplayPixel() - - view.backgroundColor = UIColor(designSystemColor: .background) } private func setUpDaxDialog() { diff --git a/DuckDuckGoTests/NewTabPageControllerPixelTests.swift b/DuckDuckGoTests/NewTabPageControllerPixelTests.swift new file mode 100644 index 0000000000..702ba0d5a2 --- /dev/null +++ b/DuckDuckGoTests/NewTabPageControllerPixelTests.swift @@ -0,0 +1,93 @@ +// +// NewTabPageControllerPixelTests.swift +// DuckDuckGo +// +// Copyright © 2024 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 Core +@testable import DuckDuckGo +import SwiftUICore + +final class NewTabPageControllerPixelTests: XCTestCase { + + override func setUp() { + super.setUp() + + PixelFiringMock.tearDown() + } + + override func tearDown() { + super.tearDown() + + PixelFiringMock.tearDown() + } + + func testHomeScreenPixelIsFiredOnAppear() { + let sut = createSUT() + sut.loadViewIfNeeded() + + sut.viewDidAppear(false) + + let count = PixelFiringMock.allPixelsFired.count { $0.pixelName == Pixel.Event.homeScreenShown.name } + + XCTAssertEqual(count, 1) + } + + func testHomeScreenPixelIsNotFired_WhenPresentingOtherController() { + let expectation = XCTestExpectation(description: "View loaded") + let sut = createSUT() + + let window = UIWindow(frame: UIScreen.main.bounds) + let presentedVC = UIViewController() + window.rootViewController = sut + window.makeKeyAndVisible() + window.rootViewController?.present(presentedVC, animated: false, completion: nil) + + DispatchQueue.main.async { + XCTAssertTrue(sut.isViewLoaded) + XCTAssertTrue(presentedVC.isViewLoaded) + XCTAssertNotNil(sut.presentedViewController) + expectation.fulfill() + } + + wait(for: [expectation], timeout: 1) + + sut.viewDidAppear(false) + + let count = PixelFiringMock.allPixelsFired.count { $0.pixelName == Pixel.Event.homeScreenShown.name } + + XCTAssertEqual(count, 0) + } + + private func createSUT() -> NewTabPageViewController { + NewTabPageViewController(tab: Tab(), + isNewTabPageCustomizationEnabled: false, + interactionModel: MockFavoritesListInteracting(), + homePageMessagesConfiguration: HomePageMessagesConfigurationMock(homeMessages: []), + variantManager: MockVariantManager(), + newTabDialogFactory: MockDaxDialogFactory(), + newTabDialogTypeProvider: MockNewTabDialogSpecProvider(), + faviconLoader: EmptyFaviconLoading(), + pixelFiring: PixelFiringMock.self) + } +} + +private class MockDaxDialogFactory: NewTabDaxDialogProvider { + func createDaxDialog(for homeDialog: DaxDialogs.HomeScreenSpec, onDismiss: @escaping () -> Void) -> EmptyView { + EmptyView() + } +} diff --git a/DuckDuckGoTests/NewTabPageMessagesModelTests.swift b/DuckDuckGoTests/NewTabPageMessagesModelTests.swift index d24d9edfa8..6ca484a5f1 100644 --- a/DuckDuckGoTests/NewTabPageMessagesModelTests.swift +++ b/DuckDuckGoTests/NewTabPageMessagesModelTests.swift @@ -250,7 +250,7 @@ final class NewTabPageMessagesModelTests: XCTestCase { } } -private class HomePageMessagesConfigurationMock: HomePageMessagesConfiguration { +class HomePageMessagesConfigurationMock: HomePageMessagesConfiguration { var homeMessages: [HomeMessage] init(homeMessages: [HomeMessage]) { From f1c14270036f24665c7f84a7dd2e15e7a5bc4c2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mariusz=20=C5=9Apiewak?= Date: Fri, 15 Nov 2024 13:47:24 +0100 Subject: [PATCH 2/3] Fix test compilation --- DuckDuckGoTests/NewTabPageControllerPixelTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DuckDuckGoTests/NewTabPageControllerPixelTests.swift b/DuckDuckGoTests/NewTabPageControllerPixelTests.swift index 702ba0d5a2..c3bc2f9c94 100644 --- a/DuckDuckGoTests/NewTabPageControllerPixelTests.swift +++ b/DuckDuckGoTests/NewTabPageControllerPixelTests.swift @@ -20,7 +20,7 @@ import XCTest import Core @testable import DuckDuckGo -import SwiftUICore +import SwiftUI final class NewTabPageControllerPixelTests: XCTestCase { From 2a6a343b9c5d740deb61456d0d2027c11902a170 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mariusz=20=C5=9Apiewak?= Date: Fri, 15 Nov 2024 13:56:15 +0100 Subject: [PATCH 3/3] Use available API --- DuckDuckGoTests/NewTabPageControllerPixelTests.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/DuckDuckGoTests/NewTabPageControllerPixelTests.swift b/DuckDuckGoTests/NewTabPageControllerPixelTests.swift index c3bc2f9c94..be66b23ad0 100644 --- a/DuckDuckGoTests/NewTabPageControllerPixelTests.swift +++ b/DuckDuckGoTests/NewTabPageControllerPixelTests.swift @@ -42,7 +42,7 @@ final class NewTabPageControllerPixelTests: XCTestCase { sut.viewDidAppear(false) - let count = PixelFiringMock.allPixelsFired.count { $0.pixelName == Pixel.Event.homeScreenShown.name } + let count = PixelFiringMock.allPixelsFired.filter { $0.pixelName == Pixel.Event.homeScreenShown.name }.count XCTAssertEqual(count, 1) } @@ -68,7 +68,7 @@ final class NewTabPageControllerPixelTests: XCTestCase { sut.viewDidAppear(false) - let count = PixelFiringMock.allPixelsFired.count { $0.pixelName == Pixel.Event.homeScreenShown.name } + let count = PixelFiringMock.allPixelsFired.filter { $0.pixelName == Pixel.Event.homeScreenShown.name }.count XCTAssertEqual(count, 0) }