Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Move Application Passwords entry to user details #23834

Merged
merged 6 commits into from
Nov 28, 2024
Merged

Move Application Passwords entry to user details #23834

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ea57e2b
Move Application Passwords entry to user details
crazytonyli Nov 20, 2024
6bebb44
Call `fetchOtherUsers` when taps "Delete User"
crazytonyli Nov 20, 2024
27855d2
Fix unit test compiling issues
crazytonyli Nov 20, 2024
9d67d18
Change the Application Password list view background
crazytonyli Nov 20, 2024
dbb1690
Remove an unnecessary IUO
crazytonyli Nov 21, 2024
10c98ee
Remove a reduandant `Group`
crazytonyli Nov 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 21 additions & 16 deletions WordPress/Classes/ApplicationToken/ApplicationTokenListView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,34 +5,39 @@ import WordPressAPI

struct ApplicationTokenListView: View {

@ObservedObject
@StateObject
private var viewModel: ApplicationTokenListViewModel

fileprivate init(tokens: [ApplicationTokenItem]) {
let dataProvider = StaticTokenProvider(tokens: .success(tokens))
self.init(viewModel: ApplicationTokenListViewModel(dataProvider: dataProvider))
self.init(dataProvider: dataProvider)
}

fileprivate init(error: Error) {
let dataProvider = StaticTokenProvider(tokens: .failure(error))
self.init(viewModel: ApplicationTokenListViewModel(dataProvider: dataProvider))
self.init(dataProvider: dataProvider)
}

init(viewModel: ApplicationTokenListViewModel) {
self.viewModel = viewModel
init(dataProvider: ApplicationTokenListDataProvider) {
_viewModel = .init(wrappedValue: ApplicationTokenListViewModel(dataProvider: dataProvider))
}

var body: some View {
VStack {
if viewModel.isLoadingData {
ProgressView()
} else if let error = viewModel.errorMessage {
EmptyStateView(Self.errorTitle, systemImage: "exclamationmark.triangle", description: error)
} else {
List(viewModel.applicationTokens) { token in
ApplicationTokenListItemView(item: token)
ZStack {
Color(.systemGroupedBackground)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(nit) this should not be needed as long as you are displaying the List at the bottom. The ProgressView could be shown as an .overlay.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't get it to work. Do you mind showing me an example?

.ignoresSafeArea()

VStack {
if viewModel.isLoadingData {
ProgressView()
} else if let error = viewModel.errorMessage {
EmptyStateView(Self.errorTitle, systemImage: "exclamationmark.triangle", description: error)
} else {
List(viewModel.applicationTokens) { token in
ApplicationTokenListItemView(item: token)
}
.listStyle(.insetGrouped)
}
.listStyle(.plain)
}
}
.navigationTitle(Self.title)
Expand All @@ -47,6 +52,7 @@ struct ApplicationTokenListView: View {
}
}

@MainActor
class ApplicationTokenListViewModel: ObservableObject {

@Published
Expand All @@ -58,14 +64,13 @@ class ApplicationTokenListViewModel: ObservableObject {
@Published
private(set) var applicationTokens: [ApplicationTokenItem]

private let dataProvider: ApplicationTokenListDataProvider!
let dataProvider: ApplicationTokenListDataProvider

init(dataProvider: ApplicationTokenListDataProvider) {
self.dataProvider = dataProvider
self.applicationTokens = []
}

@MainActor
func fetchTokens() async {
isLoadingData = true
defer {
Expand Down
4 changes: 0 additions & 4 deletions WordPress/Classes/Services/UserService.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,6 @@ actor UserService: UserServiceProtocol {
await currentUser?.capabilities.keys.contains(capability) == true
}

func isCurrentUser(_ user: DisplayUser) async -> Bool {
await currentUser?.id == user.id
}

func deleteUser(id: Int32, reassigningPostsTo newUserId: Int32) async throws {
let result = try await client.api.users.delete(
userId: id,
Expand Down
5 changes: 3 additions & 2 deletions WordPress/Classes/Users/Components/UserListItem.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,13 @@ struct UserListItem: View {
var dynamicTypeSize

let user: DisplayUser
let isCurrentUser: Bool
let userService: UserServiceProtocol
let applicationTokenListDataProvider: ApplicationTokenListDataProvider

var body: some View {
NavigationLink {
UserDetailsView(user: user, userService: userService, applicationTokenListDataProvider: applicationTokenListDataProvider)
UserDetailsView(user: user, isCurrentUser: isCurrentUser, userService: userService, applicationTokenListDataProvider: applicationTokenListDataProvider)
} label: {
HStack(alignment: .top) {
if !dynamicTypeSize.isAccessibilitySize {
Expand All @@ -30,5 +31,5 @@ struct UserListItem: View {
}

#Preview {
UserListItem(user: DisplayUser.MockUser, userService: MockUserProvider(), applicationTokenListDataProvider: StaticTokenProvider(tokens: .success(.testTokens)))
UserListItem(user: DisplayUser.MockUser, isCurrentUser: true, userService: MockUserProvider(), applicationTokenListDataProvider: StaticTokenProvider(tokens: .success(.testTokens)))
}
6 changes: 0 additions & 6 deletions WordPress/Classes/Users/UserProvider.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ public protocol UserServiceProtocol: Actor {

func fetchUsers() async throws -> [DisplayUser]

func isCurrentUser(_ user: DisplayUser) async -> Bool

func isCurrentUserCapableOf(_ capability: String) async -> Bool

func setNewPassword(id: Int32, newPassword: String) async throws
Expand Down Expand Up @@ -59,10 +57,6 @@ actor MockUserProvider: UserServiceProtocol {
}
}

func isCurrentUser(_ user: DisplayUser) async -> Bool {
true
}

func isCurrentUserCapableOf(_ capability: String) async -> Bool {
true
}
Expand Down
62 changes: 32 additions & 30 deletions WordPress/Classes/Users/Views/UserDetailsView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ struct UserDetailsView: View {

fileprivate let userService: UserServiceProtocol
let user: DisplayUser
let isCurrentUser: Bool
let applicationTokenListDataProvider: ApplicationTokenListDataProvider

@State private var presentPasswordAlert: Bool = false {
didSet {
Expand All @@ -20,19 +22,19 @@ struct UserDetailsView: View {

@StateObject
fileprivate var viewModel: UserDetailViewModel
@StateObject
fileprivate var applicationTokenListViewModel: ApplicationTokenListViewModel

@StateObject
fileprivate var deleteUserViewModel: UserDeleteViewModel

@Environment(\.dismiss)
var dismissAction: DismissAction

init(user: DisplayUser, userService: UserServiceProtocol, applicationTokenListDataProvider: ApplicationTokenListDataProvider) {
init(user: DisplayUser, isCurrentUser: Bool, userService: UserServiceProtocol, applicationTokenListDataProvider: ApplicationTokenListDataProvider) {
self.user = user
self.isCurrentUser = isCurrentUser
self.userService = userService
self.applicationTokenListDataProvider = applicationTokenListDataProvider
_viewModel = StateObject(wrappedValue: UserDetailViewModel(userService: userService))
_applicationTokenListViewModel = StateObject(wrappedValue: ApplicationTokenListViewModel(dataProvider: applicationTokenListDataProvider))
_deleteUserViewModel = StateObject(wrappedValue: UserDeleteViewModel(user: user, userService: userService))
}

Expand Down Expand Up @@ -61,29 +63,34 @@ struct UserDetailsView: View {
}
}

if !applicationTokenListViewModel.applicationTokens.isEmpty {
Section(ApplicationTokenListView.title) {
ForEach(applicationTokenListViewModel.applicationTokens) { token in
ApplicationTokenListItemView(item: token)
}
}
}

if viewModel.currentUserCanModifyUsers {
if isCurrentUser || viewModel.currentUserCanModifyUsers {
Section(Strings.accountManagementSectionTitle) {
Button(Strings.setNewPasswordActionTitle) {
presentPasswordAlert = true
if isCurrentUser {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is why I couldn't see Application Passwords for other users?

Can we make that possible?

Doesn't have to be in this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh sorry, I didn't realize your question was about the current implementation in the app.

That's in my TODO list. It's not available now because the "Application Passwords" entry was implemented as for current user only.

NavigationLink(ApplicationTokenListView.title) {
ApplicationTokenListView(dataProvider: applicationTokenListDataProvider)
}
}
Button(role: .destructive) {
presentUserPicker = true
} label: {
Text(
deleteUserViewModel.isDeletingUser ?
Strings.deletingUserActionTitle
: Strings.deleteUserActionTitle
)

if viewModel.currentUserCanModifyUsers {
Button(Strings.setNewPasswordActionTitle) {
presentPasswordAlert = true
}
Button(role: .destructive) {
presentUserPicker = true
Task {
if deleteUserViewModel.otherUsers.isEmpty {
await deleteUserViewModel.fetchOtherUsers()
}
}
} label: {
Text(
deleteUserViewModel.isDeletingUser ?
Strings.deletingUserActionTitle
: Strings.deleteUserActionTitle
)
}
.disabled(deleteUserViewModel.isDeletingUser)
}
.disabled(deleteUserViewModel.isDeletingUser)
}
}
}
Expand Down Expand Up @@ -114,11 +121,6 @@ struct UserDetailsView: View {
.onAppear() {
Task {
await viewModel.loadCurrentUserRole()
await deleteUserViewModel.fetchOtherUsers()

if await userService.isCurrentUser(user) {
await applicationTokenListViewModel.fetchTokens()
}
}
}
}
Expand Down Expand Up @@ -397,6 +399,6 @@ private extension String {

#Preview {
NavigationStack {
UserDetailsView(user: DisplayUser.MockUser, userService: MockUserProvider(), applicationTokenListDataProvider: StaticTokenProvider(tokens: .success(.testTokens)))
UserDetailsView(user: DisplayUser.MockUser, isCurrentUser: true, userService: MockUserProvider(), applicationTokenListDataProvider: StaticTokenProvider(tokens: .success(.testTokens)))
}
}
12 changes: 7 additions & 5 deletions WordPress/Classes/Users/Views/UserListView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@ public struct UserListView: View {

@StateObject
private var viewModel: UserListViewModel
private let currentUserId: Int32
private let userService: UserServiceProtocol
private let applicationTokenListDataProvider: ApplicationTokenListDataProvider

public init(userService: UserServiceProtocol, applicationTokenListDataProvider: ApplicationTokenListDataProvider) {
public init(currentUserId: Int32, userService: UserServiceProtocol, applicationTokenListDataProvider: ApplicationTokenListDataProvider) {
self.currentUserId = currentUserId
self.userService = userService
self.applicationTokenListDataProvider = applicationTokenListDataProvider
_viewModel = StateObject(wrappedValue: UserListViewModel(userService: userService))
Expand All @@ -34,7 +36,7 @@ public struct UserListView: View {
.listRowBackground(Color.clear)
} else {
ForEach(section.users) { user in
UserListItem(user: user, userService: userService, applicationTokenListDataProvider: applicationTokenListDataProvider)
UserListItem(user: user, isCurrentUser: user.id == currentUserId, userService: userService, applicationTokenListDataProvider: applicationTokenListDataProvider)
}
}
}
Expand Down Expand Up @@ -73,18 +75,18 @@ public struct UserListView: View {

#Preview("Loading") {
NavigationView {
UserListView(userService: MockUserProvider(), applicationTokenListDataProvider: StaticTokenProvider(tokens: .success(.testTokens)))
UserListView(currentUserId: 0, userService: MockUserProvider(), applicationTokenListDataProvider: StaticTokenProvider(tokens: .success(.testTokens)))
}
}

#Preview("Error") {
NavigationView {
UserListView(userService: MockUserProvider(scenario: .error), applicationTokenListDataProvider: StaticTokenProvider(tokens: .success(.testTokens)))
UserListView(currentUserId: 0, userService: MockUserProvider(scenario: .error), applicationTokenListDataProvider: StaticTokenProvider(tokens: .success(.testTokens)))
}
}

#Preview("List") {
NavigationView {
UserListView(userService: MockUserProvider(scenario: .dummyData), applicationTokenListDataProvider: StaticTokenProvider(tokens: .success(.testTokens)))
UserListView(currentUserId: 0, userService: MockUserProvider(scenario: .dummyData), applicationTokenListDataProvider: StaticTokenProvider(tokens: .success(.testTokens)))
}
}
2 changes: 1 addition & 1 deletion ...ress/Classes/ViewRelated/Blog/Blog Details/BlogDetailsViewController+SectionHelpers.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ extension BlogDetailsViewController {
let rootView = ApplicationPasswordRequiredView(blog: self.blog, localizedFeatureName: feature) { client in
let service = UserService(client: client)
let applicationPasswordService = ApplicationPasswordService(api: client, currentUserId: userId)
return UserListView(userService: service, applicationTokenListDataProvider: applicationPasswordService)
return UserListView(currentUserId: Int32(userId), userService: service, applicationTokenListDataProvider: applicationPasswordService)
}
presentationDelegate.presentBlogDetailsViewController(UIHostingController(rootView: rootView))
}
Expand Down
1 change: 1 addition & 0 deletions WordPress/WordPressTest/ApplicationPasswords/ApplicationPasswordsViewModelTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import XCTest

@testable import WordPress

@MainActor
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actors support in XCTest has been a bit shaky. I'd suggest writing all new tests in swift-testing. It's pretty similar to XCTest https://developer.apple.com/documentation/testing/migratingfromxctest

class ApplicationPasswordsViewModelTests: XCTestCase {

func testOrder() async throws {
Expand Down