From f322e89de49bfd2167d9ac681eb20154da83056d Mon Sep 17 00:00:00 2001 From: Bosco Ho Date: Tue, 10 Oct 2023 23:55:58 -0700 Subject: [PATCH] Made recent searches be stored per-account (#684) Commit: b24e9ca9287c62173d8367ca18955f0b307b0dcc [b24e9ca9] --- Mlem/Models/Saved Account.swift | 5 ++ .../Trackers/RecentSearchesTracker.swift | 51 +++++++++++-------- Mlem/Repositories/PersistenceRepository.swift | 11 ++-- .../Tabs/Search/RecentSearchesView.swift | 10 ++-- .../Tabs/Search/SearchResultListView.swift | 5 +- Mlem/Views/Tabs/Search/SearchView.swift | 24 ++++----- MlemTests/Model/LemmyURLTests.swift | 1 + .../PersistenceRepositoryTests.swift | 22 +++++--- 8 files changed, 76 insertions(+), 53 deletions(-) diff --git a/Mlem/Models/Saved Account.swift b/Mlem/Models/Saved Account.swift index 324639e36..705b518a8 100644 --- a/Mlem/Models/Saved Account.swift +++ b/Mlem/Models/Saved Account.swift @@ -15,6 +15,11 @@ struct SavedAccount: Identifiable, Codable, Equatable, Hashable { let storedNickname: String? let avatarUrl: URL? + var stableIdString: String { + assert(instanceLink.host() != nil, "nil instance link host!") + return "\(username)@\(instanceLink.host() ?? "unknown")" + } + init( id: Int, instanceLink: URL, diff --git a/Mlem/Models/Trackers/RecentSearchesTracker.swift b/Mlem/Models/Trackers/RecentSearchesTracker.swift index 55a1ad666..07d19f977 100644 --- a/Mlem/Models/Trackers/RecentSearchesTracker.swift +++ b/Mlem/Models/Trackers/RecentSearchesTracker.swift @@ -18,25 +18,31 @@ class RecentSearchesTracker: ObservableObject { var hasLoaded: Bool = false @Published var recentSearches: [AnyContentModel] = .init() - func loadRecentSearches() async throws { - hasLoaded = true - let identifiers = persistenceRepository.loadRecentSearches() - for id in identifiers { - print(id.contentType, id.contentId) - switch id.contentType { - case .post: - break - case .community: - let community: CommunityModel = try await communityRepository.loadDetails(for: id.contentId) - recentSearches.append(AnyContentModel(community)) - case .user: - let user = try await personRepository.loadDetails(for: id.contentId) - recentSearches.append(AnyContentModel(user)) + /// clears recentSearches and loads new values based on the current account + func reloadRecentSearches(accountId: String?) async throws { + defer { hasLoaded = true } + + recentSearches = .init() + if let accountId { + let identifiers = persistenceRepository.loadRecentSearches(for: accountId) + + for id in identifiers { + print(id.contentType, id.contentId) + switch id.contentType { + case .post: + break + case .community: + let community: CommunityModel = try await communityRepository.loadDetails(for: id.contentId) + recentSearches.append(AnyContentModel(community)) + case .user: + let user = try await personRepository.loadDetails(for: id.contentId) + recentSearches.append(AnyContentModel(user)) + } } } } - func addRecentSearch(_ item: AnyContentModel) { + func addRecentSearch(_ item: AnyContentModel, accountId: String?) { // if the item is already in the recent list, move it to the top if let index = recentSearches.firstIndex(of: item) { recentSearches.remove(at: index) @@ -49,17 +55,20 @@ class RecentSearchesTracker: ObservableObject { recentSearches = recentSearches.dropLast(1) } } - saveRecentSearches() + saveRecentSearches(accountId: accountId) } - func clearRecentSearches() { + func clearRecentSearches(accountId: String?) { recentSearches.removeAll() - saveRecentSearches() + saveRecentSearches(accountId: accountId) } - private func saveRecentSearches() { - Task(priority: .background) { - try await persistenceRepository.saveRecentSearches(recentSearches.map { $0.uid }) + private func saveRecentSearches(accountId: String?) { + if let accountId { + print("saving searches for \(accountId)") + Task(priority: .background) { + try await persistenceRepository.saveRecentSearches(for: accountId, with: recentSearches.map(\.uid)) + } } } } diff --git a/Mlem/Repositories/PersistenceRepository.swift b/Mlem/Repositories/PersistenceRepository.swift index 91d43a222..ff31048e3 100644 --- a/Mlem/Repositories/PersistenceRepository.swift +++ b/Mlem/Repositories/PersistenceRepository.swift @@ -83,12 +83,15 @@ class PersistenceRepository { try await save(value, to: Path.savedAccounts) } - func loadRecentSearches() -> [ContentModelIdentifier] { - load([ContentModelIdentifier].self, from: Path.recentSearches) ?? [] + func loadRecentSearches(for accountId: String) -> [ContentModelIdentifier] { + let searches = load([String: [ContentModelIdentifier]].self, from: Path.recentSearches) ?? [:] + return searches[accountId] ?? [] } - func saveRecentSearches(_ value: [ContentModelIdentifier]) async throws { - try await save(value, to: Path.recentSearches) + func saveRecentSearches(for accountId: String, with searches: [ContentModelIdentifier]) async throws { + var extant = load([String: [ContentModelIdentifier]].self, from: Path.recentSearches) ?? [:] + extant[accountId] = searches + try await save(extant, to: Path.recentSearches) } func loadFavoriteCommunities() -> [FavoriteCommunity] { diff --git a/Mlem/Views/Tabs/Search/RecentSearchesView.swift b/Mlem/Views/Tabs/Search/RecentSearchesView.swift index 2c4f5ff23..d14dd36ab 100644 --- a/Mlem/Views/Tabs/Search/RecentSearchesView.swift +++ b/Mlem/Views/Tabs/Search/RecentSearchesView.swift @@ -8,7 +8,7 @@ import SwiftUI struct RecentSearchesView: View { - + @EnvironmentObject var appState: AppState @EnvironmentObject var recentSearchesTracker: RecentSearchesTracker @StateObject var contentTracker: ContentTracker = .init() @@ -17,8 +17,8 @@ struct RecentSearchesView: View { if !recentSearchesTracker.recentSearches.isEmpty { VStack(alignment: .leading, spacing: 0) { headerView - .padding(.top, 15) - .padding(.bottom, 6) + .padding(.top, 15) + .padding(.bottom, 6) Divider() itemsView } @@ -47,7 +47,7 @@ struct RecentSearchesView: View { Spacer() Button { - recentSearchesTracker.clearRecentSearches() + recentSearchesTracker.clearRecentSearches(accountId: appState.currentActiveAccount?.stableIdString) } label: { Text("Clear") .font(.subheadline) @@ -68,7 +68,7 @@ struct RecentSearchesView: View { } } .simultaneousGesture(TapGesture().onEnded { - recentSearchesTracker.addRecentSearch(contentModel) + recentSearchesTracker.addRecentSearch(contentModel, accountId: appState.currentActiveAccount?.stableIdString) }) Divider() } diff --git a/Mlem/Views/Tabs/Search/SearchResultListView.swift b/Mlem/Views/Tabs/Search/SearchResultListView.swift index 498278d87..7c83c3b96 100644 --- a/Mlem/Views/Tabs/Search/SearchResultListView.swift +++ b/Mlem/Views/Tabs/Search/SearchResultListView.swift @@ -8,6 +8,7 @@ import SwiftUI struct SearchResultListView: View { + @EnvironmentObject var appState: AppState @EnvironmentObject var recentSearchesTracker: RecentSearchesTracker @EnvironmentObject var contentTracker: ContentTracker @@ -26,7 +27,7 @@ struct SearchResultListView: View { } } .simultaneousGesture(TapGesture().onEnded { - recentSearchesTracker.addRecentSearch(contentModel) + recentSearchesTracker.addRecentSearch(contentModel, accountId: appState.currentActiveAccount?.stableIdString) }) Divider() .onAppear { @@ -54,7 +55,7 @@ struct SearchResultListView: View { } else if contentTracker.items.isEmpty { Text("No results found.") .foregroundStyle(.secondary) - } else if contentTracker.hasReachedEnd && contentTracker.items.count > 10 { + } else if contentTracker.hasReachedEnd, contentTracker.items.count > 10 { HStack { Image(systemName: "figure.climbing") Text("I think I've found the bottom!") diff --git a/Mlem/Views/Tabs/Search/SearchView.swift b/Mlem/Views/Tabs/Search/SearchView.swift index c7d8d8379..287471ea6 100644 --- a/Mlem/Views/Tabs/Search/SearchView.swift +++ b/Mlem/Views/Tabs/Search/SearchView.swift @@ -5,11 +5,11 @@ // Created by Jake Shirley on 7/5/23. // -import Dependencies import Combine +import Dependencies import Foundation -import UIKit import SwiftUI +import UIKit private struct ViewOffsetKey: PreferenceKey { typealias Value = CGFloat @@ -27,6 +27,7 @@ struct SearchView: View { } // environment + @EnvironmentObject var appState: AppState @EnvironmentObject private var recentSearchesTracker: RecentSearchesTracker @StateObject var searchModel: SearchModel @@ -54,19 +55,16 @@ struct SearchView: View { page = .home searchModel.searchText = "" } - - } + } .autocorrectionDisabled(true) .textInputAutocapitalization(.never) .onAppear { Task(priority: .background) { - if !recentSearchesTracker.hasLoaded { - do { - try await recentSearchesTracker.loadRecentSearches() - } catch { - print("Error while loading recent searches: \(error.localizedDescription)") - errorHandler.handle(error) - } + do { + try await recentSearchesTracker.reloadRecentSearches(accountId: appState.currentActiveAccount?.stableIdString) + } catch { + print("Error while loading recent searches: \(error.localizedDescription)") + errorHandler.handle(error) } } } @@ -93,8 +91,8 @@ struct SearchView: View { .animation(.default, value: page) } .onChange(of: isSearching) { newValue in - if newValue && searchModel.searchText.isEmpty { - page = .recents + if newValue, searchModel.searchText.isEmpty { + page = .recents } } .onChange(of: searchModel.searchText) { newValue in diff --git a/MlemTests/Model/LemmyURLTests.swift b/MlemTests/Model/LemmyURLTests.swift index 864597df5..693bf685e 100644 --- a/MlemTests/Model/LemmyURLTests.swift +++ b/MlemTests/Model/LemmyURLTests.swift @@ -17,6 +17,7 @@ final class LemmyURLTests: XCTestCase { XCTAssertEqual(lemmyUrl?.url.absoluteString, validUrl) } + // NOTE: this test fails on XCode 15+ func testHandlesUnencodedURL() throws { let unencodedUrl = "https://matrix.to/#/#space:lemmy.world" let lemmyUrl = LemmyURL(string: unencodedUrl) diff --git a/MlemTests/Persistence/PersistenceRepositoryTests.swift b/MlemTests/Persistence/PersistenceRepositoryTests.swift index 9219cfd29..eebc54156 100644 --- a/MlemTests/Persistence/PersistenceRepositoryTests.swift +++ b/MlemTests/Persistence/PersistenceRepositoryTests.swift @@ -122,24 +122,30 @@ final class PersistenceRepositoryTests: XCTestCase { func testSaveRecentSearches() async throws { let searches: [ContentModelIdentifier] = [.init(contentType: .user, contentId: 1), .init(contentType: .community, contentId: 2)] - try await repository.saveRecentSearches(searches) // write the examples to disk - let searchesFromDisk = try load([ContentModelIdentifier].self) // load them from the disk _without_ using the repository + try await repository.saveRecentSearches(for: "one@test", with: searches) // write the examples to disk + let searchesFromDisk = try load([String: [ContentModelIdentifier]].self) // load them from the disk _without_ using the repository - XCTAssertEqual(searches, searchesFromDisk) // confirm what was written to disk matches what we sent in + let expected: [String: [ContentModelIdentifier]] = ["one@test": searches] + XCTAssertEqual(expected, searchesFromDisk) // confirm what was written to disk matches what we sent in } func testLoadRecentSearchesWithValues() async throws { - let searches: [ContentModelIdentifier] = [.init(contentType: .user, contentId: 1), .init(contentType: .community, contentId: 2)] + let searches1: [ContentModelIdentifier] = [.init(contentType: .user, contentId: 1), .init(contentType: .community, contentId: 2)] + let searches2: [ContentModelIdentifier] = [.init(contentType: .user, contentId: 2), .init(contentType: .community, contentId: 3)] + + try await repository.saveRecentSearches(for: "one@test", with: searches1) + try await repository.saveRecentSearches(for: "two@test", with: searches2) - try await repository.saveRecentSearches(searches) // write the example terms to the disk - let loadedSearches = repository.loadRecentSearches() // read them back + let loadedSearches1 = repository.loadRecentSearches(for: "one@test") // read them back + let loadedSearches2 = repository.loadRecentSearches(for: "two@test") - XCTAssertEqual(loadedSearches, searches) // assert we were given the same values back + XCTAssertEqual(loadedSearches1, searches1) // assert we were given the same values back + XCTAssertEqual(loadedSearches2, searches2) } func testLoadRecentSearchesWithoutValues() async throws { XCTAssert(disk.isEmpty) // assert that our mock disk has nothing in it - let loadedSearches = repository.loadRecentSearches() // perform a load knowing the disk is empty + let loadedSearches = repository.loadRecentSearches(for: "one@test") // perform a load knowing the disk is empty XCTAssert(loadedSearches.isEmpty) // assert we were returned an empty list }