Merge pull request #19360 from ipalm0423/issue/19075/fix-cached-notifications-appear-when-logout

twstokes · web-flow · commit c4bd00da79bd · 2022-10-04T13:44:07.000-04:00
Fix cached notifications appear when logout
diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt
@@ -1,6 +1,6 @@
 21.0
 -----
-
+* [*] Fixed an issue where the cached notifications are retained after logging out of WordPress.com account [#19360]
 
 20.9
 -----
diff --git a/WordPress/Classes/ViewRelated/Notifications/Controllers/NotificationsViewController+JetpackPrompt.swift b/WordPress/Classes/ViewRelated/Notifications/Controllers/NotificationsViewController+JetpackPrompt.swift
@@ -1,7 +1,11 @@
 extension NotificationsViewController {
 
+    var blogForJetpackPrompt: Blog? {
+        return Blog.lastUsed(in: managedObjectContext())
+    }
+
     func promptForJetpackCredentials() {
-        guard let blog = Blog.lastUsed(in: managedObjectContext()) else {
+        guard let blog = blogForJetpackPrompt else {
             return
         }
 
@@ -14,9 +18,16 @@ extension NotificationsViewController {
             controller.promptType = .notifications
             addChild(controller)
             tableView.addSubview(withFadeAnimation: controller.view)
-            controller.view.frame = CGRect(origin: .zero, size: view.frame.size)
+            controller.view.translatesAutoresizingMaskIntoConstraints = false
+            NSLayoutConstraint.activate([
+                controller.view.topAnchor.constraint(equalTo: tableView.safeAreaLayoutGuide.topAnchor),
+                controller.view.leadingAnchor.constraint(equalTo: tableView.safeAreaLayoutGuide.leadingAnchor),
+                controller.view.trailingAnchor.constraint(equalTo: tableView.safeAreaLayoutGuide.trailingAnchor),
+                controller.view.bottomAnchor.constraint(equalTo: tableView.safeAreaLayoutGuide.bottomAnchor)
+            ])
             configureControllerCompletion(controller, withBlog: blog)
             jetpackLoginViewController = controller
+            controller.didMove(toParent: self)
         }
     }
 
diff --git a/WordPress/Classes/ViewRelated/Notifications/Controllers/NotificationsViewController.swift b/WordPress/Classes/ViewRelated/Notifications/Controllers/NotificationsViewController.swift
@@ -56,7 +56,7 @@ class NotificationsViewController: UIViewController, UIViewControllerRestoration
 
     /// Indicates whether the view is required to reload results on viewWillAppear, or not
     ///
-    fileprivate var needsReloadResults = false
+    var needsReloadResults = false
 
     /// Cached values used for returning the estimated row heights of autosizing cells.
     ///
@@ -137,6 +137,16 @@ class NotificationsViewController: UIViewController, UIViewControllerRestoration
     /// Used by JPScrollViewDelegate to send scroll position
     internal let scrollViewTranslationPublisher = PassthroughSubject<Bool, Never>()
 
+    /// The last time when user seen notifications
+    var lastSeenTime: String? {
+        get {
+            return userDefaults.string(forKey: Settings.lastSeenTime)
+        }
+        set {
+            userDefaults.set(newValue, forKey: Settings.lastSeenTime)
+        }
+    }
+
     // MARK: - View Lifecycle
 
     required init?(coder aDecoder: NSCoder) {
@@ -200,11 +210,10 @@ class NotificationsViewController: UIViewController, UIViewControllerRestoration
             reloadTableViewPreservingSelection()
         }
 
-        if !AccountHelper.isDotcomAvailable() {
+        if shouldDisplayJetpackPrompt {
             promptForJetpackCredentials()
         } else {
-            jetpackLoginViewController?.view.removeFromSuperview()
-            jetpackLoginViewController?.removeFromParent()
+            jetpackLoginViewController?.remove()
         }
 
         showNoResultsViewIfNeeded()
@@ -686,14 +695,14 @@ private extension NotificationsViewController {
     }
 
     @objc func defaultAccountDidChange(_ note: Foundation.Notification) {
-        guard isViewLoaded == true && view.window != nil else {
-            return
-        }
-
-        needsReloadResults = true
         resetNotifications()
         resetLastSeenTime()
         resetApplicationBadge()
+        guard isViewLoaded == true && view.window != nil else {
+            needsReloadResults = true
+            return
+        }
+        reloadResultsController()
         syncNewNotifications()
     }
 
@@ -1558,7 +1567,11 @@ private extension NotificationsViewController {
             return
         }
 
-        let columnWidth: WPSplitViewControllerPrimaryColumnWidth = (shouldDisplayFullscreenNoResultsView || shouldDisplayJetpackPrompt) ? .full : .default
+        // Ref: https://github.com/wordpress-mobile/WordPress-iOS/issues/14547
+        // Don't attempt to resize the columns for full width.
+        let columnWidth: WPSplitViewControllerPrimaryColumnWidth = .default
+        // The above line should be replace with the following line when the full width issue is resolved.
+        // let columnWidth: WPSplitViewControllerPrimaryColumnWidth = (shouldDisplayFullscreenNoResultsView || shouldDisplayJetpackPrompt) ? .full : .default
 
         if splitViewController.wpPrimaryColumnWidth != columnWidth {
             splitViewController.wpPrimaryColumnWidth = columnWidth
@@ -1592,7 +1605,7 @@ private extension NotificationsViewController {
     }
 
     var shouldDisplayJetpackPrompt: Bool {
-        return AccountHelper.isDotcomAvailable() == false
+        return AccountHelper.isDotcomAvailable() == false && blogForJetpackPrompt != nil
     }
 
     var shouldDisplaySettingsButton: Bool {
@@ -1758,7 +1771,6 @@ private extension NotificationsViewController {
             selectedNotification = nil
             mainContext.deleteAllObjects(ofType: Notification.self)
             try mainContext.save()
-            tableView.reloadData()
         } catch {
             DDLogError("Error while trying to nuke Notifications Collection: [\(error)]")
         }
@@ -1851,15 +1863,6 @@ private extension NotificationsViewController {
         return UserPersistentStoreFactory.instance()
     }
 
-    var lastSeenTime: String? {
-        get {
-            return userDefaults.string(forKey: Settings.lastSeenTime)
-        }
-        set {
-            userDefaults.set(newValue, forKey: Settings.lastSeenTime)
-        }
-    }
-
     var filter: Filter {
         get {
             let selectedIndex = filterTabBar?.selectedIndex ?? Filter.none.rawValue
@@ -2079,3 +2082,11 @@ extension NotificationsViewController: JPScrollViewDelegate {
         processJetpackBannerVisibility(scrollView)
     }
 }
+
+// MARK: - StoryboardLoadable
+
+extension NotificationsViewController: StoryboardLoadable {
+    static var defaultStoryboardName: String {
+        return "Notifications"
+    }
+}
diff --git a/WordPress/Classes/ViewRelated/Notifications/Notifications.storyboard b/WordPress/Classes/ViewRelated/Notifications/Notifications.storyboard
@@ -1,17 +1,17 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<document type="com.apple.InterfaceBuilder3.CocoaTouch.Storyboard.XIB" version="3.0" toolsVersion="20037" targetRuntime="iOS.CocoaTouch" propertyAccessControl="none" useAutolayout="YES" useTraitCollections="YES" colorMatched="YES" initialViewController="aRr-Yu-ntD">
+<document type="com.apple.InterfaceBuilder3.CocoaTouch.Storyboard.XIB" version="3.0" toolsVersion="21225" targetRuntime="iOS.CocoaTouch" propertyAccessControl="none" useAutolayout="YES" useTraitCollections="YES" colorMatched="YES" initialViewController="aRr-Yu-ntD">
     <device id="retina4_7" orientation="portrait" appearance="light"/>
     <dependencies>
         <deployment identifier="iOS"/>
-        <plugIn identifier="com.apple.InterfaceBuilder.IBCocoaTouchPlugin" version="20020"/>
+        <plugIn identifier="com.apple.InterfaceBuilder.IBCocoaTouchPlugin" version="21207"/>
         <capability name="System colors in document resources" minToolsVersion="11.0"/>
         <capability name="documents saved in the Xcode 8 format" minToolsVersion="8.0"/>
     </dependencies>
     <scenes>
         <!--Notifications View Controller-->
         <scene sceneID="30e-Wx-jwX">
             <objects>
-                <viewController extendedLayoutIncludesOpaqueBars="YES" id="aRr-Yu-ntD" customClass="NotificationsViewController" customModule="WordPress" customModuleProvider="target" sceneMemberID="viewController">
+                <viewController storyboardIdentifier="NotificationsViewController" extendedLayoutIncludesOpaqueBars="YES" id="aRr-Yu-ntD" customClass="NotificationsViewController" customModule="WordPress" customModuleProvider="target" sceneMemberID="viewController">
                     <layoutGuides>
                         <viewControllerLayoutGuide type="top" id="6eI-Lt-Q4C"/>
                         <viewControllerLayoutGuide type="bottom" id="X8S-C3-Z2w"/>
diff --git a/WordPress/WordPress.xcodeproj/project.pbxproj b/WordPress/WordPress.xcodeproj/project.pbxproj
@@ -2123,6 +2123,7 @@
 		AB2211F425ED6E7A00BF72FC /* CommentServiceTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = AB2211F325ED6E7A00BF72FC /* CommentServiceTests.swift */; };
 		AB758D9E25EFDF9C00961C0B /* LikesListController.swift in Sources */ = {isa = PBXBuildFile; fileRef = AB758D9D25EFDF9C00961C0B /* LikesListController.swift */; };
 		ACACE3AE28D729FA000992F9 /* NoResultsViewControllerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = ACACE3AD28D729FA000992F9 /* NoResultsViewControllerTests.swift */; };
+		AC68C9CA28E5DF14009030A9 /* NotificationsViewControllerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = AC68C9C928E5DF14009030A9 /* NotificationsViewControllerTests.swift */; };
 		ACBAB5FE0E121C7300F38795 /* PostSettingsViewController.m in Sources */ = {isa = PBXBuildFile; fileRef = ACBAB5FD0E121C7300F38795 /* PostSettingsViewController.m */; };
 		ADF544C2195A0F620092213D /* CustomHighlightButton.m in Sources */ = {isa = PBXBuildFile; fileRef = ADF544C1195A0F620092213D /* CustomHighlightButton.m */; };
 		AE2F3125270B6DA000B2A9C2 /* Scanner+QuotedText.swift in Sources */ = {isa = PBXBuildFile; fileRef = AE2F3124270B6DA000B2A9C2 /* Scanner+QuotedText.swift */; };
@@ -7034,6 +7035,7 @@
 		AB390AA9C94F16E78184E9D1 /* Pods_WordPressScreenshotGeneration.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_WordPressScreenshotGeneration.framework; sourceTree = BUILT_PRODUCTS_DIR; };
 		AB758D9D25EFDF9C00961C0B /* LikesListController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LikesListController.swift; sourceTree = "<group>"; };
 		ACACE3AD28D729FA000992F9 /* NoResultsViewControllerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NoResultsViewControllerTests.swift; sourceTree = "<group>"; };
+		AC68C9C928E5DF14009030A9 /* NotificationsViewControllerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NotificationsViewControllerTests.swift; sourceTree = "<group>"; };
 		ACBAB5FC0E121C7300F38795 /* PostSettingsViewController.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = PostSettingsViewController.h; sourceTree = "<group>"; usesTabs = 0; };
 		ACBAB5FD0E121C7300F38795 /* PostSettingsViewController.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = PostSettingsViewController.m; sourceTree = "<group>"; usesTabs = 0; };
 		ADE06D6829F9044164BBA5AB /* Pods-WordPressIntents.release.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-WordPressIntents.release.xcconfig"; path = "../Pods/Target Support Files/Pods-WordPressIntents/Pods-WordPressIntents.release.xcconfig"; sourceTree = "<group>"; };
@@ -13325,6 +13327,22 @@
 			path = Controllers;
 			sourceTree = "<group>";
 		};
+		AC68C9C728E5DEB1009030A9 /* Notification */ = {
+			isa = PBXGroup;
+			children = (
+				AC68C9C828E5DEE0009030A9 /* Controllers */,
+			);
+			path = Notification;
+			sourceTree = "<group>";
+		};
+		AC68C9C828E5DEE0009030A9 /* Controllers */ = {
+			isa = PBXGroup;
+			children = (
+				AC68C9C928E5DF14009030A9 /* NotificationsViewControllerTests.swift */,
+			);
+			path = Controllers;
+			sourceTree = "<group>";
+		};
 		B0088A8F283D68B1008C9676 /* Stats Revamp v2 */ = {
 			isa = PBXGroup;
 			children = (
@@ -14078,6 +14096,7 @@
 		BE20F5E11B2F738E0020694C /* ViewRelated */ = {
 			isa = PBXGroup;
 			children = (
+				AC68C9C728E5DEB1009030A9 /* Notification */,
 				C3C2F84428AC8B9E00937E45 /* Jetpack */,
 				F44FB6C92878957E0001E3CE /* Mention */,
 				8B69F0E2255C2BC0006B1CEF /* Activity */,
@@ -20794,6 +20813,7 @@
 				9A9D34FF2360A4E200BC95A3 /* StatsPeriodAsyncOperationTests.swift in Sources */,
 				8BE9AB8827B6B5A300708E45 /* BlogDashboardPersistenceTests.swift in Sources */,
 				B5EFB1C91B333C5A007608A3 /* NotificationSettingsServiceTests.swift in Sources */,
+				AC68C9CA28E5DF14009030A9 /* NotificationsViewControllerTests.swift in Sources */,
 				F44FB6CD287897F90001E3CE /* SuggestionsTableViewMockDelegate.swift in Sources */,
 				179501CD27A01D4100882787 /* PublicizeAuthorizationURLComponentsTests.swift in Sources */,
 				4089C51422371EE30031CE78 /* TodayStatsTests.swift in Sources */,
diff --git a/WordPress/WordPressTest/Notification/Controllers/NotificationsViewControllerTests.swift b/WordPress/WordPressTest/Notification/Controllers/NotificationsViewControllerTests.swift
@@ -0,0 +1,85 @@
+import XCTest
+@testable import WordPress
+
+final class NotificationsViewControllerTests: XCTestCase {
+
+    private var controller: NotificationsViewController!
+    private var contextManager: ContextManagerMock!
+    private var utility: NotificationUtility!
+
+    override func setUpWithError() throws {
+        contextManager = ContextManagerMock()
+        contextManager.useAsSharedInstance(untilTestFinished: self)
+        utility = NotificationUtility(coreDataStack: contextManager)
+        controller = NotificationsViewController.loadFromStoryboard()
+    }
+
+    override func tearDownWithError() throws {
+        controller = nil
+        contextManager = nil
+        utility = nil
+    }
+
+    func testResetNotificationsWhenAccountChange() throws {
+        // Give
+        _ = try utility.loadBadgeNotification()
+
+        // When
+        postAccountChangeNotification()
+
+        // Then
+        XCTAssertEqual(notificationCount, 0)
+    }
+
+    func testResetLastSeenTimeWhenAccountChange() throws {
+        // Give
+        controller.lastSeenTime = "testTime"
+
+        // When
+        postAccountChangeNotification()
+
+        // Then
+        XCTAssertEqual(controller.lastSeenTime, nil)
+    }
+
+    func testResetApplicationBadgeWhenAccountChange() throws {
+        // Give
+        let newUnreadCount = 1
+        UIApplication.shared.applicationIconBadgeNumber = 0
+        ZendeskUtils.unreadNotificationsCount = newUnreadCount
+
+        // When
+        postAccountChangeNotification()
+
+        // Then
+        XCTAssertEqual(UIApplication.shared.applicationIconBadgeNumber, newUnreadCount)
+    }
+
+    func testNeedsReloadResultsWhenAccountChange() throws {
+        // Give
+        controller.needsReloadResults = false
+
+        // When
+        postAccountChangeNotification()
+
+        // Then
+        XCTAssertEqual(controller.needsReloadResults, true)
+    }
+
+}
+
+private extension NotificationsViewControllerTests {
+
+    enum Constants {
+        static let entityName = "Notification"
+    }
+
+    var notificationCount: Int? {
+        let request = NSFetchRequest<NSFetchRequestResult>(entityName: Constants.entityName)
+        return try? contextManager.mainContext.count(for: request)
+    }
+
+    func postAccountChangeNotification() {
+        NotificationCenter.default.post(name: .WPAccountDefaultWordPressComAccountChanged, object: nil)
+    }
+}

-Original file line number
+Diff line change
@@ @@ -1,6 +1,6 @@ @@
 .0
 -----
+-
 +* [*] Fixed an issue where the cached notifications are retained after logging out of WordPress.com account [#19360]
 .9
 -----