Merge UserDataProvider and UserManagementActionDispatcher into one type (#23792)

* Merge UserDataProvider and UserManagementActionDispatcher into one type

* Cache current user info in UserService

* Minor change to function declarations

* Add unit tests for UserService

* Use AsyncStream to publish updates instead of Combine publisher

* Remove some whitespaces

* Terminate users updates stream upon deallocating

* Use `onAppear` instead of `Task` to avoid duplicated calls

Author: crazytonyli

Modules/Sources/WordPressUI/Views/Users/Components/UserListItem.swift

@@ -9,18 +9,16 @@ struct UserListItem: View {
     var dynamicTypeSize
 
     private let user: DisplayUser
-    private let userProvider: UserDataProvider
-    private let actionDispatcher: UserManagementActionDispatcher
+    private let userService: UserServiceProtocol
 
-    init(user: DisplayUser, userProvider: UserDataProvider, actionDispatcher: UserManagementActionDispatcher) {
+    init(user: DisplayUser, userService: UserServiceProtocol) {
         self.user = user
-        self.userProvider = userProvider
-        self.actionDispatcher = actionDispatcher
+        self.userService = userService
     }
 
     var body: some View {
         NavigationLink {
-            UserDetailsView(user: user, userProvider: userProvider, actionDispatcher: actionDispatcher)
+            UserDetailsView(user: user, userService: userService)
         } label: {
             HStack(alignment: .top) {
                 if !dynamicTypeSize.isAccessibilitySize {
@@ -36,5 +34,5 @@ struct UserListItem: View {
 }
 
 #Preview {
-    UserListItem(user: DisplayUser.MockUser, userProvider: MockUserProvider(), actionDispatcher: UserManagementActionDispatcher())
+    UserListItem(user: DisplayUser.MockUser, userService: MockUserProvider())
 }

Modules/Sources/WordPressUI/Views/Users/UserProvider.swift

@@ -1,32 +1,20 @@
 import Foundation
+import Combine
 
-public protocol UserDataProvider {
+public protocol UserServiceProtocol: Actor {
+    var users: [DisplayUser]? { get }
+    nonisolated var usersUpdates: AsyncStream<[DisplayUser]> { get }
 
-    typealias CachedUserListCallback = ([WordPressUI.DisplayUser]) async -> Void
+    func fetchUsers() async throws -> [DisplayUser]
 
-    func fetchCurrentUserCan(_ capability: String) async throws -> Bool
-    func fetchUsers(cachedResults: CachedUserListCallback?) async throws -> [WordPressUI.DisplayUser]
+    func isCurrentUserCapableOf(_ capability: String) async throws -> Bool
 
-    func invalidateCaches() async throws
-}
-
-/// Subclass this and register it with the SwiftUI `.environmentObject` method
-/// to perform user management actions.
-///
-/// The default implementation is set up for testing with SwiftUI Previews
-open class UserManagementActionDispatcher: ObservableObject {
-    public init() {}
-
-    open func setNewPassword(id: Int32, newPassword: String) async throws {
-        try await Task.sleep(for: .seconds(2))
-    }
+    func setNewPassword(id: Int32, newPassword: String) async throws
 
-    open func deleteUser(id: Int32, reassigningPostsTo userId: Int32) async throws {
-        try await Task.sleep(for: .seconds(2))
-    }
+    func deleteUser(id: Int32, reassigningPostsTo newUserId: Int32) async throws
 }
 
-package struct MockUserProvider: UserDataProvider {
+package actor MockUserProvider: UserServiceProtocol {
 
     enum Scenario {
         case infinitLoading
@@ -36,29 +24,48 @@ package struct MockUserProvider: UserDataProvider {
 
     var scenario: Scenario
 
+    package nonisolated let usersUpdates: AsyncStream<[DisplayUser]>
+    private let usersUpdatesContinuation: AsyncStream<[DisplayUser]>.Continuation
+
+    package private(set) var users: [DisplayUser]? {
+        didSet {
+            if let users {
+                usersUpdatesContinuation.yield(users)
+            }
+        }
+    }
+
     init(scenario: Scenario = .dummyData) {
         self.scenario = scenario
+        (usersUpdates, usersUpdatesContinuation) = AsyncStream<[DisplayUser]>.makeStream()
     }
 
-    package func fetchUsers(cachedResults: CachedUserListCallback? = nil) async throws -> [WordPressUI.DisplayUser] {
+    package func fetchUsers() async throws -> [DisplayUser] {
         switch scenario {
         case .infinitLoading:
-            try await Task.sleep(for: .seconds(1 * 24 * 60 * 60))
+            // Do nothing
+            try await Task.sleep(for: .seconds(24 * 60 * 60))
             return []
         case .dummyData:
             let dummyDataUrl = URL(string: "https://my.api.mockaroo.com/users.json?key=067c9730")!
             let response = try await URLSession.shared.data(from: dummyDataUrl)
-            return try JSONDecoder().decode([DisplayUser].self, from: response.0)
+            let users = try JSONDecoder().decode([DisplayUser].self, from: response.0)
+            self.users = users
+            return users
         case .error:
             throw URLError(.timedOut)
         }
     }
 
-    package func fetchCurrentUserCan(_ capability: String) async throws -> Bool {
+    package func isCurrentUserCapableOf(_ capability: String) async throws -> Bool {
         true
     }
 
-    package func invalidateCaches() async throws {
-        // Do nothing
+    package func setNewPassword(id: Int32, newPassword: String) async throws {
+        // Not used in Preview
+    }
+
+    package func deleteUser(id: Int32, reassigningPostsTo newUserId: Int32) async throws {
+        // Not used in Preview
     }
 }

Modules/Sources/WordPressUI/Views/Users/ViewModel/UserDeleteViewModel.swift

@@ -13,92 +13,57 @@ public class UserDeleteViewModel: ObservableObject {
     private(set) var error: Error? = nil
 
     @Published
-    var otherUserId: Int32 = 0
+    var selectedUser: DisplayUser? = nil
 
     @Published
     private(set) var otherUsers: [DisplayUser] = []
 
     @Published
     private(set) var deleteButtonIsDisabled: Bool = true
 
-    private let userProvider: UserDataProvider
-    private let actionDispatcher: UserManagementActionDispatcher
+    private let userService: UserServiceProtocol
     let user: DisplayUser
 
-    init(user: DisplayUser, userProvider: UserDataProvider, actionDispatcher: UserManagementActionDispatcher) {
+    init(user: DisplayUser, userService: UserServiceProtocol) {
         self.user = user
-        self.userProvider = userProvider
-        self.actionDispatcher = actionDispatcher
-    }
+        self.userService = userService
 
-    func fetchOtherUsers() async {
-        withAnimation {
-            isFetchingOtherUsers = true
-            deleteButtonIsDisabled = true
-        }
+        // Default `selectedUser` to be the first one in `otherUsers`.
+        // Using Combine here because `didSet` observers don't work with `@Published` properties.
+        //
+        // The implementation is equivalent to `if selectedUser == nil { selectedUser = otherUsers.first }`
+        $otherUsers.combineLatest($selectedUser)
+            .filter { _, selectedUser in selectedUser == nil }
+            .map { others, _ in others.first }
+            .assign(to: &$selectedUser)
 
-        do {
-            let otherUsers = try await userProvider.fetchUsers { self.didReceiveUsers($0) }
+    }
 
-            self.didReceiveUsers(otherUsers)
-        } catch {
-            withAnimation {
-                self.error = error
-                deleteButtonIsDisabled = true
-            }
-        }
+    func fetchOtherUsers() async {
+        isFetchingOtherUsers = true
+        deleteButtonIsDisabled = true
 
-        withAnimation {
+        defer {
             isFetchingOtherUsers = false
+            deleteButtonIsDisabled = otherUsers.isEmpty
         }
-    }
-
-    func didReceiveUsers(_ users: [DisplayUser]) {
-        withAnimation {
-            if otherUserId == 0 {
-                otherUserId = otherUsers.first?.id ?? 0
-            }
 
-            otherUsers = users
+        do {
+            let users = try await userService.fetchUsers()
+            self.otherUsers = users
                 .filter { $0.id != self.user.id } // Don't allow re-assigning to yourself
                 .sorted(using: KeyPathComparator(\.username))
-            error = nil
-            deleteButtonIsDisabled = false
-            isFetchingOtherUsers = false
+        } catch {
+            self.error = error
         }
     }
 
-    func didTapDeleteUser(callback: @escaping () -> Void) {
-        debugPrint("Deleting \(user.username) and re-assigning their content to \(otherUserId)")
+    func deleteUser() async throws {
+        guard let otherUserId = selectedUser?.id, otherUserId != user.id else { return }
 
-        withAnimation {
-            error = nil
-        }
+        isDeletingUser = true
+        defer { isDeletingUser = false }
 
-        Task {
-            await MainActor.run {
-                withAnimation {
-                    isDeletingUser = true
-                }
-            }
-
-            do {
-                try await actionDispatcher.deleteUser(id: user.id, reassigningPostsTo: otherUserId)
-            } catch {
-                debugPrint(error.localizedDescription)
-                await MainActor.run {
-                    withAnimation {
-                        self.error = error
-                    }
-                }
-            }
-
-            await MainActor.run {
-                withAnimation {
-                    isDeletingUser = false
-                    callback()
-                }
-            }
-        }
+        try await userService.deleteUser(id: user.id, reassigningPostsTo: otherUserId)
     }
 }

Modules/Sources/WordPressUI/Views/Users/ViewModel/UserDetailViewModel.swift

@@ -2,7 +2,7 @@ import SwiftUI
 
 @MainActor
 class UserDetailViewModel: ObservableObject {
-    private let userProvider: UserDataProvider
+    private let userService: UserServiceProtocol
 
     @Published
     private(set) var currentUserCanModifyUsers: Bool = false
@@ -13,30 +13,20 @@ class UserDetailViewModel: ObservableObject {
     @Published
     private(set) var error: Error? = nil
 
-    init(userProvider: UserDataProvider) {
-        self.userProvider = userProvider
+    init(userService: UserServiceProtocol) {
+        self.userService = userService
     }
 
     func loadCurrentUserRole() async {
-        withAnimation {
-            isLoadingCurrentUser = true
-        }
+        error = nil
 
-        do {
-            let hasPermissions = try await userProvider.fetchCurrentUserCan("edit_users")
-            error = nil
+        isLoadingCurrentUser = true
+        defer { isLoadingCurrentUser = false}
 
-            withAnimation {
-                currentUserCanModifyUsers = hasPermissions
-            }
+        do {
+            currentUserCanModifyUsers = try await userService.isCurrentUserCapableOf("edit_users")
         } catch {
-            withAnimation {
-                self.error = error
-            }
-        }
-
-        withAnimation {
-            isLoadingCurrentUser = false
+            self.error = error
         }
     }
 }

Modules/Sources/WordPressUI/Views/Users/ViewModel/UserListViewModel.swift

@@ -1,4 +1,5 @@
 import SwiftUI
+import Combine
 import WordPressShared
 
 @MainActor
@@ -11,9 +12,13 @@ class UserListViewModel: ObservableObject {
     }
 
     /// The initial set of users fetched by `fetchItems`
-    private var users: [DisplayUser] = []
-    private let userProvider: UserDataProvider
-
+    private var users: [DisplayUser] = [] {
+        didSet {
+            sortedUsers = self.sortUsers(users)
+        }
+    }
+    private var updateUsersTask: Task<Void, Never>?
+    private let userService: UserServiceProtocol
     private var initialLoad = false
 
     @Published
@@ -37,43 +42,41 @@ class UserListViewModel: ObservableObject {
         }
     }
 
-    init(userProvider: UserDataProvider) {
-        self.userProvider = userProvider
+    init(userService: UserServiceProtocol) {
+        self.userService = userService
+    }
+
+    deinit {
+        updateUsersTask?.cancel()
     }
 
     func onAppear() async {
+        if updateUsersTask == nil {
+            updateUsersTask = Task { @MainActor [weak self, usersUpdates = userService.usersUpdates] in
+                for await users in usersUpdates {
+                    guard let self else { break }
+
+                    self.users = users
+                }
+            }
+        }
+
         if !initialLoad {
             initialLoad = true
             await fetchItems()
         }
     }
 
-    func fetchItems() async {
-        withAnimation {
-            isLoadingItems = true
-        }
+    private func fetchItems() async {
+        isLoadingItems = true
+        defer { isLoadingItems = false }
 
-        do {
-            let users = try await userProvider.fetchUsers { cachedResults in
-                self.setUsers(cachedResults)
-            }
-            setUsers(users)
-        } catch {
-            self.error = error
-            isLoadingItems = false
-        }
+        _ = try? await userService.fetchUsers()
     }
 
     @Sendable
     func refreshItems() async {
-        do {
-            let users = try await userProvider.fetchUsers { cachedResults in
-                self.setUsers(cachedResults)
-            }
-            setUsers(users)
-        } catch {
-            // Do nothing for now – this should probably show a "Toast" notification or something
-        }
+        _ = try? await userService.fetchUsers()
     }
 
     func setUsers(_ newValue: [DisplayUser]) {