From 5a6b0de96b14aec867ca0b9544dfd4fb0256fab8 Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Thu, 4 Apr 2024 16:41:55 +1100 Subject: [PATCH] Fix an issue that causes URLs to be shown in the search term popover --- Sources/Suggestions/Suggestion.swift | 9 +++++ .../Suggestions/SuggestionProcessing.swift | 18 ++++++++- .../SuggestionProcessingTests.swift | 40 +++++++++++++++++-- 3 files changed, 63 insertions(+), 4 deletions(-) diff --git a/Sources/Suggestions/Suggestion.swift b/Sources/Suggestions/Suggestion.swift index a5ecc191d..f1b5865b5 100644 --- a/Sources/Suggestions/Suggestion.swift +++ b/Sources/Suggestions/Suggestion.swift @@ -48,6 +48,15 @@ public enum Suggestion: Equatable { } } + var phrase: String? { + switch self { + case .phrase(let phrase): + return phrase + case .website, .bookmark, .historyEntry, .unknown: + return nil + } + } + public var allowedInTopHits: Bool { switch self { case .website: diff --git a/Sources/Suggestions/SuggestionProcessing.swift b/Sources/Suggestions/SuggestionProcessing.swift index c139a117e..f3f8f2406 100644 --- a/Sources/Suggestions/SuggestionProcessing.swift +++ b/Sources/Suggestions/SuggestionProcessing.swift @@ -61,6 +61,9 @@ final class SuggestionProcessing { navigationalSuggestions = merge(navigationalSuggestions, maximum: maximumOfNavigationalSuggestions) + // Filter out navigational link from initial suggestions. + let filteredDuckDuckGoSuggestions = remove(navigationalSuggestions: navigationalSuggestions, fromDuckDuckGoSuggestions: duckDuckGoSuggestions) + // Split the Top Hits and the History and Bookmarks section let topHits = topHits(from: navigationalSuggestions) let historyAndBookmarkSuggestions = Array(navigationalSuggestions.dropFirst(topHits.count).filter { suggestion in @@ -73,7 +76,7 @@ final class SuggestionProcessing { }) return makeResult(topHits: topHits, - duckduckgoSuggestions: duckDuckGoSuggestions, + duckduckgoSuggestions: filteredDuckDuckGoSuggestions, historyAndBookmarks: historyAndBookmarkSuggestions) } @@ -273,4 +276,17 @@ final class SuggestionProcessing { historyAndBookmarks: historyAndBookmarks) } + // MARK: - Filter out search term URL that are Navigational suggestion + + private func remove(navigationalSuggestions: [Suggestion], fromDuckDuckGoSuggestions suggestions: [Suggestion]) -> [Suggestion] { + let navigationalSuggestionsURLs = Set(navigationalSuggestions.map(\.url?.naked)) + return suggestions.filter { suggestion in + guard let phrase = suggestion.phrase else { return false } + // If the URL can't be built return true as this means that the `phrase` is a search term. + guard let suggestionURL = urlFactory(phrase) else { return true } + // Compare the URL and return false if it's a match + return !navigationalSuggestionsURLs.contains(suggestionURL.naked) + } + } + } diff --git a/Tests/SuggestionsTests/SuggestionProcessingTests.swift b/Tests/SuggestionsTests/SuggestionProcessingTests.swift index b75f8aa65..8c88bb023 100644 --- a/Tests/SuggestionsTests/SuggestionProcessingTests.swift +++ b/Tests/SuggestionsTests/SuggestionProcessingTests.swift @@ -35,6 +35,25 @@ final class SuggestionProcessingTests: XCTestCase { XCTAssertEqual(result!.topHits.first!.title, "DuckDuckGo") } + func testWhenDuckDuckGoSuggestionContainsURLThenDoNotShowAsSearchTerm() throws { + // GIVEN + let processing = SuggestionProcessing(urlFactory: URL.makeURL(fromSuggestionPhrase:)) + let facebookURLSearchTermSuggestion = Suggestion(key: Suggestion.phraseKey, value: "www.acer.com/ac/en/US/content/home") + + // WHEN + let result = processing.result( + for: "ace", + from: [], + bookmarks: [], + apiResult: .aceAPIResult + ) + + // THEN + let duckduckGoSuggestions = try XCTUnwrap(result?.duckduckgoSuggestions) + XCTAssertEqual(duckduckGoSuggestions.count, 4) + XCTAssertFalse(duckduckGoSuggestions.contains(facebookURLSearchTermSuggestion)) + } + } extension HistoryEntryMock { @@ -55,9 +74,12 @@ extension HistoryEntryMock { extension BookmarkMock { static var someBookmarks: [Bookmark] { - [ BookmarkMock(url: "http://duckduckgo.com", title: "DuckDuckGo", isFavorite: true), - BookmarkMock(url: "spreadprivacy.com", title: "Test 2", isFavorite: true), - BookmarkMock(url: "wikipedia.org", title: "Wikipedia", isFavorite: false) ] + [ + BookmarkMock(url: "http://duckduckgo.com", title: "DuckDuckGo", isFavorite: true), + BookmarkMock(url: "spreadprivacy.com", title: "Test 2", isFavorite: true), + BookmarkMock(url: "wikipedia.org", title: "Wikipedia", isFavorite: false), + BookmarkMock(url: "www.facebook.com", title: "Facebook", isFavorite: true), + ] } } @@ -74,4 +96,16 @@ extension APIResult { return result } + static let aceAPIResult: APIResult = { + var result = APIResult() + result.items = [ + [ "phrase": "acecqa" ], + [ "phrase": "acer" ], + [ "phrase": "www.acer.com/ac/en/US/content/home" ], + [ "phrase": "ace hotel sydney" ], + [ "phrase": "acer drivers" ], + ] + return result + }() + }