Skip to content

Commit

Permalink
Allowing users to delete suggestions (#1027)
Browse files Browse the repository at this point in the history
Task/Issue URL:
https://app.asana.com/0/1148564399326804/1208219569168397/f

**Description**:
Improvements of the HistoryCoordinator to allow users to delete
individual suggestions
  • Loading branch information
tomasstrba authored Nov 3, 2024
1 parent 393d955 commit 80894bf
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 6 deletions.
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

0 comments on commit 80894bf

Please sign in to comment.