Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allowing users to delete suggestions #1027

Merged
merged 4 commits into from
Nov 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions Sources/History/HistoryCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ public protocol HistoryCoordinating: AnyObject {
func burnDomains(_ baseDomains: Set<String>, tld: TLD, completion: @escaping (Set<URL>) -> Void)
func burnVisits(_ visits: [Visit], completion: @escaping () -> Void)

func removeUrlEntry(_ url: URL, completion: ((Error?) -> Void)?)
}

/// Coordinates access to History. Uses its own queue with high qos for all operations.
Expand Down Expand Up @@ -191,6 +192,20 @@ final public class HistoryCoordinator: HistoryCoordinating {
}
}

public enum EntryRemovalError: Error {
case notAvailable
}

public func removeUrlEntry(_ url: URL, completion: ((Error?) -> Void)? = nil) {
guard let historyDictionary = historyDictionary else { return }
guard let entry = historyDictionary[url] else {
completion?(EntryRemovalError.notAvailable)
return
}

removeEntries([entry], completionHandler: completion)
}

var cleaningDate: Date { .monthAgo }

@objc private func cleanOld() {
Expand Down
12 changes: 9 additions & 3 deletions Sources/Suggestions/Suggestion.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public enum Suggestion: Equatable {
case openTab(title: String, url: URL)
case unknown(value: String)

var url: URL? {
public var url: URL? {
switch self {
case .website(url: let url),
.historyEntry(title: _, url: let url, allowedInTopHits: _),
Expand Down Expand Up @@ -67,20 +67,26 @@ public enum Suggestion: Equatable {
}
}

var isOpenTab: Bool {
public var isOpenTab: Bool {
if case .openTab = self {
return true
}
return false
}

var isBookmark: Bool {
public var isBookmark: Bool {
if case .bookmark = self {
return true
}
return false
}

public var isHistoryEntry: Bool {
if case .historyEntry = self {
return true
}
return false
}
}

extension Suggestion {
Expand Down
6 changes: 3 additions & 3 deletions Sources/Suggestions/SuggestionResult.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ public struct SuggestionResult: Equatable {
SuggestionResult(topHits: [], duckduckgoSuggestions: [], localSuggestions: [])
}

private(set) public var topHits: [Suggestion]
private(set) public var duckduckgoSuggestions: [Suggestion]
private(set) public var localSuggestions: [Suggestion]
public let topHits: [Suggestion]
public let duckduckgoSuggestions: [Suggestion]
public let localSuggestions: [Suggestion]

public init(topHits: [Suggestion],
duckduckgoSuggestions: [Suggestion],
Expand Down
37 changes: 37 additions & 0 deletions Tests/HistoryTests/HistoryCoordinatorTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,43 @@ class HistoryCoordinatorTests: XCTestCase {
return bookmarksDatabase
}

func testWhenRemoveUrlEntryCalledWithExistingUrl_ThenEntryIsRemovedAndNoError() {
let (historyStoringMock, historyCoordinator) = HistoryCoordinator.aHistoryCoordinator

let url = URL(string: "https://duckduckgo.com")!
historyCoordinator.addVisit(of: url)

XCTAssertTrue(historyCoordinator.history!.contains(where: { $0.url == url }))

let removalExpectation = expectation(description: "Entry removed without error")
historyCoordinator.removeUrlEntry(url) { error in
XCTAssertNil(error, "Expected no error when removing an existing URL entry")
removalExpectation.fulfill()
}

waitForExpectations(timeout: 1.0)

XCTAssertFalse(historyCoordinator.history!.contains(where: { $0.url == url }))
XCTAssertTrue(historyStoringMock.removeEntriesCalled, "Expected removeEntries to be called")
XCTAssertEqual(historyStoringMock.removeEntriesArray.count, 1)
XCTAssertEqual(historyStoringMock.removeEntriesArray.first?.url, url)
}

func testWhenRemoveUrlEntryCalledWithNonExistingUrl_ThenEntryRemovalFailsWithNotAvailableError() {
let (_, historyCoordinator) = HistoryCoordinator.aHistoryCoordinator

let nonExistentUrl = URL(string: "https://nonexistent.com")!

let removalExpectation = expectation(description: "Entry removal fails with notAvailable error")
historyCoordinator.removeUrlEntry(nonExistentUrl) { error in
XCTAssertNotNil(error, "Expected an error when removing a non-existent URL entry")
XCTAssertEqual(error as? HistoryCoordinator.EntryRemovalError, .notAvailable, "Expected notAvailable error")
removalExpectation.fulfill()
}

waitForExpectations(timeout: 1.0)
}

}

fileprivate extension HistoryCoordinator {
Expand Down
Loading