From 1661ef5ea5fbbe51924a30b712042a7daf1ba9e8 Mon Sep 17 00:00:00 2001 From: Juan Manuel Pereira Date: Mon, 22 Jul 2024 15:01:15 -0300 Subject: [PATCH 1/7] Define pixels --- DuckDuckGo/Statistics/GeneralPixel.swift | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/DuckDuckGo/Statistics/GeneralPixel.swift b/DuckDuckGo/Statistics/GeneralPixel.swift index ba770856b4..ce87cbd408 100644 --- a/DuckDuckGo/Statistics/GeneralPixel.swift +++ b/DuckDuckGo/Statistics/GeneralPixel.swift @@ -367,6 +367,13 @@ enum GeneralPixel: PixelKitEventV2 { case bookmarksMigrationCouldNotRemoveOldStore case bookmarksMigrationCouldNotPrepareMultipleFavoriteFolders + // Bookmarks search and sort feature metrics + case bookmarksSortButtonClicked(origin: String) + case bookmarksSortButtonDismissed(origin: String) + case bookmarksSortByName(origin: String) + case bookmarksSearchExecuted(origin: String) + case bookmarksSearchResultClicked(origin: String) + case syncSentUnauthenticatedRequest case syncMetadataCouldNotLoadDatabase case syncBookmarksProviderInitializationFailed @@ -1009,6 +1016,13 @@ enum GeneralPixel: PixelKitEventV2 { case .secureVaultKeystoreEventL2KeyPasswordMigration: return "m_mac_secure_vault_keystore_event_l2-key-password-migration" case .compilationFailed: return "compilation_failed" + + // Bookmarks search and sort feature + case .bookmarksSortButtonClicked: return "m_mac_sort_bookmarks_button_clicked" + case .bookmarksSortButtonDismissed: return "m_mac_sort_bookmarks_button_dismissed" + case .bookmarksSortByName: return "m_mac_sort_bookmarks_by_name" + case .bookmarksSearchExecuted: return "m_mac_search_bookmarks_executed" + case .bookmarksSearchResultClicked: return "m_mac_search_result_clicked" } } @@ -1111,6 +1125,13 @@ enum GeneralPixel: PixelKitEventV2 { case .onboardingDuckplayerUsed5to7(let cohort): return [PixelKit.Parameters.experimentCohort: cohort] + case .bookmarksSortButtonClicked(let origin), + .bookmarksSortButtonDismissed(let origin), + .bookmarksSortByName(let origin), + .bookmarksSearchExecuted(let origin), + .bookmarksSearchResultClicked(let origin): + return ["origin": origin] + default: return nil } } From e8e154114d79352277fe50eac129f3f7ae257d81 Mon Sep 17 00:00:00 2001 From: Juan Manuel Pereira Date: Mon, 22 Jul 2024 15:15:38 -0300 Subject: [PATCH 2/7] Create bookmarks metric class --- DuckDuckGo.xcodeproj/project.pbxproj | 6 ++ .../Bookmarks/Model/BookmarksMetrics.swift | 57 +++++++++++++++++++ 2 files changed, 63 insertions(+) create mode 100644 DuckDuckGo/Bookmarks/Model/BookmarksMetrics.swift diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 5d2a9ebfab..98c19415ec 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -2533,6 +2533,8 @@ B6FA893F269C424500588ECD /* PrivacyDashboardViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6FA893E269C424500588ECD /* PrivacyDashboardViewController.swift */; }; B6FA8941269C425400588ECD /* PrivacyDashboardPopover.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6FA8940269C425400588ECD /* PrivacyDashboardPopover.swift */; }; BB5789722B2CA70F0009DFE2 /* DataBrokerProtectionSubscriptionEventHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = BB5789712B2CA70F0009DFE2 /* DataBrokerProtectionSubscriptionEventHandler.swift */; }; + BB7B5F982C4ED73800BA4AF8 /* BookmarksMetrics.swift in Sources */ = {isa = PBXBuildFile; fileRef = BB7B5F972C4ED73800BA4AF8 /* BookmarksMetrics.swift */; }; + BB7B5F992C4ED73800BA4AF8 /* BookmarksMetrics.swift in Sources */ = {isa = PBXBuildFile; fileRef = BB7B5F972C4ED73800BA4AF8 /* BookmarksMetrics.swift */; }; BBB881882C4029BA001247C6 /* BookmarkListTreeControllerSearchDataSource.swift in Sources */ = {isa = PBXBuildFile; fileRef = BBB881872C4029BA001247C6 /* BookmarkListTreeControllerSearchDataSource.swift */; }; BBB881892C4029BA001247C6 /* BookmarkListTreeControllerSearchDataSource.swift in Sources */ = {isa = PBXBuildFile; fileRef = BBB881872C4029BA001247C6 /* BookmarkListTreeControllerSearchDataSource.swift */; }; BBDFDC5A2B2B8A0900F62D90 /* DataBrokerProtectionExternalWaitlistPixels.swift in Sources */ = {isa = PBXBuildFile; fileRef = BBDFDC592B2B8A0900F62D90 /* DataBrokerProtectionExternalWaitlistPixels.swift */; }; @@ -4215,6 +4217,7 @@ B6FA893E269C424500588ECD /* PrivacyDashboardViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PrivacyDashboardViewController.swift; sourceTree = ""; }; B6FA8940269C425400588ECD /* PrivacyDashboardPopover.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PrivacyDashboardPopover.swift; sourceTree = ""; }; BB5789712B2CA70F0009DFE2 /* DataBrokerProtectionSubscriptionEventHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DataBrokerProtectionSubscriptionEventHandler.swift; sourceTree = ""; }; + BB7B5F972C4ED73800BA4AF8 /* BookmarksMetrics.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarksMetrics.swift; sourceTree = ""; }; BBB881872C4029BA001247C6 /* BookmarkListTreeControllerSearchDataSource.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarkListTreeControllerSearchDataSource.swift; sourceTree = ""; }; BBDFDC592B2B8A0900F62D90 /* DataBrokerProtectionExternalWaitlistPixels.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DataBrokerProtectionExternalWaitlistPixels.swift; sourceTree = ""; }; BBFB727E2C48047C0088884C /* SortBookmarksViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SortBookmarksViewModel.swift; sourceTree = ""; }; @@ -7597,6 +7600,7 @@ B6F9BDDB2B45B7EE00677B33 /* WebsiteInfo.swift */, 9F872DA22B90920F00138637 /* BookmarkFolderInfo.swift */, BBB881872C4029BA001247C6 /* BookmarkListTreeControllerSearchDataSource.swift */, + BB7B5F972C4ED73800BA4AF8 /* BookmarksMetrics.swift */, ); path = Model; sourceTree = ""; @@ -9878,6 +9882,7 @@ 3706FA93293F65D500E42796 /* WKWebView+Download.swift in Sources */, 3706FA94293F65D500E42796 /* TabShadowConfig.swift in Sources */, 3706FA97293F65D500E42796 /* WindowDraggingView.swift in Sources */, + BB7B5F992C4ED73800BA4AF8 /* BookmarksMetrics.swift in Sources */, B60D644A2AAF1B7C00B26F50 /* AddressBarTextSelectionNavigation.swift in Sources */, 1D01A3D52B88CF7700FE8150 /* AccessibilityPreferences.swift in Sources */, 3706FA98293F65D500E42796 /* SecureVaultSorting.swift in Sources */, @@ -11323,6 +11328,7 @@ AA80EC54256BE3BC007083E7 /* UserText.swift in Sources */, B61EF3EC266F91E700B4D78F /* WKWebView+Download.swift in Sources */, 311B262728E73E0A00FD181A /* TabShadowConfig.swift in Sources */, + BB7B5F982C4ED73800BA4AF8 /* BookmarksMetrics.swift in Sources */, B6BCC54A2AFDF24B002C5499 /* TaskWithProgress.swift in Sources */, B6DB3AEF278D5C370024C5C4 /* URLSessionExtension.swift in Sources */, 4B7A60A1273E0BE400BBDFEB /* WKWebsiteDataStoreExtension.swift in Sources */, diff --git a/DuckDuckGo/Bookmarks/Model/BookmarksMetrics.swift b/DuckDuckGo/Bookmarks/Model/BookmarksMetrics.swift new file mode 100644 index 0000000000..e15cdaee01 --- /dev/null +++ b/DuckDuckGo/Bookmarks/Model/BookmarksMetrics.swift @@ -0,0 +1,57 @@ +// +// BookmarksMetrics.swift +// +// 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 Foundation +import PixelKit + +protocol BookmarksMetricsProtocol { + func fireSortButtonClicked(origin: BookmarkOperationOrigin) + func fireSortButtonDismissed(origin: BookmarkOperationOrigin) + func fireSortByName(origin: BookmarkOperationOrigin) + func fireSearchExecuted(origin: BookmarkOperationOrigin) + func fireSearchResultClicked(origin: BookmarkOperationOrigin) +} + +enum BookmarkOperationOrigin: String { + case panel + case manager +} + +struct BookmarksMetrics: BookmarksMetricsProtocol { + private let pixelKit = PixelKit.shared + + func fireSortButtonClicked(origin: BookmarkOperationOrigin) { + pixelKit?.fire(GeneralPixel.bookmarksSortButtonClicked(origin: origin.rawValue)) + } + + func fireSortButtonDismissed(origin: BookmarkOperationOrigin) { + pixelKit?.fire(GeneralPixel.bookmarksSortButtonDismissed(origin: origin.rawValue)) + } + + func fireSortByName(origin: BookmarkOperationOrigin) { + pixelKit?.fire(GeneralPixel.bookmarksSortByName(origin: origin.rawValue)) + } + + func fireSearchExecuted(origin: BookmarkOperationOrigin) { + pixelKit?.fire(GeneralPixel.bookmarksSearchExecuted(origin: origin.rawValue)) + } + + func fireSearchResultClicked(origin: BookmarkOperationOrigin) { + pixelKit?.fire(GeneralPixel.bookmarksSearchResultClicked(origin: origin.rawValue)) + } +} From 0df6b607506093efded9dab09a24f0e990f3b0cf Mon Sep 17 00:00:00 2001 From: Juan Manuel Pereira Date: Mon, 22 Jul 2024 15:33:41 -0300 Subject: [PATCH 3/7] Implement search metrics and add unit tests --- DuckDuckGo.xcodeproj/project.pbxproj | 6 + .../Bookmarks/Model/BookmarksMetrics.swift | 10 +- .../View/BookmarkListViewController.swift | 23 ++- .../ViewModel/SortBookmarksViewModel.swift | 42 +++++- .../SortBookmarksViewModelTests.swift | 138 ++++++++++++++++++ 5 files changed, 202 insertions(+), 17 deletions(-) create mode 100644 UnitTests/Bookmarks/ViewModels/SortBookmarksViewModelTests.swift diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 98c19415ec..97bfd045f1 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -2537,6 +2537,8 @@ BB7B5F992C4ED73800BA4AF8 /* BookmarksMetrics.swift in Sources */ = {isa = PBXBuildFile; fileRef = BB7B5F972C4ED73800BA4AF8 /* BookmarksMetrics.swift */; }; BBB881882C4029BA001247C6 /* BookmarkListTreeControllerSearchDataSource.swift in Sources */ = {isa = PBXBuildFile; fileRef = BBB881872C4029BA001247C6 /* BookmarkListTreeControllerSearchDataSource.swift */; }; BBB881892C4029BA001247C6 /* BookmarkListTreeControllerSearchDataSource.swift in Sources */ = {isa = PBXBuildFile; fileRef = BBB881872C4029BA001247C6 /* BookmarkListTreeControllerSearchDataSource.swift */; }; + BBBEE1BF2C4FF63600035ABA /* SortBookmarksViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = BBBEE1BE2C4FF63600035ABA /* SortBookmarksViewModelTests.swift */; }; + BBBEE1C02C4FF63600035ABA /* SortBookmarksViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = BBBEE1BE2C4FF63600035ABA /* SortBookmarksViewModelTests.swift */; }; BBDFDC5A2B2B8A0900F62D90 /* DataBrokerProtectionExternalWaitlistPixels.swift in Sources */ = {isa = PBXBuildFile; fileRef = BBDFDC592B2B8A0900F62D90 /* DataBrokerProtectionExternalWaitlistPixels.swift */; }; BBDFDC5D2B2B8E2100F62D90 /* DataBrokerProtectionExternalWaitlistPixels.swift in Sources */ = {isa = PBXBuildFile; fileRef = BBDFDC592B2B8A0900F62D90 /* DataBrokerProtectionExternalWaitlistPixels.swift */; }; BBFB727F2C48047C0088884C /* SortBookmarksViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = BBFB727E2C48047C0088884C /* SortBookmarksViewModel.swift */; }; @@ -4219,6 +4221,7 @@ BB5789712B2CA70F0009DFE2 /* DataBrokerProtectionSubscriptionEventHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DataBrokerProtectionSubscriptionEventHandler.swift; sourceTree = ""; }; BB7B5F972C4ED73800BA4AF8 /* BookmarksMetrics.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarksMetrics.swift; sourceTree = ""; }; BBB881872C4029BA001247C6 /* BookmarkListTreeControllerSearchDataSource.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarkListTreeControllerSearchDataSource.swift; sourceTree = ""; }; + BBBEE1BE2C4FF63600035ABA /* SortBookmarksViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SortBookmarksViewModelTests.swift; sourceTree = ""; }; BBDFDC592B2B8A0900F62D90 /* DataBrokerProtectionExternalWaitlistPixels.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DataBrokerProtectionExternalWaitlistPixels.swift; sourceTree = ""; }; BBFB727E2C48047C0088884C /* SortBookmarksViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SortBookmarksViewModel.swift; sourceTree = ""; }; BBFF355C2C4AF26200DA3289 /* BookmarksSortModeTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarksSortModeTests.swift; sourceTree = ""; }; @@ -6664,6 +6667,7 @@ 9F0FFFB32BCCAE37007C87DD /* BookmarkAllTabsDialogCoordinatorViewModelTests.swift */, 9FA5A0A82BC900FC00153786 /* BookmarkAllTabsDialogViewModelTests.swift */, BBFF355C2C4AF26200DA3289 /* BookmarksSortModeTests.swift */, + BBBEE1BE2C4FF63600035ABA /* SortBookmarksViewModelTests.swift */, ); path = ViewModels; sourceTree = ""; @@ -10781,6 +10785,7 @@ 56A0542E2C201DAA007D8FAB /* MockContentBlocking.swift in Sources */, 3706FE1F293F661700E42796 /* AppStateChangePublisherTests.swift in Sources */, 9FA5A0B12BC9039300153786 /* BookmarkFolderStoreMock.swift in Sources */, + BBBEE1C02C4FF63600035ABA /* SortBookmarksViewModelTests.swift in Sources */, 9F0660792BECC81C00B8EEF1 /* PixelCapturedParameters.swift in Sources */, 3706FE20293F661700E42796 /* CLLocationManagerMock.swift in Sources */, B6656E0E2B29C733008798A1 /* FileImportViewLocalizationTests.swift in Sources */, @@ -12257,6 +12262,7 @@ F1B8EC7A2C29957A00D395F5 /* SubscriptionFeatureAvailabilityMock.swift in Sources */, 9F872DA02B90644800138637 /* ContextualMenuTests.swift in Sources */, 4B9292BE2667103100AD2C21 /* PasteboardFolderTests.swift in Sources */, + BBBEE1BF2C4FF63600035ABA /* SortBookmarksViewModelTests.swift in Sources */, 4B9292C52667104B00AD2C21 /* CoreDataTestUtilities.swift in Sources */, 4B723E1926B000DC00E14D75 /* TemporaryFileCreator.swift in Sources */, 98EB5D1027516A4800681FE6 /* AppPrivacyConfigurationTests.swift in Sources */, diff --git a/DuckDuckGo/Bookmarks/Model/BookmarksMetrics.swift b/DuckDuckGo/Bookmarks/Model/BookmarksMetrics.swift index e15cdaee01..29b05e41af 100644 --- a/DuckDuckGo/Bookmarks/Model/BookmarksMetrics.swift +++ b/DuckDuckGo/Bookmarks/Model/BookmarksMetrics.swift @@ -38,19 +38,19 @@ struct BookmarksMetrics: BookmarksMetricsProtocol { func fireSortButtonClicked(origin: BookmarkOperationOrigin) { pixelKit?.fire(GeneralPixel.bookmarksSortButtonClicked(origin: origin.rawValue)) } - + func fireSortButtonDismissed(origin: BookmarkOperationOrigin) { pixelKit?.fire(GeneralPixel.bookmarksSortButtonDismissed(origin: origin.rawValue)) } - + func fireSortByName(origin: BookmarkOperationOrigin) { pixelKit?.fire(GeneralPixel.bookmarksSortByName(origin: origin.rawValue)) } - + func fireSearchExecuted(origin: BookmarkOperationOrigin) { - pixelKit?.fire(GeneralPixel.bookmarksSearchExecuted(origin: origin.rawValue)) + pixelKit?.fire(GeneralPixel.bookmarksSearchExecuted(origin: origin.rawValue), frequency: .daily) } - + func fireSearchResultClicked(origin: BookmarkOperationOrigin) { pixelKit?.fire(GeneralPixel.bookmarksSearchResultClicked(origin: origin.rawValue)) } diff --git a/DuckDuckGo/Bookmarks/View/BookmarkListViewController.swift b/DuckDuckGo/Bookmarks/View/BookmarkListViewController.swift index dd977760f3..48f5febe89 100644 --- a/DuckDuckGo/Bookmarks/View/BookmarkListViewController.swift +++ b/DuckDuckGo/Bookmarks/View/BookmarkListViewController.swift @@ -93,7 +93,8 @@ final class BookmarkListViewController: NSViewController { private let bookmarkManager: BookmarkManager private let treeControllerDataSource: BookmarkListTreeControllerDataSource private let treeControllerSearchDataSource: BookmarkListTreeControllerSearchDataSource - private let sortBookmarksViewModel = SortBookmarksViewModel() + private let sortBookmarksViewModel: SortBookmarksViewModel + private let bookmarkMetrics: BookmarksMetricsProtocol private lazy var treeController = BookmarkTreeController(dataSource: treeControllerDataSource, sortMode: sortBookmarksViewModel.selectedSortMode, @@ -132,10 +133,13 @@ final class BookmarkListViewController: NSViewController { return .init(syncService: syncService, syncBookmarksAdapter: syncBookmarksAdapter) }() - init(bookmarkManager: BookmarkManager = LocalBookmarkManager.shared) { + init(bookmarkManager: BookmarkManager = LocalBookmarkManager.shared, + metrics: BookmarksMetricsProtocol = BookmarksMetrics()) { self.bookmarkManager = bookmarkManager self.treeControllerDataSource = BookmarkListTreeControllerDataSource(bookmarkManager: bookmarkManager) self.treeControllerSearchDataSource = BookmarkListTreeControllerSearchDataSource(bookmarkManager: bookmarkManager) + self.bookmarkMetrics = metrics + self.sortBookmarksViewModel = SortBookmarksViewModel(metrics: metrics, origin: .panel) super.init(nibName: nil, bundle: nil) } @@ -482,6 +486,8 @@ final class BookmarkListViewController: NSViewController { @objc func sortBookmarksButtonClicked(_ sender: NSButton) { let menu = sortBookmarksViewModel.selectedSortMode.menu + bookmarkMetrics.fireSortButtonClicked(origin: .panel) + menu.delegate = sortBookmarksViewModel menu.popUpAtMouseLocation(in: sortBookmarksButton) } @@ -527,6 +533,7 @@ final class BookmarkListViewController: NSViewController { let bookmark = node.representedObject as? Bookmark { onBookmarkClick(bookmark) } else if let node = item as? BookmarkNode, let folder = node.representedObject as? BookmarkFolder, dataSource.isSearching { + bookmarkMetrics.fireSearchResultClicked(origin: .panel) hideSearchBar() updateSearchAndExpand(folder) } else { @@ -535,6 +542,10 @@ final class BookmarkListViewController: NSViewController { } private func onBookmarkClick(_ bookmark: Bookmark) { + if dataSource.isSearching { + bookmarkMetrics.fireSearchResultClicked(origin: .panel) + } + WindowControllersManager.shared.open(bookmark: bookmark) delegate?.popoverShouldClose(self) } @@ -875,15 +886,15 @@ extension BookmarkListViewController: BookmarkSearchMenuItemSelectors { extension BookmarkListViewController: BookmarkSortMenuItemSelectors { func manualSort(_ sender: NSMenuItem) { - sortBookmarksViewModel.selectedSortMode = .manual + sortBookmarksViewModel.setSort(mode: .manual) } func sortByNameAscending(_ sender: NSMenuItem) { - sortBookmarksViewModel.selectedSortMode = .nameAscending + sortBookmarksViewModel.setSort(mode: .nameAscending) } func sortByNameDescending(_ sender: NSMenuItem) { - sortBookmarksViewModel.selectedSortMode = .nameDescending + sortBookmarksViewModel.setSort(mode: .nameDescending) } } @@ -900,6 +911,8 @@ extension BookmarkListViewController: NSSearchFieldDelegate { } else { showSearch(for: searchQuery) } + + bookmarkMetrics.fireSearchExecuted(origin: .panel) } } diff --git a/DuckDuckGo/Bookmarks/ViewModel/SortBookmarksViewModel.swift b/DuckDuckGo/Bookmarks/ViewModel/SortBookmarksViewModel.swift index 0884c46f15..21dd3da8b2 100644 --- a/DuckDuckGo/Bookmarks/ViewModel/SortBookmarksViewModel.swift +++ b/DuckDuckGo/Bookmarks/ViewModel/SortBookmarksViewModel.swift @@ -80,6 +80,10 @@ enum BookmarksSortMode: Codable { } } + var isNameSorting: Bool { + return self == .nameAscending || self == .nameDescending + } + private func menuItem(for mode: BookmarksSortMode, state: NSControl.StateValue, disabled: Bool = false) -> NSMenuItem { return NSMenuItem(title: mode.title, action: disabled ? nil : mode.action, state: state) } @@ -123,20 +127,44 @@ final class SortBookmarksUserDefaults: SortBookmarksRepository { } } -final class SortBookmarksViewModel { +final class SortBookmarksViewModel: NSObject { + private let metrics: BookmarksMetricsProtocol + private let origin: BookmarkOperationOrigin private var repository: SortBookmarksRepository @Published - var selectedSortMode: BookmarksSortMode = .manual { - didSet { - repository.storedSortMode = selectedSortMode + private(set) var selectedSortMode: BookmarksSortMode = .manual + private var wasSortOptionSelected = false + + init(repository: SortBookmarksRepository = SortBookmarksUserDefaults(), + metrics: BookmarksMetricsProtocol, + origin: BookmarkOperationOrigin) { + self.metrics = metrics + self.origin = origin + self.repository = repository + + selectedSortMode = repository.storedSortMode + } + + func setSort(mode: BookmarksSortMode) { + wasSortOptionSelected = true + selectedSortMode = mode + repository.storedSortMode = selectedSortMode + + if mode.isNameSorting { + metrics.fireSortByName(origin: origin) } } +} - init(repository: SortBookmarksRepository = SortBookmarksUserDefaults()) { - self.repository = repository +extension SortBookmarksViewModel: NSMenuDelegate { - selectedSortMode = repository.storedSortMode + func menuDidClose(_ menu: NSMenu) { + if !wasSortOptionSelected { + metrics.fireSortButtonDismissed(origin: origin) + } + + wasSortOptionSelected = false } } diff --git a/UnitTests/Bookmarks/ViewModels/SortBookmarksViewModelTests.swift b/UnitTests/Bookmarks/ViewModels/SortBookmarksViewModelTests.swift new file mode 100644 index 0000000000..5cc2485437 --- /dev/null +++ b/UnitTests/Bookmarks/ViewModels/SortBookmarksViewModelTests.swift @@ -0,0 +1,138 @@ +// +// SortBookmarksViewModelTests.swift +// +// 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 +@testable import DuckDuckGo_Privacy_Browser + +final class MockBookmarksMetrics: BookmarksMetricsProtocol { + var wasSortButtonClickedFired = false + var wasSortButtonDismissedFired = false + var wasSortByNameFired = false + var wasSearchExecutedFired = false + var wasSearchResultClickedFired = false + + func fireSortButtonClicked(origin: DuckDuckGo_Privacy_Browser.BookmarkOperationOrigin) { + wasSortButtonClickedFired = true + } + + func fireSortButtonDismissed(origin: DuckDuckGo_Privacy_Browser.BookmarkOperationOrigin) { + wasSortButtonDismissedFired = true + } + + func fireSortByName(origin: DuckDuckGo_Privacy_Browser.BookmarkOperationOrigin) { + wasSortByNameFired = true + } + + func fireSearchExecuted(origin: DuckDuckGo_Privacy_Browser.BookmarkOperationOrigin) { + wasSearchExecutedFired = true + } + + func fireSearchResultClicked(origin: DuckDuckGo_Privacy_Browser.BookmarkOperationOrigin) { + wasSearchResultClickedFired = true + } +} + +final class MockSortBookmarksRepository: SortBookmarksRepository { + var storedSortMode: BookmarksSortMode + + init(storedSortMode: BookmarksSortMode = .manual) { + self.storedSortMode = storedSortMode + } +} + +class SortBookmarksViewModelTests: XCTestCase { + + let repository = MockSortBookmarksRepository() + + func testWhenSortingIsNameAscending_thenSortByNameMetricIsFired() { + let metrics = MockBookmarksMetrics() + let sut = SortBookmarksViewModel(repository: repository, metrics: metrics, origin: .panel) + + sut.setSort(mode: .nameAscending) + + XCTAssertTrue(metrics.wasSortByNameFired) + } + + func testWhenSortingIsNameDescending_thenSortByNameMetricIsFired() { + let metrics = MockBookmarksMetrics() + let sut = SortBookmarksViewModel(repository: repository, metrics: metrics, origin: .panel) + + sut.setSort(mode: .nameDescending) + + XCTAssertTrue(metrics.wasSortByNameFired) + } + + func testWhenSortingIsManual_thenSortByNameMetricIsNotFired() { + let metrics = MockBookmarksMetrics() + let sut = SortBookmarksViewModel(repository: repository, metrics: metrics, origin: .panel) + + sut.setSort(mode: .manual) + + XCTAssertFalse(metrics.wasSortByNameFired) + } + + func testWhenSortingIsManual_thenIsSavedToRepository() { + let metrics = MockBookmarksMetrics() + let sut = SortBookmarksViewModel(repository: repository, metrics: metrics, origin: .panel) + + sut.setSort(mode: .manual) + + XCTAssertEqual(repository.storedSortMode, .manual) + } + + func testWhenSortingIsNameAscending_thenIsSavedToRepository() { + let metrics = MockBookmarksMetrics() + let repository = MockSortBookmarksRepository() + let sut = SortBookmarksViewModel(repository: repository, metrics: metrics, origin: .panel) + + sut.setSort(mode: .nameAscending) + + XCTAssertEqual(repository.storedSortMode, .nameAscending) + } + + func testWhenSortingIsNameDescending_thenIsSavedToRepository() { + let metrics = MockBookmarksMetrics() + let repository = MockSortBookmarksRepository() + let sut = SortBookmarksViewModel(repository: repository, metrics: metrics, origin: .panel) + + sut.setSort(mode: .nameDescending) + + XCTAssertEqual(repository.storedSortMode, .nameDescending) + } + + func testWhenMenuIsClosedAndNoOptionWasSelected_thenSortButtonDismissedIsFired() { + let metrics = MockBookmarksMetrics() + let repository = MockSortBookmarksRepository() + let sut = SortBookmarksViewModel(repository: repository, metrics: metrics, origin: .panel) + + sut.menuDidClose(NSMenu()) + + XCTAssertTrue(metrics.wasSortButtonDismissedFired) + } + + func testWhenMenuIsClosedAndOptionWasSelected_thenSortButtonDismissedIsNotFired() { + let metrics = MockBookmarksMetrics() + let repository = MockSortBookmarksRepository() + let sut = SortBookmarksViewModel(repository: repository, metrics: metrics, origin: .panel) + + sut.setSort(mode: .nameDescending) + sut.menuDidClose(NSMenu()) + + XCTAssertFalse(metrics.wasSortButtonDismissedFired) + } +} From 942b7e9a1dc9c24d8c2c94884e0a785b13041d77 Mon Sep 17 00:00:00 2001 From: Juan Manuel Pereira Date: Tue, 23 Jul 2024 16:10:14 -0300 Subject: [PATCH 4/7] Remove abstraction for metrics --- .../Bookmarks/Model/BookmarksMetrics.swift | 22 +-- .../View/BookmarkListViewController.swift | 4 +- .../ViewModel/SortBookmarksViewModel.swift | 4 +- .../SortBookmarksViewModelTests.swift | 131 ++++++++++-------- 4 files changed, 82 insertions(+), 79 deletions(-) diff --git a/DuckDuckGo/Bookmarks/Model/BookmarksMetrics.swift b/DuckDuckGo/Bookmarks/Model/BookmarksMetrics.swift index 29b05e41af..8e3908e448 100644 --- a/DuckDuckGo/Bookmarks/Model/BookmarksMetrics.swift +++ b/DuckDuckGo/Bookmarks/Model/BookmarksMetrics.swift @@ -19,39 +19,29 @@ import Foundation import PixelKit -protocol BookmarksMetricsProtocol { - func fireSortButtonClicked(origin: BookmarkOperationOrigin) - func fireSortButtonDismissed(origin: BookmarkOperationOrigin) - func fireSortByName(origin: BookmarkOperationOrigin) - func fireSearchExecuted(origin: BookmarkOperationOrigin) - func fireSearchResultClicked(origin: BookmarkOperationOrigin) -} - enum BookmarkOperationOrigin: String { case panel case manager } -struct BookmarksMetrics: BookmarksMetricsProtocol { - private let pixelKit = PixelKit.shared - +struct BookmarksMetrics { func fireSortButtonClicked(origin: BookmarkOperationOrigin) { - pixelKit?.fire(GeneralPixel.bookmarksSortButtonClicked(origin: origin.rawValue)) + PixelKit.fire(GeneralPixel.bookmarksSortButtonClicked(origin: origin.rawValue)) } func fireSortButtonDismissed(origin: BookmarkOperationOrigin) { - pixelKit?.fire(GeneralPixel.bookmarksSortButtonDismissed(origin: origin.rawValue)) + PixelKit.fire(GeneralPixel.bookmarksSortButtonDismissed(origin: origin.rawValue)) } func fireSortByName(origin: BookmarkOperationOrigin) { - pixelKit?.fire(GeneralPixel.bookmarksSortByName(origin: origin.rawValue)) + PixelKit.fire(GeneralPixel.bookmarksSortByName(origin: origin.rawValue)) } func fireSearchExecuted(origin: BookmarkOperationOrigin) { - pixelKit?.fire(GeneralPixel.bookmarksSearchExecuted(origin: origin.rawValue), frequency: .daily) + PixelKit.fire(GeneralPixel.bookmarksSearchExecuted(origin: origin.rawValue), frequency: .daily) } func fireSearchResultClicked(origin: BookmarkOperationOrigin) { - pixelKit?.fire(GeneralPixel.bookmarksSearchResultClicked(origin: origin.rawValue)) + PixelKit.fire(GeneralPixel.bookmarksSearchResultClicked(origin: origin.rawValue)) } } diff --git a/DuckDuckGo/Bookmarks/View/BookmarkListViewController.swift b/DuckDuckGo/Bookmarks/View/BookmarkListViewController.swift index 48f5febe89..1a5f1d487a 100644 --- a/DuckDuckGo/Bookmarks/View/BookmarkListViewController.swift +++ b/DuckDuckGo/Bookmarks/View/BookmarkListViewController.swift @@ -94,7 +94,7 @@ final class BookmarkListViewController: NSViewController { private let treeControllerDataSource: BookmarkListTreeControllerDataSource private let treeControllerSearchDataSource: BookmarkListTreeControllerSearchDataSource private let sortBookmarksViewModel: SortBookmarksViewModel - private let bookmarkMetrics: BookmarksMetricsProtocol + private let bookmarkMetrics: BookmarksMetrics private lazy var treeController = BookmarkTreeController(dataSource: treeControllerDataSource, sortMode: sortBookmarksViewModel.selectedSortMode, @@ -134,7 +134,7 @@ final class BookmarkListViewController: NSViewController { }() init(bookmarkManager: BookmarkManager = LocalBookmarkManager.shared, - metrics: BookmarksMetricsProtocol = BookmarksMetrics()) { + metrics: BookmarksMetrics = BookmarksMetrics()) { self.bookmarkManager = bookmarkManager self.treeControllerDataSource = BookmarkListTreeControllerDataSource(bookmarkManager: bookmarkManager) self.treeControllerSearchDataSource = BookmarkListTreeControllerSearchDataSource(bookmarkManager: bookmarkManager) diff --git a/DuckDuckGo/Bookmarks/ViewModel/SortBookmarksViewModel.swift b/DuckDuckGo/Bookmarks/ViewModel/SortBookmarksViewModel.swift index 21dd3da8b2..755a2c9f41 100644 --- a/DuckDuckGo/Bookmarks/ViewModel/SortBookmarksViewModel.swift +++ b/DuckDuckGo/Bookmarks/ViewModel/SortBookmarksViewModel.swift @@ -129,7 +129,7 @@ final class SortBookmarksUserDefaults: SortBookmarksRepository { final class SortBookmarksViewModel: NSObject { - private let metrics: BookmarksMetricsProtocol + private let metrics: BookmarksMetrics private let origin: BookmarkOperationOrigin private var repository: SortBookmarksRepository @@ -138,7 +138,7 @@ final class SortBookmarksViewModel: NSObject { private var wasSortOptionSelected = false init(repository: SortBookmarksRepository = SortBookmarksUserDefaults(), - metrics: BookmarksMetricsProtocol, + metrics: BookmarksMetrics, origin: BookmarkOperationOrigin) { self.metrics = metrics self.origin = origin diff --git a/UnitTests/Bookmarks/ViewModels/SortBookmarksViewModelTests.swift b/UnitTests/Bookmarks/ViewModels/SortBookmarksViewModelTests.swift index 5cc2485437..8b78914f46 100644 --- a/UnitTests/Bookmarks/ViewModels/SortBookmarksViewModelTests.swift +++ b/UnitTests/Bookmarks/ViewModels/SortBookmarksViewModelTests.swift @@ -17,36 +17,10 @@ // import XCTest +import PixelKitTestingUtilities +@testable import PixelKit @testable import DuckDuckGo_Privacy_Browser -final class MockBookmarksMetrics: BookmarksMetricsProtocol { - var wasSortButtonClickedFired = false - var wasSortButtonDismissedFired = false - var wasSortByNameFired = false - var wasSearchExecutedFired = false - var wasSearchResultClickedFired = false - - func fireSortButtonClicked(origin: DuckDuckGo_Privacy_Browser.BookmarkOperationOrigin) { - wasSortButtonClickedFired = true - } - - func fireSortButtonDismissed(origin: DuckDuckGo_Privacy_Browser.BookmarkOperationOrigin) { - wasSortButtonDismissedFired = true - } - - func fireSortByName(origin: DuckDuckGo_Privacy_Browser.BookmarkOperationOrigin) { - wasSortByNameFired = true - } - - func fireSearchExecuted(origin: DuckDuckGo_Privacy_Browser.BookmarkOperationOrigin) { - wasSearchExecutedFired = true - } - - func fireSearchResultClicked(origin: DuckDuckGo_Privacy_Browser.BookmarkOperationOrigin) { - wasSearchResultClickedFired = true - } -} - final class MockSortBookmarksRepository: SortBookmarksRepository { var storedSortMode: BookmarksSortMode @@ -56,38 +30,32 @@ final class MockSortBookmarksRepository: SortBookmarksRepository { } class SortBookmarksViewModelTests: XCTestCase { - + let testUserDefault = UserDefaults(suiteName: #function)! let repository = MockSortBookmarksRepository() + let metrics = BookmarksMetrics() - func testWhenSortingIsNameAscending_thenSortByNameMetricIsFired() { - let metrics = MockBookmarksMetrics() + func testWhenSortingIsNameAscending_thenSortByNameMetricIsFired() async throws { let sut = SortBookmarksViewModel(repository: repository, metrics: metrics, origin: .panel) + let expectedPixel = GeneralPixel.bookmarksSortByName(origin: "panel") - sut.setSort(mode: .nameAscending) - - XCTAssertTrue(metrics.wasSortByNameFired) + try await verify(expectedPixel: expectedPixel, for: { sut.setSort(mode: .nameAscending) }) } - func testWhenSortingIsNameDescending_thenSortByNameMetricIsFired() { - let metrics = MockBookmarksMetrics() + func testWhenSortingIsNameDescending_thenSortByNameMetricIsFired() async throws { let sut = SortBookmarksViewModel(repository: repository, metrics: metrics, origin: .panel) + let expectedPixel = GeneralPixel.bookmarksSortByName(origin: "panel") - sut.setSort(mode: .nameDescending) - - XCTAssertTrue(metrics.wasSortByNameFired) + try await verify(expectedPixel: expectedPixel, for: { sut.setSort(mode: .nameDescending) }) } - func testWhenSortingIsManual_thenSortByNameMetricIsNotFired() { - let metrics = MockBookmarksMetrics() + func testWhenSortingIsManual_thenSortByNameMetricIsNotFired() async throws { let sut = SortBookmarksViewModel(repository: repository, metrics: metrics, origin: .panel) + let notExpectedPixel = GeneralPixel.bookmarksSortByName(origin: "panel") - sut.setSort(mode: .manual) - - XCTAssertFalse(metrics.wasSortByNameFired) + try await verifyNotFired(pixel: notExpectedPixel, for: { sut.setSort(mode: .manual) }) } func testWhenSortingIsManual_thenIsSavedToRepository() { - let metrics = MockBookmarksMetrics() let sut = SortBookmarksViewModel(repository: repository, metrics: metrics, origin: .panel) sut.setSort(mode: .manual) @@ -96,7 +64,6 @@ class SortBookmarksViewModelTests: XCTestCase { } func testWhenSortingIsNameAscending_thenIsSavedToRepository() { - let metrics = MockBookmarksMetrics() let repository = MockSortBookmarksRepository() let sut = SortBookmarksViewModel(repository: repository, metrics: metrics, origin: .panel) @@ -106,7 +73,6 @@ class SortBookmarksViewModelTests: XCTestCase { } func testWhenSortingIsNameDescending_thenIsSavedToRepository() { - let metrics = MockBookmarksMetrics() let repository = MockSortBookmarksRepository() let sut = SortBookmarksViewModel(repository: repository, metrics: metrics, origin: .panel) @@ -115,24 +81,71 @@ class SortBookmarksViewModelTests: XCTestCase { XCTAssertEqual(repository.storedSortMode, .nameDescending) } - func testWhenMenuIsClosedAndNoOptionWasSelected_thenSortButtonDismissedIsFired() { - let metrics = MockBookmarksMetrics() - let repository = MockSortBookmarksRepository() + @MainActor + func testWhenMenuIsClosedAndNoOptionWasSelected_thenSortButtonDismissedIsFired() async throws { let sut = SortBookmarksViewModel(repository: repository, metrics: metrics, origin: .panel) + let expectedPixel = GeneralPixel.bookmarksSortButtonDismissed(origin: "panel") - sut.menuDidClose(NSMenu()) - - XCTAssertTrue(metrics.wasSortButtonDismissedFired) + try await verify(expectedPixel: expectedPixel, for: { sut.menuDidClose(NSMenu()) }) } - func testWhenMenuIsClosedAndOptionWasSelected_thenSortButtonDismissedIsNotFired() { - let metrics = MockBookmarksMetrics() - let repository = MockSortBookmarksRepository() + @MainActor + func testWhenMenuIsClosedAndOptionWasSelected_thenSortButtonDismissedIsNotFired() async throws { let sut = SortBookmarksViewModel(repository: repository, metrics: metrics, origin: .panel) + let notExpectedPixel = GeneralPixel.bookmarksSortButtonDismissed(origin: "panel") - sut.setSort(mode: .nameDescending) - sut.menuDidClose(NSMenu()) + try await verifyNotFired(pixel: notExpectedPixel, for: { + sut.setSort(mode: .manual) + sut.menuDidClose(NSMenu()) + }) + } + + // MARK: - Pixel testing helper methods + + private func verify(expectedPixel: GeneralPixel, for code: () -> Void) async throws { + let pixelExpectation = expectation(description: "Pixel fired") + try await verify(pixel: expectedPixel, for: code, expectation: pixelExpectation) { + await fulfillment(of: [pixelExpectation], timeout: 1.0) + } + } + + private func verifyNotFired(pixel: GeneralPixel, for code: () -> Void) async throws { + let pixelExpectation = expectation(description: "Pixel not fired") + try await verify(pixel: pixel, for: code, expectation: pixelExpectation) { + let result = await XCTWaiter().fulfillment(of: [pixelExpectation], timeout: 1) + + if result == .timedOut { + pixelExpectation.fulfill() + } else { + XCTFail("Pixel was fired") + } + } + } + + private func verify(pixel: GeneralPixel, for code: () -> Void, expectation: XCTestExpectation, verification: () async -> Void) async throws { + let pixelKit = createPixelKit(pixelNamePrefix: pixel.name, pixelExpectation: expectation) + + PixelKit.setSharedForTesting(pixelKit: pixelKit) + + code() + await verification() + + cleanUp(pixelKit: pixelKit) + } + + private func createPixelKit(pixelNamePrefix: String, pixelExpectation: XCTestExpectation) -> PixelKit { + return PixelKit(dryRun: false, + appVersion: "1.0.0", + defaultHeaders: [:], + defaults: testUserDefault) { pixelName, _, _, _, _, _ in + if pixelName.hasPrefix(pixelNamePrefix) { + pixelExpectation.fulfill() + } + } + } - XCTAssertFalse(metrics.wasSortButtonDismissedFired) + private func cleanUp(pixelKit: PixelKit) { + PixelKit.tearDown() + pixelKit.clearFrequencyHistoryForAllPixels() } } From 6b270122e3d4ca78273ac0a8456563f0056820a3 Mon Sep 17 00:00:00 2001 From: Juan Manuel Pereira Date: Tue, 23 Jul 2024 16:12:49 -0300 Subject: [PATCH 5/7] Rename BookmarkMetrics --- DuckDuckGo.xcodeproj/project.pbxproj | 12 ++++++------ ...ics.swift => BookmarksSearchAndSortMetrics.swift} | 4 ++-- .../Bookmarks/View/BookmarkListViewController.swift | 4 ++-- .../Bookmarks/ViewModel/SortBookmarksViewModel.swift | 4 ++-- .../ViewModels/SortBookmarksViewModelTests.swift | 2 +- 5 files changed, 13 insertions(+), 13 deletions(-) rename DuckDuckGo/Bookmarks/Model/{BookmarksMetrics.swift => BookmarksSearchAndSortMetrics.swift} (95%) diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 97bfd045f1..deb1f11609 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -2533,8 +2533,8 @@ B6FA893F269C424500588ECD /* PrivacyDashboardViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6FA893E269C424500588ECD /* PrivacyDashboardViewController.swift */; }; B6FA8941269C425400588ECD /* PrivacyDashboardPopover.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6FA8940269C425400588ECD /* PrivacyDashboardPopover.swift */; }; BB5789722B2CA70F0009DFE2 /* DataBrokerProtectionSubscriptionEventHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = BB5789712B2CA70F0009DFE2 /* DataBrokerProtectionSubscriptionEventHandler.swift */; }; - BB7B5F982C4ED73800BA4AF8 /* BookmarksMetrics.swift in Sources */ = {isa = PBXBuildFile; fileRef = BB7B5F972C4ED73800BA4AF8 /* BookmarksMetrics.swift */; }; - BB7B5F992C4ED73800BA4AF8 /* BookmarksMetrics.swift in Sources */ = {isa = PBXBuildFile; fileRef = BB7B5F972C4ED73800BA4AF8 /* BookmarksMetrics.swift */; }; + BB7B5F982C4ED73800BA4AF8 /* BookmarksSearchAndSortMetrics.swift in Sources */ = {isa = PBXBuildFile; fileRef = BB7B5F972C4ED73800BA4AF8 /* BookmarksSearchAndSortMetrics.swift */; }; + BB7B5F992C4ED73800BA4AF8 /* BookmarksSearchAndSortMetrics.swift in Sources */ = {isa = PBXBuildFile; fileRef = BB7B5F972C4ED73800BA4AF8 /* BookmarksSearchAndSortMetrics.swift */; }; BBB881882C4029BA001247C6 /* BookmarkListTreeControllerSearchDataSource.swift in Sources */ = {isa = PBXBuildFile; fileRef = BBB881872C4029BA001247C6 /* BookmarkListTreeControllerSearchDataSource.swift */; }; BBB881892C4029BA001247C6 /* BookmarkListTreeControllerSearchDataSource.swift in Sources */ = {isa = PBXBuildFile; fileRef = BBB881872C4029BA001247C6 /* BookmarkListTreeControllerSearchDataSource.swift */; }; BBBEE1BF2C4FF63600035ABA /* SortBookmarksViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = BBBEE1BE2C4FF63600035ABA /* SortBookmarksViewModelTests.swift */; }; @@ -4219,7 +4219,7 @@ B6FA893E269C424500588ECD /* PrivacyDashboardViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PrivacyDashboardViewController.swift; sourceTree = ""; }; B6FA8940269C425400588ECD /* PrivacyDashboardPopover.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PrivacyDashboardPopover.swift; sourceTree = ""; }; BB5789712B2CA70F0009DFE2 /* DataBrokerProtectionSubscriptionEventHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DataBrokerProtectionSubscriptionEventHandler.swift; sourceTree = ""; }; - BB7B5F972C4ED73800BA4AF8 /* BookmarksMetrics.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarksMetrics.swift; sourceTree = ""; }; + BB7B5F972C4ED73800BA4AF8 /* BookmarksSearchAndSortMetrics.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarksSearchAndSortMetrics.swift; sourceTree = ""; }; BBB881872C4029BA001247C6 /* BookmarkListTreeControllerSearchDataSource.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarkListTreeControllerSearchDataSource.swift; sourceTree = ""; }; BBBEE1BE2C4FF63600035ABA /* SortBookmarksViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SortBookmarksViewModelTests.swift; sourceTree = ""; }; BBDFDC592B2B8A0900F62D90 /* DataBrokerProtectionExternalWaitlistPixels.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DataBrokerProtectionExternalWaitlistPixels.swift; sourceTree = ""; }; @@ -7604,7 +7604,7 @@ B6F9BDDB2B45B7EE00677B33 /* WebsiteInfo.swift */, 9F872DA22B90920F00138637 /* BookmarkFolderInfo.swift */, BBB881872C4029BA001247C6 /* BookmarkListTreeControllerSearchDataSource.swift */, - BB7B5F972C4ED73800BA4AF8 /* BookmarksMetrics.swift */, + BB7B5F972C4ED73800BA4AF8 /* BookmarksSearchAndSortMetrics.swift */, ); path = Model; sourceTree = ""; @@ -9886,7 +9886,7 @@ 3706FA93293F65D500E42796 /* WKWebView+Download.swift in Sources */, 3706FA94293F65D500E42796 /* TabShadowConfig.swift in Sources */, 3706FA97293F65D500E42796 /* WindowDraggingView.swift in Sources */, - BB7B5F992C4ED73800BA4AF8 /* BookmarksMetrics.swift in Sources */, + BB7B5F992C4ED73800BA4AF8 /* BookmarksSearchAndSortMetrics.swift in Sources */, B60D644A2AAF1B7C00B26F50 /* AddressBarTextSelectionNavigation.swift in Sources */, 1D01A3D52B88CF7700FE8150 /* AccessibilityPreferences.swift in Sources */, 3706FA98293F65D500E42796 /* SecureVaultSorting.swift in Sources */, @@ -11333,7 +11333,7 @@ AA80EC54256BE3BC007083E7 /* UserText.swift in Sources */, B61EF3EC266F91E700B4D78F /* WKWebView+Download.swift in Sources */, 311B262728E73E0A00FD181A /* TabShadowConfig.swift in Sources */, - BB7B5F982C4ED73800BA4AF8 /* BookmarksMetrics.swift in Sources */, + BB7B5F982C4ED73800BA4AF8 /* BookmarksSearchAndSortMetrics.swift in Sources */, B6BCC54A2AFDF24B002C5499 /* TaskWithProgress.swift in Sources */, B6DB3AEF278D5C370024C5C4 /* URLSessionExtension.swift in Sources */, 4B7A60A1273E0BE400BBDFEB /* WKWebsiteDataStoreExtension.swift in Sources */, diff --git a/DuckDuckGo/Bookmarks/Model/BookmarksMetrics.swift b/DuckDuckGo/Bookmarks/Model/BookmarksSearchAndSortMetrics.swift similarity index 95% rename from DuckDuckGo/Bookmarks/Model/BookmarksMetrics.swift rename to DuckDuckGo/Bookmarks/Model/BookmarksSearchAndSortMetrics.swift index 8e3908e448..31d8fc3b6e 100644 --- a/DuckDuckGo/Bookmarks/Model/BookmarksMetrics.swift +++ b/DuckDuckGo/Bookmarks/Model/BookmarksSearchAndSortMetrics.swift @@ -1,5 +1,5 @@ // -// BookmarksMetrics.swift +// BookmarksSearchAndSortMetrics.swift // // Copyright © 2024 DuckDuckGo. All rights reserved. // @@ -24,7 +24,7 @@ enum BookmarkOperationOrigin: String { case manager } -struct BookmarksMetrics { +struct BookmarksSearchAndSortMetrics { func fireSortButtonClicked(origin: BookmarkOperationOrigin) { PixelKit.fire(GeneralPixel.bookmarksSortButtonClicked(origin: origin.rawValue)) } diff --git a/DuckDuckGo/Bookmarks/View/BookmarkListViewController.swift b/DuckDuckGo/Bookmarks/View/BookmarkListViewController.swift index 1a5f1d487a..86ec82f09b 100644 --- a/DuckDuckGo/Bookmarks/View/BookmarkListViewController.swift +++ b/DuckDuckGo/Bookmarks/View/BookmarkListViewController.swift @@ -94,7 +94,7 @@ final class BookmarkListViewController: NSViewController { private let treeControllerDataSource: BookmarkListTreeControllerDataSource private let treeControllerSearchDataSource: BookmarkListTreeControllerSearchDataSource private let sortBookmarksViewModel: SortBookmarksViewModel - private let bookmarkMetrics: BookmarksMetrics + private let bookmarkMetrics: BookmarksSearchAndSortMetrics private lazy var treeController = BookmarkTreeController(dataSource: treeControllerDataSource, sortMode: sortBookmarksViewModel.selectedSortMode, @@ -134,7 +134,7 @@ final class BookmarkListViewController: NSViewController { }() init(bookmarkManager: BookmarkManager = LocalBookmarkManager.shared, - metrics: BookmarksMetrics = BookmarksMetrics()) { + metrics: BookmarksSearchAndSortMetrics = BookmarksSearchAndSortMetrics()) { self.bookmarkManager = bookmarkManager self.treeControllerDataSource = BookmarkListTreeControllerDataSource(bookmarkManager: bookmarkManager) self.treeControllerSearchDataSource = BookmarkListTreeControllerSearchDataSource(bookmarkManager: bookmarkManager) diff --git a/DuckDuckGo/Bookmarks/ViewModel/SortBookmarksViewModel.swift b/DuckDuckGo/Bookmarks/ViewModel/SortBookmarksViewModel.swift index 755a2c9f41..c55d020b4a 100644 --- a/DuckDuckGo/Bookmarks/ViewModel/SortBookmarksViewModel.swift +++ b/DuckDuckGo/Bookmarks/ViewModel/SortBookmarksViewModel.swift @@ -129,7 +129,7 @@ final class SortBookmarksUserDefaults: SortBookmarksRepository { final class SortBookmarksViewModel: NSObject { - private let metrics: BookmarksMetrics + private let metrics: BookmarksSearchAndSortMetrics private let origin: BookmarkOperationOrigin private var repository: SortBookmarksRepository @@ -138,7 +138,7 @@ final class SortBookmarksViewModel: NSObject { private var wasSortOptionSelected = false init(repository: SortBookmarksRepository = SortBookmarksUserDefaults(), - metrics: BookmarksMetrics, + metrics: BookmarksSearchAndSortMetrics, origin: BookmarkOperationOrigin) { self.metrics = metrics self.origin = origin diff --git a/UnitTests/Bookmarks/ViewModels/SortBookmarksViewModelTests.swift b/UnitTests/Bookmarks/ViewModels/SortBookmarksViewModelTests.swift index 8b78914f46..92d34fb425 100644 --- a/UnitTests/Bookmarks/ViewModels/SortBookmarksViewModelTests.swift +++ b/UnitTests/Bookmarks/ViewModels/SortBookmarksViewModelTests.swift @@ -32,7 +32,7 @@ final class MockSortBookmarksRepository: SortBookmarksRepository { class SortBookmarksViewModelTests: XCTestCase { let testUserDefault = UserDefaults(suiteName: #function)! let repository = MockSortBookmarksRepository() - let metrics = BookmarksMetrics() + let metrics = BookmarksSearchAndSortMetrics() func testWhenSortingIsNameAscending_thenSortByNameMetricIsFired() async throws { let sut = SortBookmarksViewModel(repository: repository, metrics: metrics, origin: .panel) From a217b5abbc484fdb3fe626e5ba22ede94d2264f7 Mon Sep 17 00:00:00 2001 From: Juan Manuel Pereira Date: Tue, 23 Jul 2024 16:14:57 -0300 Subject: [PATCH 6/7] Refactor code --- .../Bookmarks/ViewModels/SortBookmarksViewModelTests.swift | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/UnitTests/Bookmarks/ViewModels/SortBookmarksViewModelTests.swift b/UnitTests/Bookmarks/ViewModels/SortBookmarksViewModelTests.swift index 92d34fb425..e3935dd54d 100644 --- a/UnitTests/Bookmarks/ViewModels/SortBookmarksViewModelTests.swift +++ b/UnitTests/Bookmarks/ViewModels/SortBookmarksViewModelTests.swift @@ -122,7 +122,10 @@ class SortBookmarksViewModelTests: XCTestCase { } } - private func verify(pixel: GeneralPixel, for code: () -> Void, expectation: XCTestExpectation, verification: () async -> Void) async throws { + private func verify(pixel: GeneralPixel, + for code: () -> Void, + expectation: XCTestExpectation, + verification: () async -> Void) async throws { let pixelKit = createPixelKit(pixelNamePrefix: pixel.name, pixelExpectation: expectation) PixelKit.setSharedForTesting(pixelKit: pixelKit) From 269848c64371f24dc2e686e3bf957706b51b6b02 Mon Sep 17 00:00:00 2001 From: Juan Manuel Pereira Date: Wed, 24 Jul 2024 10:11:22 -0300 Subject: [PATCH 7/7] Fix SwiftLint --- .../Bookmarks/ViewModels/SortBookmarksViewModelTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/UnitTests/Bookmarks/ViewModels/SortBookmarksViewModelTests.swift b/UnitTests/Bookmarks/ViewModels/SortBookmarksViewModelTests.swift index e3935dd54d..3c85fa0947 100644 --- a/UnitTests/Bookmarks/ViewModels/SortBookmarksViewModelTests.swift +++ b/UnitTests/Bookmarks/ViewModels/SortBookmarksViewModelTests.swift @@ -122,7 +122,7 @@ class SortBookmarksViewModelTests: XCTestCase { } } - private func verify(pixel: GeneralPixel, + private func verify(pixel: GeneralPixel, for code: () -> Void, expectation: XCTestExpectation, verification: () async -> Void) async throws {