From 00e009a82cf7e527161ec813b71990785e30bf41 Mon Sep 17 00:00:00 2001 From: Phil Viso Date: Sun, 8 Sep 2019 16:48:50 -0500 Subject: [PATCH 01/10] Added ability to group sorted articles by feed --- NetNewsWire.xcodeproj/project.pbxproj | 10 +- Shared/Data/ArticleUtilities.swift | 18 +++ Shared/Timeline/ArticleArray.swift | 17 +-- Shared/Timeline/ArticleSorter.swift | 57 +++++++++ .../NetNewsWireTests/ArticleArrayTests.swift | 109 ++++++++++++++++++ 5 files changed, 196 insertions(+), 15 deletions(-) create mode 100644 Shared/Timeline/ArticleSorter.swift create mode 100644 Tests/NetNewsWireTests/ArticleArrayTests.swift diff --git a/NetNewsWire.xcodeproj/project.pbxproj b/NetNewsWire.xcodeproj/project.pbxproj index 49b9d9b98..cf30eedea 100644 --- a/NetNewsWire.xcodeproj/project.pbxproj +++ b/NetNewsWire.xcodeproj/project.pbxproj @@ -340,6 +340,8 @@ D5F4EDB920074D7C00B9E363 /* Folder+Scriptability.swift in Sources */ = {isa = PBXBuildFile; fileRef = D5F4EDB820074D7C00B9E363 /* Folder+Scriptability.swift */; }; DD82AB0A231003F6002269DF /* SharingTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = DD82AB09231003F6002269DF /* SharingTests.swift */; }; DF999FF722B5AEFA0064B687 /* SafariView.swift in Sources */ = {isa = PBXBuildFile; fileRef = DF999FF622B5AEFA0064B687 /* SafariView.swift */; }; + FF3ABF13232599810074C542 /* ArticleArrayTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = FF3ABF09232599450074C542 /* ArticleArrayTests.swift */; }; + FF3ABF1523259DDB0074C542 /* ArticleSorter.swift in Sources */ = {isa = PBXBuildFile; fileRef = FF3ABF1423259DDB0074C542 /* ArticleSorter.swift */; }; /* End PBXBuildFile section */ /* Begin PBXContainerItemProxy section */ @@ -975,6 +977,8 @@ D5F4EDB820074D7C00B9E363 /* Folder+Scriptability.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Folder+Scriptability.swift"; sourceTree = ""; }; DD82AB09231003F6002269DF /* SharingTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SharingTests.swift; sourceTree = ""; }; DF999FF622B5AEFA0064B687 /* SafariView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SafariView.swift; sourceTree = ""; }; + FF3ABF09232599450074C542 /* ArticleArrayTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ArticleArrayTests.swift; sourceTree = ""; }; + FF3ABF1423259DDB0074C542 /* ArticleSorter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ArticleSorter.swift; sourceTree = ""; }; /* End PBXFileReference section */ /* Begin PBXFrameworksBuildPhase section */ @@ -1254,8 +1258,9 @@ isa = PBXGroup; children = ( 84F204DF1FAACBB30076E152 /* ArticleArray.swift */, - 84CAFCA322BC8C08007694F0 /* FetchRequestQueue.swift */, + FF3ABF1423259DDB0074C542 /* ArticleSorter.swift */, 84CAFCAE22BC8C35007694F0 /* FetchRequestOperation.swift */, + 84CAFCA322BC8C08007694F0 /* FetchRequestQueue.swift */, 849A97731ED9EC04007D329B /* TimelineStringFormatter.swift */, ); path = Timeline; @@ -1877,6 +1882,7 @@ isa = PBXGroup; children = ( 84F9EAD0213660A100CF2DE4 /* ScriptingTests */, + FF3ABF09232599450074C542 /* ArticleArrayTests.swift */, 84F9EAE3213660A100CF2DE4 /* NetNewsWireTests.swift */, DD82AB09231003F6002269DF /* SharingTests.swift */, 84F9EAE4213660A100CF2DE4 /* Info.plist */, @@ -2608,6 +2614,7 @@ 849A978A1ED9ECEF007D329B /* ArticleStylesManager.swift in Sources */, 8405DD8A2213E0E3008CE1BF /* DetailContainerView.swift in Sources */, 519B8D332143397200FA689C /* SharingServiceDelegate.swift in Sources */, + FF3ABF1523259DDB0074C542 /* ArticleSorter.swift in Sources */, 84E8E0DB202EC49300562D8F /* TimelineViewController+ContextualMenus.swift in Sources */, 849A97791ED9EC04007D329B /* TimelineStringFormatter.swift in Sources */, 84E185C3203BB12600F69BFA /* MultilineTextFieldSizer.swift in Sources */, @@ -2686,6 +2693,7 @@ isa = PBXSourcesBuildPhase; buildActionMask = 2147483647; files = ( + FF3ABF13232599810074C542 /* ArticleArrayTests.swift in Sources */, DD82AB0A231003F6002269DF /* SharingTests.swift in Sources */, 84F9EAEB213660A100CF2DE4 /* testIterativeCreateAndDeleteFeed.applescript in Sources */, 84F9EAF4213660A100CF2DE4 /* testGenericScript.applescript in Sources */, diff --git a/Shared/Data/ArticleUtilities.swift b/Shared/Data/ArticleUtilities.swift index 538f09bf3..bb46b045f 100644 --- a/Shared/Data/ArticleUtilities.swift +++ b/Shared/Data/ArticleUtilities.swift @@ -90,3 +90,21 @@ extension Article { } } + +// MARK: SortableArticle + +extension Article: SortableArticle { + + var sortableName: String { + return feed?.name ?? "" + } + + var sortableDate: Date { + return logicalDatePublished + } + + var sortableID: String { + return articleID + } + +} diff --git a/Shared/Timeline/ArticleArray.swift b/Shared/Timeline/ArticleArray.swift index 1e40e7593..60394f084 100644 --- a/Shared/Timeline/ArticleArray.swift +++ b/Shared/Timeline/ArticleArray.swift @@ -49,22 +49,11 @@ extension Array where Element == Article { return articleAtRow(oneIndex) }) } - - func sortedByDate(_ sortDirection: ComparisonResult) -> ArticleArray { - - let articles = sorted { (article1, article2) -> Bool in - if article1.logicalDatePublished == article2.logicalDatePublished { - return article1.articleID < article2.articleID - } - if sortDirection == .orderedDescending { - return article1.logicalDatePublished > article2.logicalDatePublished - } - return article1.logicalDatePublished < article2.logicalDatePublished - } - return articles + func sortedByDate(_ sortDirection: ComparisonResult, groupByFeed: Bool = false) -> ArticleArray { + return ArticleSorter.sortedByDate(articles: self, sortDirection: sortDirection, groupByFeed: groupByFeed) } - + func canMarkAllAsRead() -> Bool { return anyArticleIsUnread() diff --git a/Shared/Timeline/ArticleSorter.swift b/Shared/Timeline/ArticleSorter.swift new file mode 100644 index 000000000..d1b8814ce --- /dev/null +++ b/Shared/Timeline/ArticleSorter.swift @@ -0,0 +1,57 @@ +// +// ArticleSorter.swift +// NetNewsWire +// +// Created by Phil Viso on 9/8/19. +// Copyright © 2019 Ranchero Software. All rights reserved. +// + +import Articles +import Foundation + +protocol SortableArticle { + var sortableName: String { get } + var sortableDate: Date { get } + var sortableID: String { get } +} + +struct ArticleSorter { + + static func sortedByDate(articles: [T], + sortDirection: ComparisonResult, + groupByFeed: Bool) -> [T] { + let articles = articles.sorted { (article1, article2) -> Bool in + if groupByFeed { + let feedName1 = article1.sortableName + let feedName2 = article2.sortableName + + let comparison = feedName1.caseInsensitiveCompare(feedName2) + switch comparison { + case .orderedSame: + return isSortedByDate(article1, article2, sortDirection: sortDirection) + case .orderedAscending, .orderedDescending: + return comparison == .orderedAscending + } + } else { + return isSortedByDate(article1, article2, sortDirection: sortDirection) + } + } + + return articles + } + + // MARK: - + + private static func isSortedByDate(_ lhs: SortableArticle, + _ rhs: SortableArticle, + sortDirection: ComparisonResult) -> Bool { + if lhs.sortableDate == rhs.sortableDate { + return lhs.sortableID < rhs.sortableID + } + if sortDirection == .orderedDescending { + return lhs.sortableDate > rhs.sortableDate + } + return lhs.sortableDate < rhs.sortableDate + } + +} diff --git a/Tests/NetNewsWireTests/ArticleArrayTests.swift b/Tests/NetNewsWireTests/ArticleArrayTests.swift new file mode 100644 index 000000000..b5ed8d3d0 --- /dev/null +++ b/Tests/NetNewsWireTests/ArticleArrayTests.swift @@ -0,0 +1,109 @@ +// +// ArticleArrayTests.swift +// NetNewsWire +// +// Created by Phil Viso on 9/8/19. +// Copyright © 2019 Ranchero Software. All rights reserved. +// + +import Articles +import Foundation +import XCTest + +@testable import NetNewsWire + +class ArticleArrayTests: XCTestCase { + + func testSortByDateAscending() { + let now = Date() + + // Test data includes a mixture of articles in the past and future, as well as articles with the same date + let article1 = TestArticle(sortableName: "Phil's Feed", sortableDate: now, sortableID: "456") + let article2 = TestArticle(sortableName: "Matt's Feed", sortableDate: now, sortableID: "789") + let article3 = TestArticle(sortableName: "Sally's Feed", sortableDate: now, sortableID: "123") + let article4 = TestArticle(sortableName: "Susie's Feed", sortableDate: Date(timeInterval: -60.0, since: now), sortableID: "345") + let article5 = TestArticle(sortableName: "Paul's Feed", sortableDate: Date(timeInterval: -120.0, since: now), sortableID: "567") + let article6 = TestArticle(sortableName: "phil's Feed", sortableDate: Date(timeInterval: 60.0, since: now), sortableID: "567") + + let articles = [article1, article2, article3, article4, article5, article6] + let sortedArticles = ArticleSorter.sortedByDate(articles: articles, + sortDirection: .orderedAscending, + groupByFeed: false) + XCTAssertEqual(sortedArticles, [article5, article4, article3, article1, article2, article6]) + } + + func testSortByDateAscendingWithGroupByFeed() { + let now = Date() + + // Test data includes multiple groups (with case-insentive names), articles in the past and future, + // as well as articles with the same date + let article1 = TestArticle(sortableName: "Susie's Feed", sortableDate: Date(timeInterval: -240.0, since: now), sortableID: "123") + let article2 = TestArticle(sortableName: "Phil's Feed", sortableDate: now, sortableID: "456") + let article3 = TestArticle(sortableName: "Matt's Feed", sortableDate: now, sortableID: "234") + let article4 = TestArticle(sortableName: "Susie's Feed", sortableDate: Date(timeInterval: -120.0, since: now), sortableID: "123") + let article5 = TestArticle(sortableName: "phil's feed", sortableDate: Date(timeInterval: 60.0, since: now), sortableID: "456") + let article6 = TestArticle(sortableName: "Matt's Feed", sortableDate: now, sortableID: "123") + let article7 = TestArticle(sortableName: "Susie's Feed", sortableDate: Date(timeInterval: -60.0, since: now), sortableID: "123") + let article8 = TestArticle(sortableName: "phil's Feed", sortableDate: Date(timeInterval: -60.0, since: now), sortableID: "456") + let article9 = TestArticle(sortableName: "Matt's Feed", sortableDate: now, sortableID: "345") + let article10 = TestArticle(sortableName: "Susie's Feed", sortableDate: Date(timeInterval: -15.0, since: now), sortableID: "123") + let article11 = TestArticle(sortableName: "Matt's Feed", sortableDate: Date(timeInterval: 60.0, since: now), sortableID: "123") + let article12 = TestArticle(sortableName: "Claire's Feed", sortableDate: now, sortableID: "123") + + let articles = [article1, article2, article3, article4, article5, article6, article7, article8, article9, article10, article11, article12] + let sortedArticles = ArticleSorter.sortedByDate(articles: articles, + sortDirection: .orderedAscending, + groupByFeed: true) + XCTAssertEqual(sortedArticles, [article12, article6, article3, article9, article11, article8, article2, article5, article1, article4, article7, article10]) + } + + func testSortByDateDescending() { + let now = Date() + + // Test data includes a mixture of articles in the past and future, as well as articles with the same date + let article1 = TestArticle(sortableName: "Phil's Feed", sortableDate: now, sortableID: "456") + let article2 = TestArticle(sortableName: "Matt's Feed", sortableDate: now, sortableID: "789") + let article3 = TestArticle(sortableName: "Sally's Feed", sortableDate: now, sortableID: "123") + let article4 = TestArticle(sortableName: "Susie's Feed", sortableDate: Date(timeInterval: -60.0, since: now), sortableID: "345") + let article5 = TestArticle(sortableName: "Paul's Feed", sortableDate: Date(timeInterval: -120.0, since: now), sortableID: "567") + let article6 = TestArticle(sortableName: "phil's Feed", sortableDate: Date(timeInterval: 60.0, since: now), sortableID: "567") + + let articles = [article1, article2, article3, article4, article5, article6] + let sortedArticles = ArticleSorter.sortedByDate(articles: articles, + sortDirection: .orderedDescending, + groupByFeed: false) + XCTAssertEqual(sortedArticles, [article6, article3, article1, article2, article4, article5]) + } + + func testSortByDateDescendingWithGroupByFeed() { + let now = Date() + + // Test data includes multiple groups (with case-insentive names), articles in the past and future, + // as well as articles with the same date + let article1 = TestArticle(sortableName: "Susie's Feed", sortableDate: Date(timeInterval: -240.0, since: now), sortableID: "123") + let article2 = TestArticle(sortableName: "Phil's Feed", sortableDate: now, sortableID: "456") + let article3 = TestArticle(sortableName: "Matt's Feed", sortableDate: now, sortableID: "234") + let article4 = TestArticle(sortableName: "Susie's Feed", sortableDate: Date(timeInterval: -120.0, since: now), sortableID: "123") + let article5 = TestArticle(sortableName: "phil's feed", sortableDate: Date(timeInterval: 60.0, since: now), sortableID: "456") + let article6 = TestArticle(sortableName: "Matt's Feed", sortableDate: now, sortableID: "123") + let article7 = TestArticle(sortableName: "Susie's Feed", sortableDate: Date(timeInterval: -60.0, since: now), sortableID: "123") + let article8 = TestArticle(sortableName: "phil's Feed", sortableDate: Date(timeInterval: -60.0, since: now), sortableID: "456") + let article9 = TestArticle(sortableName: "Matt's Feed", sortableDate: now, sortableID: "345") + let article10 = TestArticle(sortableName: "Susie's Feed", sortableDate: Date(timeInterval: -15.0, since: now), sortableID: "123") + let article11 = TestArticle(sortableName: "Matt's Feed", sortableDate: Date(timeInterval: 60.0, since: now), sortableID: "123") + let article12 = TestArticle(sortableName: "Claire's Feed", sortableDate: now, sortableID: "123") + + let articles = [article1, article2, article3, article4, article5, article6, article7, article8, article9, article10, article11, article12] + let sortedArticles = ArticleSorter.sortedByDate(articles: articles, + sortDirection: .orderedDescending, + groupByFeed: true) + XCTAssertEqual(sortedArticles, [article12, article11, article6, article3, article9, article5, article2, article8, article10, article7, article4, article1]) + } + +} + +private struct TestArticle: SortableArticle, Equatable { + let sortableName: String + let sortableDate: Date + let sortableID: String +} From 32d6678fdda0ce900af6da911bf4780ef72e75c7 Mon Sep 17 00:00:00 2001 From: Phil Viso Date: Sun, 8 Sep 2019 17:09:26 -0500 Subject: [PATCH 02/10] Added group by feed menu item --- Mac/AppDefaults.swift | 18 +++++++++++++++++- Mac/AppDelegate.swift | 13 +++++++++++++ Mac/Base.lproj/Main.storyboard | 12 ++++++++++-- .../Timeline/TimelineViewController.swift | 18 ++++++++++++++++-- 4 files changed, 56 insertions(+), 5 deletions(-) diff --git a/Mac/AppDefaults.swift b/Mac/AppDefaults.swift index f4180440b..9afd69ecb 100644 --- a/Mac/AppDefaults.swift +++ b/Mac/AppDefaults.swift @@ -22,6 +22,7 @@ struct AppDefaults { static let sidebarFontSize = "sidebarFontSize" static let timelineFontSize = "timelineFontSize" static let timelineSortDirection = "timelineSortDirection" + static let timelineGroupByFeed = "timelineGroupByFeed" static let detailFontSize = "detailFontSize" static let openInBrowserInBackground = "openInBrowserInBackground" static let mainWindowWidths = "mainWindowWidths" @@ -137,6 +138,15 @@ struct AppDefaults { } } + static var timelineGroupByFeed: Bool { + get { + return bool(for: Key.timelineGroupByFeed) + } + set { + setBool(for: Key.timelineGroupByFeed, newValue) + } + } + static var timelineShowsSeparators: Bool { return bool(for: Key.timelineShowsSeparators) } @@ -161,7 +171,13 @@ struct AppDefaults { } static func registerDefaults() { - let defaults: [String : Any] = [Key.sidebarFontSize: FontSize.medium.rawValue, Key.timelineFontSize: FontSize.medium.rawValue, Key.detailFontSize: FontSize.medium.rawValue, Key.timelineSortDirection: ComparisonResult.orderedDescending.rawValue, "NSScrollViewShouldScrollUnderTitlebar": false, Key.refreshInterval: RefreshInterval.everyHour.rawValue] + let defaults: [String : Any] = [Key.sidebarFontSize: FontSize.medium.rawValue, + Key.timelineFontSize: FontSize.medium.rawValue, + Key.detailFontSize: FontSize.medium.rawValue, + Key.timelineSortDirection: ComparisonResult.orderedDescending.rawValue, + Key.timelineGroupByFeed: false, + "NSScrollViewShouldScrollUnderTitlebar": false, + Key.refreshInterval: RefreshInterval.everyHour.rawValue] UserDefaults.standard.register(defaults: defaults) diff --git a/Mac/AppDelegate.swift b/Mac/AppDelegate.swift index dd300884a..46bc8e864 100644 --- a/Mac/AppDelegate.swift +++ b/Mac/AppDelegate.swift @@ -41,6 +41,7 @@ class AppDelegate: NSObject, NSApplicationDelegate, NSUserInterfaceValidations, @IBOutlet var debugMenuItem: NSMenuItem! @IBOutlet var sortByOldestArticleOnTopMenuItem: NSMenuItem! @IBOutlet var sortByNewestArticleOnTopMenuItem: NSMenuItem! + @IBOutlet var groupArticlesByFeedMenuItem: NSMenuItem! @IBOutlet var checkForUpdatesMenuItem: NSMenuItem! var unreadCount = 0 { @@ -146,6 +147,7 @@ class AppDelegate: NSObject, NSApplicationDelegate, NSUserInterfaceValidations, feedIconDownloader = FeedIconDownloader(imageDownloader: imageDownloader, folder: cacheFolder) updateSortMenuItems() + updateGroupByFeedMenuItem() createAndShowMainWindow() if isFirstRun { mainWindowController?.window?.center() @@ -257,6 +259,7 @@ class AppDelegate: NSObject, NSApplicationDelegate, NSUserInterfaceValidations, @objc func userDefaultsDidChange(_ note: Notification) { updateSortMenuItems() + updateGroupByFeedMenuItem() refreshTimer?.update() updateDockBadge() } @@ -507,6 +510,11 @@ class AppDelegate: NSObject, NSApplicationDelegate, NSUserInterfaceValidations, AppDefaults.timelineSortDirection = .orderedDescending } + + @IBAction func groupByFeedToggled(_ sender: NSMenuItem) { + AppDefaults.timelineGroupByFeed.toggle() + } + } // MARK: - Debug Menu @@ -544,6 +552,11 @@ private extension AppDelegate { sortByNewestArticleOnTopMenuItem.state = sortByNewestOnTop ? .on : .off sortByOldestArticleOnTopMenuItem.state = sortByNewestOnTop ? .off : .on } + + func updateGroupByFeedMenuItem() { + let groupByFeedEnabled = AppDefaults.timelineGroupByFeed + groupArticlesByFeedMenuItem.state = groupByFeedEnabled ? .on : .off + } } /* diff --git a/Mac/Base.lproj/Main.storyboard b/Mac/Base.lproj/Main.storyboard index 8d270183a..9d5854258 100644 --- a/Mac/Base.lproj/Main.storyboard +++ b/Mac/Base.lproj/Main.storyboard @@ -1,7 +1,8 @@ - + - + + @@ -349,6 +350,12 @@ + + + + + + @@ -581,6 +588,7 @@ + diff --git a/Mac/MainWindow/Timeline/TimelineViewController.swift b/Mac/MainWindow/Timeline/TimelineViewController.swift index 54fef9c54..55fcfd0f4 100644 --- a/Mac/MainWindow/Timeline/TimelineViewController.swift +++ b/Mac/MainWindow/Timeline/TimelineViewController.swift @@ -128,6 +128,13 @@ final class TimelineViewController: NSViewController, UndoableCommandRunner, Unr } } } + private var groupByFeed = AppDefaults.timelineGroupByFeed { + didSet { + if groupByFeed != oldValue { + groupByFeedDidChange() + } + } + } private var fontSize: FontSize = AppDefaults.timelineFontSize { didSet { if fontSize != oldValue { @@ -553,6 +560,7 @@ final class TimelineViewController: NSViewController, UndoableCommandRunner, Unr self.fontSize = AppDefaults.timelineFontSize self.sortDirection = AppDefaults.timelineSortDirection + self.groupByFeed = AppDefaults.timelineGroupByFeed } @objc func appleInterfaceThemeChanged(_ note: Notification) { @@ -875,7 +883,13 @@ private extension TimelineViewController { } func sortDirectionDidChange() { - + performBlockAndRestoreSelection { + let unsortedArticles = Set(articles) + replaceArticles(with: unsortedArticles) + } + } + + func groupByFeedDidChange() { performBlockAndRestoreSelection { let unsortedArticles = Set(articles) replaceArticles(with: unsortedArticles) @@ -978,7 +992,7 @@ private extension TimelineViewController { } func replaceArticles(with unsortedArticles: Set
) { - let sortedArticles = Array(unsortedArticles).sortedByDate(sortDirection) + let sortedArticles = Array(unsortedArticles).sortedByDate(sortDirection, groupByFeed: groupByFeed) if articles != sortedArticles { articles = sortedArticles } From 045bc61684861e6c763ee0eebb3fcbe217d774ae Mon Sep 17 00:00:00 2001 From: Phil Viso Date: Sun, 8 Sep 2019 17:12:18 -0500 Subject: [PATCH 03/10] Renamed Sort By menu item to Sort Articles By --- Mac/Base.lproj/Main.storyboard | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Mac/Base.lproj/Main.storyboard b/Mac/Base.lproj/Main.storyboard index 9d5854258..13c78b0d4 100644 --- a/Mac/Base.lproj/Main.storyboard +++ b/Mac/Base.lproj/Main.storyboard @@ -331,9 +331,9 @@ - + - + From 01c48e788b727f3c30af272204db7fc62ff2ef80 Mon Sep 17 00:00:00 2001 From: Phil Viso Date: Sun, 8 Sep 2019 17:41:00 -0500 Subject: [PATCH 04/10] Implemented group by feed for iOS --- iOS/AppDefaults.swift | 17 +++++++++++++++-- iOS/SceneCoordinator.swift | 14 +++++++++++++- iOS/Settings/SettingsView.swift | 13 +++++++++++++ 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/iOS/AppDefaults.swift b/iOS/AppDefaults.swift index 52b98d30d..98c59261a 100644 --- a/iOS/AppDefaults.swift +++ b/iOS/AppDefaults.swift @@ -12,10 +12,11 @@ struct AppDefaults { struct Key { static let firstRunDate = "firstRunDate" + static let timelineGroupByFeed = "timelineGroupByFeed" + static let timelineNumberOfLines = "timelineNumberOfLines" static let timelineSortDirection = "timelineSortDirection" static let refreshInterval = "refreshInterval" static let lastRefresh = "lastRefresh" - static let timelineNumberOfLines = "timelineNumberOfLines" } static let isFirstRun: Bool = { @@ -35,6 +36,15 @@ struct AppDefaults { UserDefaults.standard.set(newValue.rawValue, forKey: Key.refreshInterval) } } + + static var timelineGroupByFeed: Bool { + get { + return bool(for: Key.timelineGroupByFeed) + } + set { + setBool(for: Key.timelineGroupByFeed, newValue) + } + } static var timelineSortDirection: ComparisonResult { get { @@ -64,7 +74,10 @@ struct AppDefaults { } static func registerDefaults() { - let defaults: [String : Any] = [Key.timelineSortDirection: ComparisonResult.orderedDescending.rawValue, Key.refreshInterval: RefreshInterval.everyHour.rawValue, Key.timelineNumberOfLines: 3] + let defaults: [String : Any] = [Key.refreshInterval: RefreshInterval.everyHour.rawValue, + Key.timelineGroupByFeed: false, + Key.timelineNumberOfLines: 3, + Key.timelineSortDirection: ComparisonResult.orderedDescending.rawValue] UserDefaults.standard.register(defaults: defaults) } diff --git a/iOS/SceneCoordinator.swift b/iOS/SceneCoordinator.swift index 8db81d157..eb3c97550 100644 --- a/iOS/SceneCoordinator.swift +++ b/iOS/SceneCoordinator.swift @@ -68,6 +68,13 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { } } } + private(set) var groupByFeed = AppDefaults.timelineGroupByFeed { + didSet { + if groupByFeed != oldValue { + groupByFeedDidChange() + } + } + } private let treeControllerDelegate = FeedTreeControllerDelegate() private lazy var treeController: TreeController = { @@ -412,6 +419,7 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { @objc func userDefaultsDidChange(_ note: Notification) { self.sortDirection = AppDefaults.timelineSortDirection + self.groupByFeed = AppDefaults.timelineGroupByFeed } @objc func accountDidDownloadArticles(_ note: Notification) { @@ -1344,8 +1352,12 @@ private extension SceneCoordinator { replaceArticles(with: Set(articles), animate: true) } + func groupByFeedDidChange() { + replaceArticles(with: Set(articles), animate: true) + } + func replaceArticles(with unsortedArticles: Set
, animate: Bool) { - let sortedArticles = Array(unsortedArticles).sortedByDate(sortDirection) + let sortedArticles = Array(unsortedArticles).sortedByDate(sortDirection, groupByFeed: groupByFeed) if articles != sortedArticles { diff --git a/iOS/Settings/SettingsView.swift b/iOS/Settings/SettingsView.swift index 3636dcaed..a293d534e 100644 --- a/iOS/Settings/SettingsView.swift +++ b/iOS/Settings/SettingsView.swift @@ -56,6 +56,9 @@ struct SettingsView : View { Toggle(isOn: $viewModel.sortOldestToNewest) { Text("Sort Oldest to Newest") } + Toggle(isOn: $viewModel.groupByFeed) { + Text("Group By Feed") + } Stepper(value: $viewModel.timelineNumberOfLines, in: 2...6) { Text("Number of Text Lines: \(viewModel.timelineNumberOfLines)") } @@ -221,6 +224,16 @@ struct SettingsView : View { } } + var groupByFeed: Bool { + get { + return AppDefaults.timelineGroupByFeed + } + set { + objectWillChange.send() + AppDefaults.timelineGroupByFeed = newValue + } + } + var timelineNumberOfLines: Int { get { return AppDefaults.timelineNumberOfLines From cb215c46d70b94e91fd7630eb9169c8987d4b55a Mon Sep 17 00:00:00 2001 From: Phil Viso Date: Sun, 8 Sep 2019 17:41:34 -0500 Subject: [PATCH 05/10] Added ArticleSorter to iOS project --- NetNewsWire.xcodeproj/project.pbxproj | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NetNewsWire.xcodeproj/project.pbxproj b/NetNewsWire.xcodeproj/project.pbxproj index cf30eedea..b7675242d 100644 --- a/NetNewsWire.xcodeproj/project.pbxproj +++ b/NetNewsWire.xcodeproj/project.pbxproj @@ -342,6 +342,7 @@ DF999FF722B5AEFA0064B687 /* SafariView.swift in Sources */ = {isa = PBXBuildFile; fileRef = DF999FF622B5AEFA0064B687 /* SafariView.swift */; }; FF3ABF13232599810074C542 /* ArticleArrayTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = FF3ABF09232599450074C542 /* ArticleArrayTests.swift */; }; FF3ABF1523259DDB0074C542 /* ArticleSorter.swift in Sources */ = {isa = PBXBuildFile; fileRef = FF3ABF1423259DDB0074C542 /* ArticleSorter.swift */; }; + FF3ABF162325AF5D0074C542 /* ArticleSorter.swift in Sources */ = {isa = PBXBuildFile; fileRef = FF3ABF1423259DDB0074C542 /* ArticleSorter.swift */; }; /* End PBXBuildFile section */ /* Begin PBXContainerItemProxy section */ @@ -2480,6 +2481,7 @@ 51D5948722668EFA00DFC836 /* MarkStatusCommand.swift in Sources */, 514B7C8323205EFB00BAC947 /* RootSplitViewController.swift in Sources */, 5152E0F923248F6200E5C7AD /* SettingsLocalAccountView.swift in Sources */, + FF3ABF162325AF5D0074C542 /* ArticleSorter.swift in Sources */, 51C4525C226508DF00C03939 /* String-Extensions.swift in Sources */, 51C452792265091600C03939 /* MasterTimelineTableViewCell.swift in Sources */, 51C452852265093600C03939 /* AddFeedFolderPickerData.swift in Sources */, From cf404859e412680c2bdd92835bf376398a255285 Mon Sep 17 00:00:00 2001 From: Phil Viso Date: Sun, 8 Sep 2019 17:42:43 -0500 Subject: [PATCH 06/10] Fixed sort order description being flipped --- iOS/Settings/SettingsView.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/iOS/Settings/SettingsView.swift b/iOS/Settings/SettingsView.swift index a293d534e..253530f66 100644 --- a/iOS/Settings/SettingsView.swift +++ b/iOS/Settings/SettingsView.swift @@ -54,7 +54,7 @@ struct SettingsView : View { func buildTimelineSection() -> some View { Section(header: Text("TIMELINE")) { Toggle(isOn: $viewModel.sortOldestToNewest) { - Text("Sort Oldest to Newest") + Text("Sort Newest to Oldest") } Toggle(isOn: $viewModel.groupByFeed) { Text("Group By Feed") From 269364a3379087f0a04faf0557b03046b5dc24da Mon Sep 17 00:00:00 2001 From: Phil Viso Date: Fri, 13 Sep 2019 07:43:28 -0500 Subject: [PATCH 07/10] Re-worked sorting logic to handle multiple feeds having the same name --- Shared/Data/ArticleUtilities.swift | 6 +- Shared/Timeline/ArticleSorter.swift | 68 +++-- .../NetNewsWireTests/ArticleArrayTests.swift | 247 ++++++++++++++---- 3 files changed, 234 insertions(+), 87 deletions(-) diff --git a/Shared/Data/ArticleUtilities.swift b/Shared/Data/ArticleUtilities.swift index bb46b045f..b822ab706 100644 --- a/Shared/Data/ArticleUtilities.swift +++ b/Shared/Data/ArticleUtilities.swift @@ -103,8 +103,12 @@ extension Article: SortableArticle { return logicalDatePublished } - var sortableID: String { + var sortableArticleID: String { return articleID } + var sortableFeedID: String { + return feedID + } + } diff --git a/Shared/Timeline/ArticleSorter.swift b/Shared/Timeline/ArticleSorter.swift index d1b8814ce..9196ee765 100644 --- a/Shared/Timeline/ArticleSorter.swift +++ b/Shared/Timeline/ArticleSorter.swift @@ -12,46 +12,58 @@ import Foundation protocol SortableArticle { var sortableName: String { get } var sortableDate: Date { get } - var sortableID: String { get } + var sortableArticleID: String { get } + var sortableFeedID: String { get } } struct ArticleSorter { - + static func sortedByDate(articles: [T], sortDirection: ComparisonResult, groupByFeed: Bool) -> [T] { - let articles = articles.sorted { (article1, article2) -> Bool in - if groupByFeed { - let feedName1 = article1.sortableName - let feedName2 = article2.sortableName - - let comparison = feedName1.caseInsensitiveCompare(feedName2) - switch comparison { - case .orderedSame: - return isSortedByDate(article1, article2, sortDirection: sortDirection) - case .orderedAscending, .orderedDescending: - return comparison == .orderedAscending - } - } else { - return isSortedByDate(article1, article2, sortDirection: sortDirection) - } + if groupByFeed { + return sortedByFeedName(articles: articles, sortByDateDirection: sortDirection) + } else { + return sortedByDate(articles: articles, sortDirection: sortDirection) } - - return articles } // MARK: - + + private static func sortedByFeedName(articles: [T], + sortByDateDirection: ComparisonResult) -> [T] { + // Group articles by feed - feed ID is used to differentiate between + // two feeds that have the same name + var groupedArticles = Dictionary(grouping: articles) { "\($0.sortableName.lowercased())-\($0.sortableFeedID)" } + + // Sort the articles within each group + for tuple in groupedArticles { + groupedArticles[tuple.key] = sortedByDate(articles: tuple.value, + sortDirection: sortByDateDirection) + } + + // Flatten the articles dictionary back into an array sorted by feed name + var sortedArticles: [T] = [] + for feedName in groupedArticles.keys.sorted() { + sortedArticles.append(contentsOf: groupedArticles[feedName] ?? []) + } + + return sortedArticles + } - private static func isSortedByDate(_ lhs: SortableArticle, - _ rhs: SortableArticle, - sortDirection: ComparisonResult) -> Bool { - if lhs.sortableDate == rhs.sortableDate { - return lhs.sortableID < rhs.sortableID + private static func sortedByDate(articles: [T], + sortDirection: ComparisonResult) -> [T] { + let articles = articles.sorted { (article1, article2) -> Bool in + if article1.sortableDate == article2.sortableDate { + return article1.sortableArticleID < article2.sortableArticleID + } + if sortDirection == .orderedDescending { + return article1.sortableDate > article2.sortableDate + } + + return article1.sortableDate < article2.sortableDate } - if sortDirection == .orderedDescending { - return lhs.sortableDate > rhs.sortableDate - } - return lhs.sortableDate < rhs.sortableDate + return articles } } diff --git a/Tests/NetNewsWireTests/ArticleArrayTests.swift b/Tests/NetNewsWireTests/ArticleArrayTests.swift index b5ed8d3d0..dd78ace25 100644 --- a/Tests/NetNewsWireTests/ArticleArrayTests.swift +++ b/Tests/NetNewsWireTests/ArticleArrayTests.swift @@ -14,96 +14,227 @@ import XCTest class ArticleArrayTests: XCTestCase { + // MARK: sortByDate ascending tests + func testSortByDateAscending() { let now = Date() - // Test data includes a mixture of articles in the past and future, as well as articles with the same date - let article1 = TestArticle(sortableName: "Phil's Feed", sortableDate: now, sortableID: "456") - let article2 = TestArticle(sortableName: "Matt's Feed", sortableDate: now, sortableID: "789") - let article3 = TestArticle(sortableName: "Sally's Feed", sortableDate: now, sortableID: "123") - let article4 = TestArticle(sortableName: "Susie's Feed", sortableDate: Date(timeInterval: -60.0, since: now), sortableID: "345") - let article5 = TestArticle(sortableName: "Paul's Feed", sortableDate: Date(timeInterval: -120.0, since: now), sortableID: "567") - let article6 = TestArticle(sortableName: "phil's Feed", sortableDate: Date(timeInterval: 60.0, since: now), sortableID: "567") + let article1 = TestArticle(sortableName: "Susie's Feed", sortableDate: now.addingTimeInterval(-60.0), sortableArticleID: "1", sortableFeedID: "4") + let article2 = TestArticle(sortableName: "Phil's Feed", sortableDate: now.addingTimeInterval(60.0), sortableArticleID: "2", sortableFeedID: "6") + let article3 = TestArticle(sortableName: "Phil's Feed", sortableDate: now.addingTimeInterval(120.0), sortableArticleID: "3", sortableFeedID: "6") + let article4 = TestArticle(sortableName: "Susie's Feed", sortableDate: now.addingTimeInterval(-120.0), sortableArticleID: "4", sortableFeedID: "5") - let articles = [article1, article2, article3, article4, article5, article6] + let articles = [article1, article2, article3, article4] let sortedArticles = ArticleSorter.sortedByDate(articles: articles, sortDirection: .orderedAscending, groupByFeed: false) - XCTAssertEqual(sortedArticles, [article5, article4, article3, article1, article2, article6]) + + XCTAssertEqual(sortedArticles.count, articles.count) + XCTAssertEqual(sortedArticles.articleAtRow(0), article4) + XCTAssertEqual(sortedArticles.articleAtRow(1), article1) + XCTAssertEqual(sortedArticles.articleAtRow(2), article2) + XCTAssertEqual(sortedArticles.articleAtRow(3), article3) + } + + func testSortByDateAscendingWithSameDate() { + let now = Date() + + // Articles with the same date should end up being sorted by their article ID + let article1 = TestArticle(sortableName: "Phil's Feed", sortableDate: now, sortableArticleID: "1", sortableFeedID: "1") + let article2 = TestArticle(sortableName: "Matt's Feed", sortableDate: now, sortableArticleID: "2", sortableFeedID: "2") + let article3 = TestArticle(sortableName: "Sally's Feed", sortableDate: now, sortableArticleID: "3", sortableFeedID: "3") + let article4 = TestArticle(sortableName: "Susie's Feed", sortableDate: Date(timeInterval: -60.0, since: now), sortableArticleID: "4", sortableFeedID: "4") + let article5 = TestArticle(sortableName: "Paul's Feed", sortableDate: Date(timeInterval: -120.0, since: now), sortableArticleID: "5", sortableFeedID: "5") + + let articles = [article1, article2, article3, article4, article5] + let sortedArticles = ArticleSorter.sortedByDate(articles: articles, + sortDirection: .orderedAscending, + groupByFeed: false) + + XCTAssertEqual(sortedArticles.count, articles.count) + XCTAssertEqual(sortedArticles.articleAtRow(0), article5) + XCTAssertEqual(sortedArticles.articleAtRow(1), article4) + XCTAssertEqual(sortedArticles.articleAtRow(2), article1) + XCTAssertEqual(sortedArticles.articleAtRow(3), article2) + XCTAssertEqual(sortedArticles.articleAtRow(4), article3) } func testSortByDateAscendingWithGroupByFeed() { let now = Date() + + let article1 = TestArticle(sortableName: "Phil's Feed", sortableDate: Date(timeInterval: -100.0, since: now), sortableArticleID: "1", sortableFeedID: "1") + let article2 = TestArticle(sortableName: "Jenny's Feed", sortableDate: now, sortableArticleID: "1", sortableFeedID: "2") + let article3 = TestArticle(sortableName: "Jenny's Feed", sortableDate: Date(timeInterval: -10.0, since: now), sortableArticleID: "2", sortableFeedID: "2") + let article4 = TestArticle(sortableName: "Gordy's Blog", sortableDate: Date(timeInterval: -1000.0, since: now), sortableArticleID: "1", sortableFeedID: "3") + let article5 = TestArticle(sortableName: "Gordy's Blog", sortableDate: Date(timeInterval: -10.0, since: now), sortableArticleID: "2", sortableFeedID: "3") + let article6 = TestArticle(sortableName: "Jenny's Feed", sortableDate: Date(timeInterval: 10.0, since: now), sortableArticleID: "3", sortableFeedID: "2") + let article7 = TestArticle(sortableName: "Phil's Feed", sortableDate: now, sortableArticleID: "2", sortableFeedID: "1") + let article8 = TestArticle(sortableName: "Zippy's Feed", sortableDate: now, sortableArticleID: "1", sortableFeedID: "0") + let article9 = TestArticle(sortableName: "Zippy's Feed", sortableDate: now, sortableArticleID: "2", sortableFeedID: "0") - // Test data includes multiple groups (with case-insentive names), articles in the past and future, - // as well as articles with the same date - let article1 = TestArticle(sortableName: "Susie's Feed", sortableDate: Date(timeInterval: -240.0, since: now), sortableID: "123") - let article2 = TestArticle(sortableName: "Phil's Feed", sortableDate: now, sortableID: "456") - let article3 = TestArticle(sortableName: "Matt's Feed", sortableDate: now, sortableID: "234") - let article4 = TestArticle(sortableName: "Susie's Feed", sortableDate: Date(timeInterval: -120.0, since: now), sortableID: "123") - let article5 = TestArticle(sortableName: "phil's feed", sortableDate: Date(timeInterval: 60.0, since: now), sortableID: "456") - let article6 = TestArticle(sortableName: "Matt's Feed", sortableDate: now, sortableID: "123") - let article7 = TestArticle(sortableName: "Susie's Feed", sortableDate: Date(timeInterval: -60.0, since: now), sortableID: "123") - let article8 = TestArticle(sortableName: "phil's Feed", sortableDate: Date(timeInterval: -60.0, since: now), sortableID: "456") - let article9 = TestArticle(sortableName: "Matt's Feed", sortableDate: now, sortableID: "345") - let article10 = TestArticle(sortableName: "Susie's Feed", sortableDate: Date(timeInterval: -15.0, since: now), sortableID: "123") - let article11 = TestArticle(sortableName: "Matt's Feed", sortableDate: Date(timeInterval: 60.0, since: now), sortableID: "123") - let article12 = TestArticle(sortableName: "Claire's Feed", sortableDate: now, sortableID: "123") + let articles = [article1, article2, article3, article4, article5, article6, article7, article8, article9] + let sortedArticles = ArticleSorter.sortedByDate(articles: articles, sortDirection: .orderedAscending, groupByFeed: true) - let articles = [article1, article2, article3, article4, article5, article6, article7, article8, article9, article10, article11, article12] - let sortedArticles = ArticleSorter.sortedByDate(articles: articles, - sortDirection: .orderedAscending, - groupByFeed: true) - XCTAssertEqual(sortedArticles, [article12, article6, article3, article9, article11, article8, article2, article5, article1, article4, article7, article10]) + XCTAssertEqual(sortedArticles.count, 9) + + // Gordy's feed articles + XCTAssertEqual(sortedArticles.articleAtRow(0), article4) + XCTAssertEqual(sortedArticles.articleAtRow(1), article5) + // Jenny's feed articles + XCTAssertEqual(sortedArticles.articleAtRow(2), article3) + XCTAssertEqual(sortedArticles.articleAtRow(3), article2) + XCTAssertEqual(sortedArticles.articleAtRow(4), article6) + // Phil's feed articles + XCTAssertEqual(sortedArticles.articleAtRow(5), article1) + XCTAssertEqual(sortedArticles.articleAtRow(6), article7) + // Zippy's feed articles + XCTAssertEqual(sortedArticles.articleAtRow(7), article8) + XCTAssertEqual(sortedArticles.articleAtRow(8), article9) } + + // MARK: sortByDate descending tests func testSortByDateDescending() { let now = Date() - // Test data includes a mixture of articles in the past and future, as well as articles with the same date - let article1 = TestArticle(sortableName: "Phil's Feed", sortableDate: now, sortableID: "456") - let article2 = TestArticle(sortableName: "Matt's Feed", sortableDate: now, sortableID: "789") - let article3 = TestArticle(sortableName: "Sally's Feed", sortableDate: now, sortableID: "123") - let article4 = TestArticle(sortableName: "Susie's Feed", sortableDate: Date(timeInterval: -60.0, since: now), sortableID: "345") - let article5 = TestArticle(sortableName: "Paul's Feed", sortableDate: Date(timeInterval: -120.0, since: now), sortableID: "567") - let article6 = TestArticle(sortableName: "phil's Feed", sortableDate: Date(timeInterval: 60.0, since: now), sortableID: "567") + let article1 = TestArticle(sortableName: "Susie's Feed", sortableDate: now.addingTimeInterval(-60.0), sortableArticleID: "1", sortableFeedID: "4") + let article2 = TestArticle(sortableName: "Phil's Feed", sortableDate: now.addingTimeInterval(60.0), sortableArticleID: "2", sortableFeedID: "6") + let article3 = TestArticle(sortableName: "Phil's Feed", sortableDate: now.addingTimeInterval(120.0), sortableArticleID: "3", sortableFeedID: "6") + let article4 = TestArticle(sortableName: "Susie's Feed", sortableDate: now.addingTimeInterval(-120.0), sortableArticleID: "4", sortableFeedID: "5") - let articles = [article1, article2, article3, article4, article5, article6] + let articles = [article1, article2, article3, article4] let sortedArticles = ArticleSorter.sortedByDate(articles: articles, sortDirection: .orderedDescending, groupByFeed: false) - XCTAssertEqual(sortedArticles, [article6, article3, article1, article2, article4, article5]) + + XCTAssertEqual(sortedArticles.count, articles.count) + XCTAssertEqual(sortedArticles.articleAtRow(0), article3) + XCTAssertEqual(sortedArticles.articleAtRow(1), article2) + XCTAssertEqual(sortedArticles.articleAtRow(2), article1) + XCTAssertEqual(sortedArticles.articleAtRow(3), article4) + } + + func testSortByDateDescendingWithSameDate() { + let now = Date() + + // Articles with the same date should end up being sorted by their article ID + let article1 = TestArticle(sortableName: "Phil's Feed", sortableDate: now, sortableArticleID: "1", sortableFeedID: "1") + let article2 = TestArticle(sortableName: "Matt's Feed", sortableDate: now, sortableArticleID: "2", sortableFeedID: "2") + let article3 = TestArticle(sortableName: "Sally's Feed", sortableDate: now, sortableArticleID: "3", sortableFeedID: "3") + let article4 = TestArticle(sortableName: "Susie's Feed", sortableDate: Date(timeInterval: -60.0, since: now), sortableArticleID: "4", sortableFeedID: "4") + let article5 = TestArticle(sortableName: "Paul's Feed", sortableDate: Date(timeInterval: -120.0, since: now), sortableArticleID: "5", sortableFeedID: "5") + + let articles = [article1, article2, article3, article4, article5] + let sortedArticles = ArticleSorter.sortedByDate(articles: articles, + sortDirection: .orderedDescending, + groupByFeed: false) + + XCTAssertEqual(sortedArticles.count, articles.count) + XCTAssertEqual(sortedArticles.articleAtRow(0), article1) + XCTAssertEqual(sortedArticles.articleAtRow(1), article2) + XCTAssertEqual(sortedArticles.articleAtRow(2), article3) + XCTAssertEqual(sortedArticles.articleAtRow(3), article4) + XCTAssertEqual(sortedArticles.articleAtRow(4), article5) } func testSortByDateDescendingWithGroupByFeed() { let now = Date() + + let article1 = TestArticle(sortableName: "Phil's Feed", sortableDate: Date(timeInterval: -100.0, since: now), sortableArticleID: "1", sortableFeedID: "1") + let article2 = TestArticle(sortableName: "Jenny's Feed", sortableDate: now, sortableArticleID: "1", sortableFeedID: "2") + let article3 = TestArticle(sortableName: "Jenny's Feed", sortableDate: Date(timeInterval: -10.0, since: now), sortableArticleID: "2", sortableFeedID: "2") + let article4 = TestArticle(sortableName: "Gordy's Blog", sortableDate: Date(timeInterval: -1000.0, since: now), sortableArticleID: "1", sortableFeedID: "3") + let article5 = TestArticle(sortableName: "Gordy's Blog", sortableDate: Date(timeInterval: -10.0, since: now), sortableArticleID: "2", sortableFeedID: "3") + let article6 = TestArticle(sortableName: "Jenny's Feed", sortableDate: Date(timeInterval: 10.0, since: now), sortableArticleID: "3", sortableFeedID: "2") + let article7 = TestArticle(sortableName: "Phil's Feed", sortableDate: now, sortableArticleID: "2", sortableFeedID: "1") + let article8 = TestArticle(sortableName: "Zippy's Feed", sortableDate: now, sortableArticleID: "1", sortableFeedID: "0") + let article9 = TestArticle(sortableName: "Zippy's Feed", sortableDate: now, sortableArticleID: "2", sortableFeedID: "0") - // Test data includes multiple groups (with case-insentive names), articles in the past and future, - // as well as articles with the same date - let article1 = TestArticle(sortableName: "Susie's Feed", sortableDate: Date(timeInterval: -240.0, since: now), sortableID: "123") - let article2 = TestArticle(sortableName: "Phil's Feed", sortableDate: now, sortableID: "456") - let article3 = TestArticle(sortableName: "Matt's Feed", sortableDate: now, sortableID: "234") - let article4 = TestArticle(sortableName: "Susie's Feed", sortableDate: Date(timeInterval: -120.0, since: now), sortableID: "123") - let article5 = TestArticle(sortableName: "phil's feed", sortableDate: Date(timeInterval: 60.0, since: now), sortableID: "456") - let article6 = TestArticle(sortableName: "Matt's Feed", sortableDate: now, sortableID: "123") - let article7 = TestArticle(sortableName: "Susie's Feed", sortableDate: Date(timeInterval: -60.0, since: now), sortableID: "123") - let article8 = TestArticle(sortableName: "phil's Feed", sortableDate: Date(timeInterval: -60.0, since: now), sortableID: "456") - let article9 = TestArticle(sortableName: "Matt's Feed", sortableDate: now, sortableID: "345") - let article10 = TestArticle(sortableName: "Susie's Feed", sortableDate: Date(timeInterval: -15.0, since: now), sortableID: "123") - let article11 = TestArticle(sortableName: "Matt's Feed", sortableDate: Date(timeInterval: 60.0, since: now), sortableID: "123") - let article12 = TestArticle(sortableName: "Claire's Feed", sortableDate: now, sortableID: "123") + let articles = [article1, article2, article3, article4, article5, article6, article7, article8, article9] + let sortedArticles = ArticleSorter.sortedByDate(articles: articles, sortDirection: .orderedDescending, groupByFeed: true) - let articles = [article1, article2, article3, article4, article5, article6, article7, article8, article9, article10, article11, article12] - let sortedArticles = ArticleSorter.sortedByDate(articles: articles, - sortDirection: .orderedDescending, - groupByFeed: true) - XCTAssertEqual(sortedArticles, [article12, article11, article6, article3, article9, article5, article2, article8, article10, article7, article4, article1]) + XCTAssertEqual(sortedArticles.count, 9) + + // Gordy's feed articles + XCTAssertEqual(sortedArticles.articleAtRow(0), article5) + XCTAssertEqual(sortedArticles.articleAtRow(1), article4) + // Jenny's feed articles + XCTAssertEqual(sortedArticles.articleAtRow(2), article6) + XCTAssertEqual(sortedArticles.articleAtRow(3), article2) + XCTAssertEqual(sortedArticles.articleAtRow(4), article3) + // Phil's feed articles + XCTAssertEqual(sortedArticles.articleAtRow(5), article7) + XCTAssertEqual(sortedArticles.articleAtRow(6), article1) + // Zippy's feed articles + XCTAssertEqual(sortedArticles.articleAtRow(7), article8) + XCTAssertEqual(sortedArticles.articleAtRow(8), article9) } - + + // MARK: Additional group by feed tests + + func testGroupByFeedWithCaseInsensitiveFeedNames() { + let now = Date() + + let article1 = TestArticle(sortableName: "phil's feed", sortableDate: now, sortableArticleID: "1", sortableFeedID: "1") + let article2 = TestArticle(sortableName: "PhIl's FEed", sortableDate: now, sortableArticleID: "2", sortableFeedID: "1") + let article3 = TestArticle(sortableName: "APPLE's feed", sortableDate: now, sortableArticleID: "3", sortableFeedID: "2") + let article4 = TestArticle(sortableName: "PHIL'S FEED", sortableDate: now, sortableArticleID: "4", sortableFeedID: "1") + let article5 = TestArticle(sortableName: "apple's feed", sortableDate: now, sortableArticleID: "5", sortableFeedID: "2") + + let articles = [article1, article2, article3, article4, article5] + let sortedArticles = ArticleSorter.sortedByDate(articles: articles, + sortDirection: .orderedAscending, + groupByFeed: true) + + XCTAssertEqual(sortedArticles.count, articles.count) + + // Apple's feed articles + XCTAssertEqual(sortedArticles.articleAtRow(0), article3) + XCTAssertEqual(sortedArticles.articleAtRow(1), article5) + // Phil's feed articles + XCTAssertEqual(sortedArticles.articleAtRow(2), article1) + XCTAssertEqual(sortedArticles.articleAtRow(3), article2) + XCTAssertEqual(sortedArticles.articleAtRow(4), article4) + } + + func testGroupByFeedWithSameFeedNames() { + let now = Date() + + // Articles with the same feed name should be sorted by feed ID + let article1 = TestArticle(sortableName: "Phil's Feed", sortableDate: now, sortableArticleID: "1", sortableFeedID: "2") + let article2 = TestArticle(sortableName: "Phil's Feed", sortableDate: now, sortableArticleID: "2", sortableFeedID: "2") + let article3 = TestArticle(sortableName: "Phil's Feed", sortableDate: now, sortableArticleID: "3", sortableFeedID: "1") + let article4 = TestArticle(sortableName: "Phil's Feed", sortableDate: now, sortableArticleID: "4", sortableFeedID: "2") + let article5 = TestArticle(sortableName: "Phil's Feed", sortableDate: now, sortableArticleID: "5", sortableFeedID: "1") + + let articles = [article1, article2, article3, article4, article5] + let sortedArticles = ArticleSorter.sortedByDate(articles: articles, + sortDirection: .orderedAscending, + groupByFeed: true) + + XCTAssertEqual(sortedArticles.count, articles.count) + XCTAssertEqual(sortedArticles.articleAtRow(0), article3) + XCTAssertEqual(sortedArticles.articleAtRow(1), article5) + XCTAssertEqual(sortedArticles.articleAtRow(2), article1) + XCTAssertEqual(sortedArticles.articleAtRow(3), article2) + XCTAssertEqual(sortedArticles.articleAtRow(4), article4) + } + } private struct TestArticle: SortableArticle, Equatable { let sortableName: String let sortableDate: Date - let sortableID: String + let sortableArticleID: String + let sortableFeedID: String +} + +private extension Array where Element == TestArticle { + func articleAtRow(_ row: Int) -> TestArticle? { + if row < 0 || row == NSNotFound || row > count - 1 { + return nil + } + return self[row] + } + } From 61d3dae10eb0e80382aa372fcd548aa1c693c35e Mon Sep 17 00:00:00 2001 From: Phil Viso Date: Fri, 13 Sep 2019 08:03:56 -0500 Subject: [PATCH 08/10] Re-wrote sorting code to be smaller and easier to understand --- Shared/Timeline/ArticleSorter.swift | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/Shared/Timeline/ArticleSorter.swift b/Shared/Timeline/ArticleSorter.swift index 9196ee765..c7ba1a748 100644 --- a/Shared/Timeline/ArticleSorter.swift +++ b/Shared/Timeline/ArticleSorter.swift @@ -32,28 +32,21 @@ struct ArticleSorter { private static func sortedByFeedName(articles: [T], sortByDateDirection: ComparisonResult) -> [T] { - // Group articles by feed - feed ID is used to differentiate between + // Group articles by "feed-feedID" - feed ID is used to differentiate between // two feeds that have the same name - var groupedArticles = Dictionary(grouping: articles) { "\($0.sortableName.lowercased())-\($0.sortableFeedID)" } - - // Sort the articles within each group - for tuple in groupedArticles { - groupedArticles[tuple.key] = sortedByDate(articles: tuple.value, - sortDirection: sortByDateDirection) + let groupedArticles = Dictionary(grouping: articles) { "\($0.sortableName.lowercased())-\($0.sortableFeedID)" } + return groupedArticles + .sorted { $0.key < $1.key } + .flatMap { (tuple) -> [T] in + let (_, articles) = tuple + + return sortedByDate(articles: articles, sortDirection: sortByDateDirection) } - - // Flatten the articles dictionary back into an array sorted by feed name - var sortedArticles: [T] = [] - for feedName in groupedArticles.keys.sorted() { - sortedArticles.append(contentsOf: groupedArticles[feedName] ?? []) - } - - return sortedArticles } private static func sortedByDate(articles: [T], sortDirection: ComparisonResult) -> [T] { - let articles = articles.sorted { (article1, article2) -> Bool in + return articles.sorted { (article1, article2) -> Bool in if article1.sortableDate == article2.sortableDate { return article1.sortableArticleID < article2.sortableArticleID } @@ -63,7 +56,6 @@ struct ArticleSorter { return article1.sortableDate < article2.sortableDate } - return articles } } From cc6767e0f6b1a71bfbc588a1fcd4ff6bb6c6d2df Mon Sep 17 00:00:00 2001 From: Phil Viso Date: Fri, 13 Sep 2019 08:29:56 -0500 Subject: [PATCH 09/10] Removed duplicate sort parameter change handling functions --- .../Timeline/TimelineViewController.swift | 13 +++---------- iOS/SceneCoordinator.swift | 12 ++++-------- 2 files changed, 7 insertions(+), 18 deletions(-) diff --git a/Mac/MainWindow/Timeline/TimelineViewController.swift b/Mac/MainWindow/Timeline/TimelineViewController.swift index bda2a0ba6..16a694d09 100644 --- a/Mac/MainWindow/Timeline/TimelineViewController.swift +++ b/Mac/MainWindow/Timeline/TimelineViewController.swift @@ -126,14 +126,14 @@ final class TimelineViewController: NSViewController, UndoableCommandRunner, Unr private var sortDirection = AppDefaults.timelineSortDirection { didSet { if sortDirection != oldValue { - sortDirectionDidChange() + sortParametersDidChange() } } } private var groupByFeed = AppDefaults.timelineGroupByFeed { didSet { if groupByFeed != oldValue { - groupByFeedDidChange() + sortParametersDidChange() } } } @@ -884,20 +884,13 @@ private extension TimelineViewController { } } - func sortDirectionDidChange() { + func sortParametersDidChange() { performBlockAndRestoreSelection { let unsortedArticles = Set(articles) replaceArticles(with: unsortedArticles) } } - func groupByFeedDidChange() { - performBlockAndRestoreSelection { - let unsortedArticles = Set(articles) - replaceArticles(with: unsortedArticles) - } - } - func selectedArticleIDs() -> [String] { return selectedArticles.articleIDs() diff --git a/iOS/SceneCoordinator.swift b/iOS/SceneCoordinator.swift index f249a77d1..275a2d3ac 100644 --- a/iOS/SceneCoordinator.swift +++ b/iOS/SceneCoordinator.swift @@ -66,14 +66,14 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { private(set) var sortDirection = AppDefaults.timelineSortDirection { didSet { if sortDirection != oldValue { - sortDirectionDidChange() + sortParametersDidChange() } } } private(set) var groupByFeed = AppDefaults.timelineGroupByFeed { didSet { if groupByFeed != oldValue { - groupByFeedDidChange() + sortParametersDidChange() } } } @@ -1234,14 +1234,10 @@ private extension SceneCoordinator { } } - func sortDirectionDidChange() { + func sortParametersDidChange() { replaceArticles(with: Set(articles), animate: true) } - - func groupByFeedDidChange() { - replaceArticles(with: Set(articles), animate: true) - } - + func replaceArticles(with unsortedArticles: Set
, animate: Bool) { let sortedArticles = Array(unsortedArticles).sortedByDate(sortDirection, groupByFeed: groupByFeed) From 365a98b33a3112bfa142e4eb4167a2f653640ee8 Mon Sep 17 00:00:00 2001 From: Phil Viso Date: Fri, 13 Sep 2019 08:59:31 -0500 Subject: [PATCH 10/10] Updated test name to match the class its testing --- NetNewsWire.xcodeproj/project.pbxproj | 8 ++++---- .../{ArticleArrayTests.swift => ArticleSorterTests.swift} | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) rename Tests/NetNewsWireTests/{ArticleArrayTests.swift => ArticleSorterTests.swift} (99%) diff --git a/NetNewsWire.xcodeproj/project.pbxproj b/NetNewsWire.xcodeproj/project.pbxproj index a5f798a12..0f6df4fbc 100644 --- a/NetNewsWire.xcodeproj/project.pbxproj +++ b/NetNewsWire.xcodeproj/project.pbxproj @@ -366,7 +366,7 @@ D5F4EDB920074D7C00B9E363 /* Folder+Scriptability.swift in Sources */ = {isa = PBXBuildFile; fileRef = D5F4EDB820074D7C00B9E363 /* Folder+Scriptability.swift */; }; DD82AB0A231003F6002269DF /* SharingTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = DD82AB09231003F6002269DF /* SharingTests.swift */; }; DF999FF722B5AEFA0064B687 /* SafariView.swift in Sources */ = {isa = PBXBuildFile; fileRef = DF999FF622B5AEFA0064B687 /* SafariView.swift */; }; - FF3ABF13232599810074C542 /* ArticleArrayTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = FF3ABF09232599450074C542 /* ArticleArrayTests.swift */; }; + FF3ABF13232599810074C542 /* ArticleSorterTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = FF3ABF09232599450074C542 /* ArticleSorterTests.swift */; }; FF3ABF1523259DDB0074C542 /* ArticleSorter.swift in Sources */ = {isa = PBXBuildFile; fileRef = FF3ABF1423259DDB0074C542 /* ArticleSorter.swift */; }; FF3ABF162325AF5D0074C542 /* ArticleSorter.swift in Sources */ = {isa = PBXBuildFile; fileRef = FF3ABF1423259DDB0074C542 /* ArticleSorter.swift */; }; /* End PBXBuildFile section */ @@ -1049,7 +1049,7 @@ D5F4EDB820074D7C00B9E363 /* Folder+Scriptability.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Folder+Scriptability.swift"; sourceTree = ""; }; DD82AB09231003F6002269DF /* SharingTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SharingTests.swift; sourceTree = ""; }; DF999FF622B5AEFA0064B687 /* SafariView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SafariView.swift; sourceTree = ""; }; - FF3ABF09232599450074C542 /* ArticleArrayTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ArticleArrayTests.swift; sourceTree = ""; }; + FF3ABF09232599450074C542 /* ArticleSorterTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ArticleSorterTests.swift; sourceTree = ""; }; FF3ABF1423259DDB0074C542 /* ArticleSorter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ArticleSorter.swift; sourceTree = ""; }; /* End PBXFileReference section */ @@ -1985,7 +1985,7 @@ isa = PBXGroup; children = ( 84F9EAD0213660A100CF2DE4 /* ScriptingTests */, - FF3ABF09232599450074C542 /* ArticleArrayTests.swift */, + FF3ABF09232599450074C542 /* ArticleSorterTests.swift */, 84F9EAE3213660A100CF2DE4 /* NetNewsWireTests.swift */, DD82AB09231003F6002269DF /* SharingTests.swift */, 84F9EAE4213660A100CF2DE4 /* Info.plist */, @@ -2864,7 +2864,7 @@ isa = PBXSourcesBuildPhase; buildActionMask = 2147483647; files = ( - FF3ABF13232599810074C542 /* ArticleArrayTests.swift in Sources */, + FF3ABF13232599810074C542 /* ArticleSorterTests.swift in Sources */, DD82AB0A231003F6002269DF /* SharingTests.swift in Sources */, 84F9EAEB213660A100CF2DE4 /* testIterativeCreateAndDeleteFeed.applescript in Sources */, 84F9EAF4213660A100CF2DE4 /* testGenericScript.applescript in Sources */, diff --git a/Tests/NetNewsWireTests/ArticleArrayTests.swift b/Tests/NetNewsWireTests/ArticleSorterTests.swift similarity index 99% rename from Tests/NetNewsWireTests/ArticleArrayTests.swift rename to Tests/NetNewsWireTests/ArticleSorterTests.swift index dd78ace25..1991e88ff 100644 --- a/Tests/NetNewsWireTests/ArticleArrayTests.swift +++ b/Tests/NetNewsWireTests/ArticleSorterTests.swift @@ -1,5 +1,5 @@ // -// ArticleArrayTests.swift +// ArticleSorterTests.swift // NetNewsWire // // Created by Phil Viso on 9/8/19. @@ -12,7 +12,7 @@ import XCTest @testable import NetNewsWire -class ArticleArrayTests: XCTestCase { +class ArticleSorterTests: XCTestCase { // MARK: sortByDate ascending tests