diff --git a/WordPress/Classes/Models/ReaderDefaultTopic.swift b/WordPress/Classes/Models/ReaderDefaultTopic.swift index c93a49944081..f96d33f5128d 100644 --- a/WordPress/Classes/Models/ReaderDefaultTopic.swift +++ b/WordPress/Classes/Models/ReaderDefaultTopic.swift @@ -4,4 +4,26 @@ import Foundation override open class var TopicType: String { return "default" } + + /// - note: Starting with 25.5, the app no longer uses the default topics + /// fetched from `/read/menu`. Instead, it uses the topics created locally. + /// The only reason these topics are needed is to filter entities in Core Data. + static func make(path: Path, in context: NSManagedObjectContext) throws -> ReaderDefaultTopic { + if let topic = context.firstObject(ofType: ReaderDefaultTopic.self, matching: NSPredicate(format: "path == %@", path.rawValue)) { + return topic + } + let topic = ReaderDefaultTopic(entity: ReaderDefaultTopic.entity(), insertInto: context) + topic.type = TopicType + topic.path = path.rawValue + topic.title = "" + try context.save() + return topic + } + + enum Path: String { + // These are arbitrary values just to identify the entites. + case recommended = "/jpios/discover/recommended" + case firstPosts = "/jpios/discover/first-posts" + case latest = "/jpios/discover/latest" + } } diff --git a/WordPress/Classes/Services/ReaderTopicService.h b/WordPress/Classes/Services/ReaderTopicService.h index 9569fdaab917..08d2917d2839 100644 --- a/WordPress/Classes/Services/ReaderTopicService.h +++ b/WordPress/Classes/Services/ReaderTopicService.h @@ -1,8 +1,6 @@ #import #import "CoreDataService.h" -extern NSString * const ReaderTopicFreshlyPressedPathCommponent; - @class ReaderAbstractTopic; @class ReaderTagTopic; @class ReaderSiteTopic; @@ -10,10 +8,6 @@ extern NSString * const ReaderTopicFreshlyPressedPathCommponent; @interface ReaderTopicService : CoreDataService -- (ReaderAbstractTopic *)currentTopicInContext:(NSManagedObjectContext *)context; - -- (void)setCurrentTopic:(ReaderAbstractTopic *)topic; - /** Fetches the topics for the reader's menu. diff --git a/WordPress/Classes/Services/ReaderTopicService.m b/WordPress/Classes/Services/ReaderTopicService.m index b69d1f04b059..be392a5c7ef9 100644 --- a/WordPress/Classes/Services/ReaderTopicService.m +++ b/WordPress/Classes/Services/ReaderTopicService.m @@ -10,9 +10,6 @@ @import WordPressKit; @import WordPressShared; -NSString * const ReaderTopicFreshlyPressedPathCommponent = @"freshly-pressed"; -static NSString * const ReaderTopicCurrentTopicPathKey = @"ReaderTopicCurrentTopicPathKey"; - @implementation ReaderTopicService - (void)fetchReaderMenuWithSuccess:(void (^)(void))success failure:(void (^)(NSError *error))failure @@ -43,73 +40,6 @@ - (void)fetchReaderMenuWithSuccess:(void (^)(void))success failure:(void (^)(NSE } completion:nil onQueue:dispatch_get_main_queue()]; } -- (ReaderAbstractTopic *)currentTopicInContext:(NSManagedObjectContext *)context -{ - ReaderAbstractTopic *topic; - topic = [self currentTopicFromSavedPathInContext:context]; - - if (!topic) { - topic = [self currentTopicFromDefaultTopicInContext:context]; - [self setCurrentTopic:topic]; - } - - return topic; -} - -- (ReaderAbstractTopic *)currentTopicFromSavedPathInContext:(NSManagedObjectContext *)context -{ - ReaderAbstractTopic *topic; - NSString *topicPathString = [[UserPersistentStoreFactory userDefaultsInstance] stringForKey:ReaderTopicCurrentTopicPathKey]; - if (topicPathString) { - NSFetchRequest *request = [NSFetchRequest fetchRequestWithEntityName:[ReaderAbstractTopic classNameWithoutNamespaces]]; - request.predicate = [NSPredicate predicateWithFormat:@"path = %@", topicPathString]; - - NSError *error; - topic = [[context executeFetchRequest:request error:&error] firstObject]; - if (error) { - DDLogError(@"%@ error fetching topic: %@", NSStringFromSelector(_cmd), error); - } - } - return topic; -} - -- (ReaderAbstractTopic *)currentTopicFromDefaultTopicInContext:(NSManagedObjectContext *)context -{ - // Return the default topic - ReaderAbstractTopic *topic; - NSError *error; - NSFetchRequest *request = [NSFetchRequest fetchRequestWithEntityName:[ReaderDefaultTopic classNameWithoutNamespaces]]; - NSSortDescriptor *sortDescriptor = [NSSortDescriptor sortDescriptorWithKey:@"title" ascending:YES]; - request.sortDescriptors = @[sortDescriptor]; - NSArray *topics = [context executeFetchRequest:request error:&error]; - if (error) { - DDLogError(@"%@ error fetching topic: %@", NSStringFromSelector(_cmd), error); - return nil; - } - - if ([topics count] == 0) { - return nil; - } - - NSArray *matches = [topics filteredArrayUsingPredicate:[NSPredicate predicateWithFormat:@"path CONTAINS[cd] %@", ReaderTopicFreshlyPressedPathCommponent]]; - if ([matches count]) { - topic = matches[0]; - } else { - topic = topics[0]; - } - - return topic; -} - -- (void)setCurrentTopic:(ReaderAbstractTopic *)topic -{ - if (!topic) { - [[UserPersistentStoreFactory userDefaultsInstance] removeObjectForKey:ReaderTopicCurrentTopicPathKey]; - } else { - [[UserPersistentStoreFactory userDefaultsInstance] setObject:topic.path forKey:ReaderTopicCurrentTopicPathKey]; - } -} - - (void)deleteAllSearchTopics { [self.coreDataStack performAndSaveUsingBlock:^(NSManagedObjectContext *context) { @@ -176,7 +106,6 @@ - (void)clearInUseFlags - (void)deleteAllTopics { [self.coreDataStack performAndSaveUsingBlock:^(NSManagedObjectContext *context) { - [self setCurrentTopic:nil]; NSArray *currentTopics = [ReaderAbstractTopic lookupAllInContext:context error:nil]; for (ReaderAbstractTopic *topic in currentTopics) { DDLogInfo(@"Deleting topic: %@", topic.title); @@ -305,8 +234,6 @@ - (void)followTagNamed:(NSString *)topicName NSDictionary *properties = @{@"tag":topicName, @"source":source}; [WPAnalytics trackReaderStat:WPAnalyticsStatReaderTagFollowed properties:properties]; [self.coreDataStack performAndSaveUsingBlock:^(NSManagedObjectContext *context) { - [self selectTopicWithID:topicID inContext:context]; - // mark tag topic as followed ReaderTagTopic *tag = [ReaderTagTopic lookupWithTagID:topicID inContext:context]; if (tag) { @@ -597,16 +524,6 @@ - (WordPressComRestApi *)apiForRequest return api; } -/** - Finds an existing topic matching the specified topicID and, if found, makes it the - selected topic. - */ -- (void)selectTopicWithID:(NSNumber *)topicID inContext:(NSManagedObjectContext *)context -{ - ReaderAbstractTopic *topic = [ReaderTagTopic lookupWithTagID:topicID inContext:context]; - [self setCurrentTopic:topic]; -} - /** Create a new `ReaderAbstractTopic` or update an existing `ReaderAbstractTopic`. @@ -862,10 +779,7 @@ - (void)mergeMenuTopics:(NSArray *)topics isLoggedIn:(BOOL)isLoggedIn withSucces if ([currentTopics count] > 0) { for (ReaderAbstractTopic *topic in currentTopics) { if (![topic isKindOfClass:[ReaderSiteTopic class]] && ![topicsToKeep containsObject:topic]) { - - if ([topic isEqual:[self currentTopicInContext:context]]) { - self.currentTopic = nil; - } + if (topic.inUse) { if (!ReaderHelpers.isLoggedIn && [topic isKindOfClass:ReaderTagTopic.class]) { DDLogInfo(@"Not unfollowing a locally saved topic: %@", topic.title); diff --git a/WordPress/Classes/System/Root View/ReaderPresenter.swift b/WordPress/Classes/System/Root View/ReaderPresenter.swift index fcda27c1748c..e2cae6ff5413 100644 --- a/WordPress/Classes/System/Root View/ReaderPresenter.swift +++ b/WordPress/Classes/System/Root View/ReaderPresenter.swift @@ -117,15 +117,20 @@ final class ReaderPresenter: NSObject, SplitViewDisplayable { private func makeViewController(for screen: ReaderStaticScreen) -> UIViewController { switch screen { - case .recent, .discover, .likes: - if let topic = screen.topicType.flatMap(sidebarViewModel.getTopic) { - if screen == .discover { - return ReaderDiscoverViewController(topic: topic) - } else { - return ReaderStreamViewController.controllerWithTopic(topic) - } + case .discover: + return ReaderDiscoverViewController() + case .recent: + // TODO: (tech debt) Fix an issue where this fails if opened before the menus are fetched for the first time + if let topic = sidebarViewModel.getRecentTopic() { + return ReaderStreamViewController.controllerWithTopic(topic) + } else { + return makeErrorViewController() + } + case .likes: + if let topic = sidebarViewModel.getLikesTopic() { + return ReaderStreamViewController.controllerWithTopic(topic) } else { - return makeErrorViewController() // This should never happen + return makeErrorViewController() } case .saved: return ReaderStreamViewController.controllerForContentType(.saved) diff --git a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderDiscoverViewController.swift b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderDiscoverViewController.swift index 5729b75351fd..df8ce2ec1803 100644 --- a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderDiscoverViewController.swift +++ b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderDiscoverViewController.swift @@ -7,7 +7,6 @@ import WordPressShared class ReaderDiscoverViewController: UIViewController, ReaderDiscoverHeaderViewDelegate { private let headerView = ReaderDiscoverHeaderView() private var selectedChannel: ReaderDiscoverChannel = .recommended - private let topic: ReaderAbstractTopic private var streamVC: ReaderStreamViewController? private weak var selectInterestsVC: ReaderSelectInterestsViewController? private let selectInterestsCoordinator = ReaderSelectInterestsCoordinator() @@ -15,10 +14,8 @@ class ReaderDiscoverViewController: UIViewController, ReaderDiscoverHeaderViewDe private let viewContext: NSManagedObjectContext private var cancellables: [AnyCancellable] = [] - init(topic: ReaderAbstractTopic) { - wpAssert(ReaderHelpers.topicIsDiscover(topic)) + init() { self.viewContext = ContextManager.shared.mainContext - self.topic = topic self.tags = ManagedObjectsObserver( predicate: ReaderSidebarTagsSection.predicate, sortDescriptors: [SortDescriptor(\.title, order: .forward)], @@ -40,6 +37,8 @@ class ReaderDiscoverViewController: UIViewController, ReaderDiscoverHeaderViewDe configureStream(for: selectedChannel) showSelectInterestsIfNeeded() + + WPAnalytics.trackReaderEvent(.readerDiscoverShown, properties: [:]) } private func setupNavigation() { @@ -66,17 +65,21 @@ class ReaderDiscoverViewController: UIViewController, ReaderDiscoverHeaderViewDe // MARK: - Selected Stream private func configureStream(for channel: ReaderDiscoverChannel) { - showStreamViewController(makeViewController(for: channel)) + do { + showStreamViewController(try makeViewController(for: channel)) + } catch { + wpAssertionFailure("failed to show stream", userInfo: ["error": "\(error)"]) + } } - private func makeViewController(for channel: ReaderDiscoverChannel) -> ReaderStreamViewController { + private func makeViewController(for channel: ReaderDiscoverChannel) throws -> ReaderStreamViewController { switch channel { case .recommended: - ReaderDiscoverStreamViewController(topic: topic) + ReaderDiscoverStreamViewController(topic: try makeTopic(path: .recommended)) case .firstPosts: - ReaderDiscoverStreamViewController(topic: topic, stream: .firstPosts, sorting: .date) + ReaderDiscoverStreamViewController(topic: try makeTopic(path: .firstPosts), stream: .firstPosts, sorting: .date) case .latest: - ReaderDiscoverStreamViewController(topic: topic, sorting: .date) + ReaderDiscoverStreamViewController(topic: try makeTopic(path: .latest), sorting: .date) case .dailyPrompts: ReaderStreamViewController.controllerWithTagSlug(ReaderTagTopic.dailyPromptTag) case .tag(let tag): @@ -84,6 +87,10 @@ class ReaderDiscoverViewController: UIViewController, ReaderDiscoverHeaderViewDe } } + private func makeTopic(path: ReaderDefaultTopic.Path) throws -> ReaderDefaultTopic { + try ReaderDefaultTopic.make(path: path, in: viewContext) + } + private func showStreamViewController(_ streamVC: ReaderStreamViewController) { if let currentVC = self.streamVC { deleteCachedReaderCards() diff --git a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderHelpers.swift b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderHelpers.swift index b9ae0f5a01b8..30b4feafb15a 100644 --- a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderHelpers.swift +++ b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderHelpers.swift @@ -106,28 +106,6 @@ struct ReaderNotificationKeys { return topic.isKind(of: ReaderSearchTopic.self) } - /// Check if the specified topic is for Freshly Pressed - /// - /// - Parameters: - /// - topic: A ReaderAbstractTopic - /// - /// - Returns: True if the topic is for Freshly Pressed - /// - @objc open class func topicIsFreshlyPressed(_ topic: ReaderAbstractTopic) -> Bool { - return topic.path.hasSuffix("/freshly-pressed") - } - - /// Check if the specified topic is for Discover - /// - /// - Parameters: - /// - topic: A ReaderAbstractTopic - /// - /// - Returns: True if the topic is for Discover - /// - @objc open class func topicIsDiscover(_ topic: ReaderAbstractTopic) -> Bool { - return topic.path.contains("/read/sites/53424024/posts") - } - /// Check if the specified topic is for Following /// /// - Parameters: @@ -155,10 +133,7 @@ struct ReaderNotificationKeys { class func trackLoadedTopic(_ topic: ReaderAbstractTopic, withProperties properties: [AnyHashable: Any]) { var stat: WPAnalyticsStat? - if topicIsFreshlyPressed(topic) { - stat = .readerFreshlyPressedLoaded - - } else if topicIsFollowing(topic) { + if topicIsFollowing(topic) { WPAnalytics.trackReader(.readerFollowingShown, properties: properties) } else if topicIsLiked(topic) { @@ -167,16 +142,11 @@ struct ReaderNotificationKeys { } else if isTopicSite(topic) { WPAnalytics.trackReader(.readerBlogPreviewed, properties: properties) - } else if isTopicDefault(topic) && topicIsDiscover(topic) { - // Tracks Discover only if it was one of the default menu items. - WPAnalytics.trackReaderEvent(.readerDiscoverShown, properties: properties) - } else if isTopicList(topic) { stat = .readerListLoaded } else if isTopicTag(topic) { stat = .readerTagLoaded - } else if let teamTopic = topic as? ReaderTeamTopic { WPAnalytics.trackReader(teamTopic.shownTrackEvent, properties: properties) } @@ -260,38 +230,6 @@ struct ReaderNotificationKeys { Blog.lookup(withID: siteID, in: ContextManager.sharedInstance().mainContext)?.isAdmin ?? false } - // convenience method that returns the topic type - class func topicType(_ topic: ReaderAbstractTopic?) -> ReaderTopicType { - guard let topic = topic else { - return .noTopic - } - if topicIsDiscover(topic) { - return .discover - } - if topicIsFollowing(topic) { - return .following - } - if topicIsLiked(topic) { - return .likes - } - if isTopicList(topic) { - return .list - } - if isTopicSearchTopic(topic) { - return .search - } - if isTopicSite(topic) { - return .site - } - if isTopicTag(topic) { - return .tag - } - if topic is ReaderTeamTopic { - return .organization - } - return .noTopic - } - // MARK: Logged in helper @objc open class func isLoggedIn() -> Bool { @@ -527,19 +465,6 @@ struct ReaderNotificationKeys { } } -/// Typed topic type -enum ReaderTopicType { - case discover - case following - case likes - case list - case search - case site - case tag - case organization - case noTopic -} - @objc enum SiteOrganizationType: Int { // site does not belong to an organization case none diff --git a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderPostActions/ReaderPostMenu.swift b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderPostActions/ReaderPostMenu.swift index 73e89417ec12..54b36609cedb 100644 --- a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderPostActions/ReaderPostMenu.swift +++ b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderPostActions/ReaderPostMenu.swift @@ -161,10 +161,10 @@ struct ReaderPostMenu { guard let topic else { return false } - return ReaderHelpers.isTopicTag(topic) || - ReaderHelpers.topicIsDiscover(topic) || - ReaderHelpers.topicIsFreshlyPressed(topic) || - ReaderHelpers.topicIsFollowing(topic) + if ReaderHelpers.isTopicTag(topic) { + return true + } + return (topic is ReaderDefaultTopic) && !ReaderHelpers.topicIsLiked(topic) } // MARK: Helpers diff --git a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderStreamViewController.swift b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderStreamViewController.swift index ebb63c6dd294..a43ba4d54c38 100644 --- a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderStreamViewController.swift +++ b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderStreamViewController.swift @@ -215,11 +215,6 @@ import AutomatticTracks /// - Returns: An instance of the controller /// @objc class func controllerWithTopic(_ topic: ReaderAbstractTopic) -> ReaderStreamViewController { - // if a default discover topic is provided, treat it as a site to retrieve the header - if ReaderHelpers.topicIsDiscover(topic) { - return controllerWithSiteID(ReaderHelpers.discoverSiteID, isFeed: false) - } - let controller = ReaderStreamViewController() controller.readerTopic = topic return controller @@ -238,7 +233,6 @@ import AutomatticTracks let controller = ReaderStreamViewController() controller.isFeed = isFeed controller.siteID = siteID - return controller } diff --git a/WordPress/Classes/ViewRelated/Reader/Manage/ReaderTagsTableViewModel.swift b/WordPress/Classes/ViewRelated/Reader/Manage/ReaderTagsTableViewModel.swift index d89f3c859b18..cddab288403d 100644 --- a/WordPress/Classes/ViewRelated/Reader/Manage/ReaderTagsTableViewModel.swift +++ b/WordPress/Classes/ViewRelated/Reader/Manage/ReaderTagsTableViewModel.swift @@ -181,15 +181,8 @@ extension ReaderTagsTableViewModel { let generator = UINotificationFeedbackGenerator() generator.prepare() - service.followTagNamed(tagName, withSuccess: { [weak self] in + service.followTagNamed(tagName, withSuccess: { generator.notificationOccurred(.success) - - guard let self else { return } - - // A successful follow makes the new tag the currentTopic. - if let tag = service.currentTopic(in: self.context) as? ReaderTagTopic { - self.scrollToTag(tag) - } }, failure: { (error) in DDLogError("Could not follow topic named \(tagName) : \(String(describing: error))") @@ -219,17 +212,6 @@ extension ReaderTagsTableViewModel { alert.presentFromRootViewController() } } - - /// Scrolls the tableView so the specified tag is in view and flashes that row - /// - /// - Parameters: - /// - tag: The tag to scroll into view. - private func scrollToTag(_ tag: ReaderTagTopic) { - guard let indexPath = tableViewHandler.resultsController?.indexPath(forObject: tag) else { - return - } - tableView?.flashRowAtIndexPath(tableViewHandler.adjustedToTable(indexPath: indexPath), scrollPosition: .middle, completion: {}) - } } extension ReaderTagTopic { diff --git a/WordPress/Classes/ViewRelated/Reader/Sidebar/ReaderSidebarViewModel.swift b/WordPress/Classes/ViewRelated/Reader/Sidebar/ReaderSidebarViewModel.swift index e54ad900af32..1e51a77e70c9 100644 --- a/WordPress/Classes/ViewRelated/Reader/Sidebar/ReaderSidebarViewModel.swift +++ b/WordPress/Classes/ViewRelated/Reader/Sidebar/ReaderSidebarViewModel.swift @@ -34,10 +34,14 @@ final class ReaderSidebarViewModel: ObservableObject { } } - func getTopic(for topicType: ReaderTopicType) -> ReaderAbstractTopic? { - return try? ReaderAbstractTopic.lookupAllMenus(in: contextManager.mainContext).first { - ReaderHelpers.topicType($0) == topicType - } + func getRecentTopic() -> ReaderAbstractTopic? { + try? ReaderAbstractTopic.lookupAllMenus(in: contextManager.mainContext) + .first(where: ReaderHelpers.topicIsFollowing) + } + + func getLikesTopic() -> ReaderAbstractTopic? { + try? ReaderAbstractTopic.lookupAllMenus(in: contextManager.mainContext) + .first(where: ReaderHelpers.topicIsLiked) } func onAppear() { @@ -107,16 +111,6 @@ enum ReaderStaticScreen: String, CaseIterable, Identifiable, Hashable { } } - var topicType: ReaderTopicType? { - switch self { - case .recent: .following - case .discover: .discover - case .saved: nil - case .likes: .likes - case .search: nil - } - } - var accessibilityIdentifier: String { "reader_sidebar_\(rawValue)" } diff --git a/WordPress/WordPressTest/ReaderTopicServiceTest.swift b/WordPress/WordPressTest/ReaderTopicServiceTest.swift index bdcdeba21a9d..499cdff6321c 100644 --- a/WordPress/WordPressTest/ReaderTopicServiceTest.swift +++ b/WordPress/WordPressTest/ReaderTopicServiceTest.swift @@ -305,40 +305,6 @@ final class ReaderTopicSwiftTest: CoreDataTestCase { } } - /** - Ensure that a default topic can be set and retrieved. - */ - func testGettingSettingCurrentTopic() { - let remoteTopics = remoteAndDefaultTopicForTests() - - // Setup - let expect = expectation(description: "topics saved expectation") - let service = ReaderTopicService(coreDataStack: contextManager) - service.setCurrentTopic(nil) - - // Current topic is not nil after a sync - service.mergeMenuTopics(remoteTopics, withSuccess: { () -> Void in - expect.fulfill() - }) - waitForExpectations(timeout: expectationTimeout, handler: nil) - - XCTAssertNotNil(service.currentTopic, "The current topic was nil.") - - let sortDescriptor = NSSortDescriptor(key: "title", ascending: true) - let request = NSFetchRequest(entityName: ReaderAbstractTopic.classNameWithoutNamespaces()) - request.sortDescriptors = [sortDescriptor] - - let results = try! mainContext.fetch(request) - - var topic = results.last as! ReaderAbstractTopic - XCTAssertEqual(service.currentTopic(in: mainContext).type, ReaderDefaultTopic.TopicType, "The curent topic should have been a default topic") - - topic = results.first as! ReaderAbstractTopic - service.setCurrentTopic(topic) - - XCTAssertEqual(service.currentTopic(in: mainContext).path, topic.path, "The current topic did not match the topic we assiged to it") - } - /** Ensure all topics are deleted when an account is changed. */