From a3dab2e165a039faa1af09794879d3dc3815227c Mon Sep 17 00:00:00 2001 From: Joe Kribs Date: Thu, 24 Apr 2025 10:06:18 -0600 Subject: [PATCH] [iOS & tvOS] Cleanup Permission Validation (#1499) * Move permissions to centralized spot * Move `identifiableTypes` to `BaseItemKind`. Use `showEditMenu` * Cleanup showMenu options for iOS and tvOS. Metadata allows Subtitle, Lyrics, and Collection edits as well. * Comment out Lyrics and Subtitles with a TODO for when they are available. * Update BaseItemKind.swift Co-authored-by: Ethan Pippin * Review Revisions --------- Co-authored-by: Ethan Pippin --- .../Extensions/JellyfinAPI/BaseItemKind.swift | 7 ++ Shared/Objects/UserPermissions.swift | 68 ++++++++++++- Shared/Strings/Strings.swift | 4 +- .../ActionButtonHStack.swift | 28 +++--- .../Components/Sections/ItemSection.swift | 12 +-- .../Views/ItemEditorView/ItemEditorView.swift | 89 ++++++++++++++---- Swiftfin/Views/ItemView/ItemView.swift | 37 +++----- .../Components/Sections/ItemSection.swift | 35 +++---- Translations/en.lproj/Localizable.strings | Bin 97886 -> 97886 bytes 9 files changed, 194 insertions(+), 86 deletions(-) diff --git a/Shared/Extensions/JellyfinAPI/BaseItemKind.swift b/Shared/Extensions/JellyfinAPI/BaseItemKind.swift index 230050cf..9509063b 100644 --- a/Shared/Extensions/JellyfinAPI/BaseItemKind.swift +++ b/Shared/Extensions/JellyfinAPI/BaseItemKind.swift @@ -29,3 +29,10 @@ extension BaseItemKind: ItemFilter { rawValue } } + +extension BaseItemKind { + + static var itemIdentifiableCases: [BaseItemKind] { + [.boxSet, .movie, .person, .series] + } +} diff --git a/Shared/Objects/UserPermissions.swift b/Shared/Objects/UserPermissions.swift index d97cfc88..4fb439de 100644 --- a/Shared/Objects/UserPermissions.swift +++ b/Shared/Objects/UserPermissions.swift @@ -19,12 +19,17 @@ struct UserPermissions { } struct UserItemPermissions { - + /// This user has server permissions to delete items let canDelete: Bool + /// This user has server permissions to download items let canDownload: Bool + /// This user has server permissions to edit items' metadata let canEditMetadata: Bool + /// This user has server permissions to edit items' subtitles let canManageSubtitles: Bool + /// This user has server permissions to edit collection let canManageCollections: Bool + /// This user has server permissions to edit items' lyrics let canManageLyrics: Bool init(_ policy: UserPolicy?, isAdministrator: Bool) { @@ -35,5 +40,66 @@ struct UserPermissions { self.canManageCollections = isAdministrator || policy?.enableCollectionManagement ?? false self.canManageLyrics = isAdministrator || policy?.enableSubtitleManagement ?? false } + + // MARK: - Item Specific Validation + + /// Does this user have permission to delete this item? + func canDelete(item: BaseItemDto) -> Bool { + switch item.type { + case .playlist: + /// Playlists can only be edited by owners who can also delete + return item.canDelete == true + case .boxSet: + return canManageCollections + && StoredValues[.User.enableCollectionManagement] + && item.canDelete == true + default: + return canDelete + && StoredValues[.User.enableItemDeletion] + && item.canDelete == true + } + } + + /// Does this user have permission to download this item? + func canDownload(item: BaseItemDto) -> Bool { + canDownload && item.canDownload == true + } + + /// Does this user have permission to edit this item's metadata? + func canEditMetadata(item: BaseItemDto) -> Bool { + switch item.type { + case .playlist: + /// Playlists can only be edited by owners who can also delete + return item.canDelete == true + case .boxSet: + return (canManageCollections || canEditMetadata) + && StoredValues[.User.enableCollectionManagement] + default: + return canEditMetadata + && StoredValues[.User.enableItemEditing] + } + } + + /// Does this user have permission to edit this item's subtitles? + func canManageSubtitles(item: BaseItemDto) -> Bool { + switch item.type { + case .episode, .movie, .musicVideo, .trailer, .video: + return (canManageSubtitles || canEditMetadata) + && StoredValues[.User.enableItemEditing] + default: + return false + } + } + + /// Does this user have permission to edit this item's lyrics? + func canManageLyrics(item: BaseItemDto) -> Bool { + switch item.type { + case .audio: + return (canManageLyrics || canEditMetadata) + && StoredValues[.User.enableItemEditing] + default: + return false + } + } } } diff --git a/Shared/Strings/Strings.swift b/Shared/Strings/Strings.swift index cc5dcc43..452ebc8c 100644 --- a/Shared/Strings/Strings.swift +++ b/Shared/Strings/Strings.swift @@ -540,8 +540,8 @@ internal enum L10n { internal static let dvd = L10n.tr("Localizable", "dvd", fallback: "DVD") /// Edit internal static let edit = L10n.tr("Localizable", "edit", fallback: "Edit") - /// Edit Collections - internal static let editCollections = L10n.tr("Localizable", "editCollections", fallback: "Edit Collections") + /// Edit collections + internal static let editCollections = L10n.tr("Localizable", "editCollections", fallback: "Edit collections") /// Edit media internal static let editMedia = L10n.tr("Localizable", "editMedia", fallback: "Edit media") /// Editor diff --git a/Swiftfin tvOS/Views/ItemView/Components/ActionButtonHStack/ActionButtonHStack.swift b/Swiftfin tvOS/Views/ItemView/Components/ActionButtonHStack/ActionButtonHStack.swift index 9ec50ddd..03604777 100644 --- a/Swiftfin tvOS/Views/ItemView/Components/ActionButtonHStack/ActionButtonHStack.swift +++ b/Swiftfin tvOS/Views/ItemView/Components/ActionButtonHStack/ActionButtonHStack.swift @@ -47,21 +47,19 @@ extension ItemView { // MARK: - Can Delete Item private var canDelete: Bool { - if viewModel.item.type == .boxSet { - return enableCollectionManagement && viewModel.item.canDelete ?? false - } else { - return enableItemDeletion && viewModel.item.canDelete ?? false - } + viewModel.userSession.user.permissions.items.canDelete(item: viewModel.item) } - // MARK: - Refresh Item + // MARK: - Can Refresh Item private var canRefresh: Bool { - if viewModel.item.type == .boxSet { - return enableCollectionManagement - } else { - return enableItemEditing - } + viewModel.userSession.user.permissions.items.canEditMetadata(item: viewModel.item) + } + + // MARK: - Deletion or Refreshing is Enabled + + private var enableMenu: Bool { + canDelete || canRefresh } // MARK: - Has Trailers @@ -131,15 +129,17 @@ extension ItemView { // MARK: Advanced Options - if canRefresh || canDelete { + if enableMenu { ActionButton(L10n.advanced, icon: "ellipsis", isCompact: true) { if canRefresh { RefreshMetadataButton(item: viewModel.item) } if canDelete { - Button(L10n.delete, systemImage: "trash", role: .destructive) { - showConfirmationDialog = true + Section { + Button(L10n.delete, systemImage: "trash", role: .destructive) { + showConfirmationDialog = true + } } } } diff --git a/Swiftfin tvOS/Views/SettingsView/CustomizeViewsSettings/Components/Sections/ItemSection.swift b/Swiftfin tvOS/Views/SettingsView/CustomizeViewsSettings/Components/Sections/ItemSection.swift index 1498c4c5..7330efd8 100644 --- a/Swiftfin tvOS/Views/SettingsView/CustomizeViewsSettings/Components/Sections/ItemSection.swift +++ b/Swiftfin tvOS/Views/SettingsView/CustomizeViewsSettings/Components/Sections/ItemSection.swift @@ -41,18 +41,18 @@ extension CustomizeViewsSettings { ListRowMenu(L10n.enabledTrailers, selection: $enabledTrailers) + /// Enable Refreshing & Deleting Collections + if userSession?.user.permissions.items.canManageCollections == true { + Toggle(L10n.editCollections, isOn: $enableCollectionManagement) + } /// Enable Refreshing Items from All Visible LIbraries - if userSession?.user.permissions.items.canEditMetadata ?? false { + if userSession?.user.permissions.items.canEditMetadata == true { Toggle(L10n.editMedia, isOn: $enableItemEditing) } /// Enable Deleting Items from Approved Libraries - if userSession?.user.permissions.items.canDelete ?? false { + if userSession?.user.permissions.items.canDelete == true { Toggle(L10n.deleteMedia, isOn: $enableItemDeletion) } - /// Enable Refreshing & Deleting Collections - if userSession?.user.permissions.items.canManageCollections ?? false { - Toggle(L10n.editCollections, isOn: $enableCollectionManagement) - } } } } diff --git a/Swiftfin/Views/ItemEditorView/ItemEditorView.swift b/Swiftfin/Views/ItemEditorView/ItemEditorView.swift index 27403120..a5812842 100644 --- a/Swiftfin/Views/ItemEditorView/ItemEditorView.swift +++ b/Swiftfin/Views/ItemEditorView/ItemEditorView.swift @@ -12,15 +12,30 @@ import SwiftUI struct ItemEditorView: View { - @Injected(\.currentUserSession) - private var userSession - @EnvironmentObject private var router: ItemEditorCoordinator.Router @ObservedObject var viewModel: ItemViewModel + // MARK: - Can Edit Metadata + + private var canEditMetadata: Bool { + viewModel.userSession.user.permissions.items.canEditMetadata(item: viewModel.item) == true + } + + // MARK: - Can Manage Subtitles + + private var canManageSubtitles: Bool { + viewModel.userSession.user.permissions.items.canManageSubtitles(item: viewModel.item) == true + } + + // MARK: - Can Manage Lyrics + + private var canManageLyrics: Bool { + viewModel.userSession.user.permissions.items.canManageLyrics(item: viewModel.item) == true + } + // MARK: - Body var body: some View { @@ -48,9 +63,24 @@ struct ItemEditorView: View { description: viewModel.item.path ) - refreshButtonView + /// Hide metadata options to Lyric/Subtitle only users + if canEditMetadata { - editView + refreshButtonView + + Section(L10n.edit) { + editMetadataView + editTextView + } + + editComponentsView + } /* else if canManageSubtitles || canManageLyrics { + + // TODO: Enable when Subtitle / Lyric Editing is added + Section(L10n.edit) { + editTextView + } + }*/ } } @@ -70,7 +100,6 @@ struct ItemEditorView: View { private var refreshButtonView: some View { Section { RefreshMetadataButton(item: viewModel.item) - .environment(\.isEnabled, userSession?.user.permissions.isAdministrator ?? false) } footer: { LearnMoreButton(L10n.metadata) { TextPair( @@ -93,24 +122,46 @@ struct ItemEditorView: View { } } - // MARK: - Editable Routing Buttons + // MARK: - Editable Metadata Routing Buttons @ViewBuilder - private var editView: some View { - Section(L10n.edit) { - if [.boxSet, .movie, .person, .series].contains(viewModel.item.type) { - ChevronButton(L10n.identify) { - router.route(to: \.identifyItem, viewModel.item) - } - } - ChevronButton(L10n.images) { - router.route(to: \.editImages, ItemImagesViewModel(item: viewModel.item)) - } - ChevronButton(L10n.metadata) { - router.route(to: \.editMetadata, viewModel.item) + private var editMetadataView: some View { + + if let itemKind = viewModel.item.type, + BaseItemKind.itemIdentifiableCases.contains(itemKind) + { + ChevronButton(L10n.identify) { + router.route(to: \.identifyItem, viewModel.item) } } + ChevronButton(L10n.images) { + router.route(to: \.editImages, ItemImagesViewModel(item: viewModel.item)) + } + ChevronButton(L10n.metadata) { + router.route(to: \.editMetadata, viewModel.item) + } + } + // MARK: - Editable Text Routing Buttons + + @ViewBuilder + private var editTextView: some View { + if canManageLyrics { +// ChevronButton(L10n.lyrics) { +// router.route(to: \.editImages, ItemImagesViewModel(item: viewModel.item)) +// } + } + if canManageSubtitles { +// ChevronButton(L10n.subtitles) { +// router.route(to: \.editImages, ItemImagesViewModel(item: viewModel.item)) +// } + } + } + + // MARK: - Editable Metadata Components Routing Buttons + + @ViewBuilder + private var editComponentsView: some View { Section { ChevronButton(L10n.genres) { router.route(to: \.editGenres, viewModel.item) diff --git a/Swiftfin/Views/ItemView/ItemView.swift b/Swiftfin/Views/ItemView/ItemView.swift index efb19402..5e651c83 100644 --- a/Swiftfin/Views/ItemView/ItemView.swift +++ b/Swiftfin/Views/ItemView/ItemView.swift @@ -30,33 +30,25 @@ struct ItemView: View { @State private var error: JellyfinAPIError? - @StoredValue(.User.enableItemDeletion) - private var enableItemDeletion: Bool - @StoredValue(.User.enableItemEditing) - private var enableItemEditing: Bool - @StoredValue(.User.enableCollectionManagement) - private var enableCollectionManagement: Bool + // MARK: - Can Delete Item private var canDelete: Bool { - if viewModel.item.type == .boxSet { - return enableCollectionManagement && viewModel.item.canDelete ?? false - } else { - return enableItemDeletion && viewModel.item.canDelete ?? false - } + viewModel.userSession.user.permissions.items.canDelete(item: viewModel.item) } + // MARK: - Can Edit Item + private var canEdit: Bool { - if viewModel.item.type == .boxSet { - return enableCollectionManagement - } else { - return enableItemEditing - } + viewModel.userSession.user.permissions.items.canEditMetadata(item: viewModel.item) + // TODO: Enable when Subtitle / Lyric Editing is added + // || viewModel.userSession.user.permissions.items.canManageLyrics(item: viewModel.item) + // || viewModel.userSession.user.permissions.items.canManageSubtitles(item: viewModel.item) } - // Use to hide the menu button when not needed. - // Add more checks as needed. For example, canDownload. + // MARK: - Deletion or Editing is Enabled + private var enableMenu: Bool { - canDelete || canEdit + canEdit || canDelete } private static func typeViewModel(for item: BaseItemDto) -> ItemViewModel { @@ -149,9 +141,10 @@ struct ItemView: View { } if canDelete { - Divider() - Button(L10n.delete, systemImage: "trash", role: .destructive) { - showConfirmationDialog = true + Section { + Button(L10n.delete, systemImage: "trash", role: .destructive) { + showConfirmationDialog = true + } } } } diff --git a/Swiftfin/Views/SettingsView/CustomizeViewsSettings/Components/Sections/ItemSection.swift b/Swiftfin/Views/SettingsView/CustomizeViewsSettings/Components/Sections/ItemSection.swift index 96df806f..5b081334 100644 --- a/Swiftfin/Views/SettingsView/CustomizeViewsSettings/Components/Sections/ItemSection.swift +++ b/Swiftfin/Views/SettingsView/CustomizeViewsSettings/Components/Sections/ItemSection.swift @@ -44,30 +44,21 @@ extension CustomizeViewsSettings { selection: $enabledTrailers ) - /// Enable Editing Items from All Visible LIbraries - if userSession?.user.permissions.items.canEditMetadata ?? false { - Toggle(L10n.editMedia, isOn: $enableItemEditing) - } - /// Enable Deleting Items from Approved Libraries - if userSession?.user.permissions.items.canDelete ?? false { - Toggle(L10n.deleteMedia, isOn: $enableItemDeletion) - } - /// Enable Downloading All Items - /* if userSession?.user.permissions.items.canDownload ?? false { - Toggle(L10n.itemDownloading, isOn: $enableItemDownloads) - } */ - /// Enable Deleting or Editing Collections - if userSession?.user.permissions.items.canManageCollections ?? false { + /// Enabled Collection Management for collection managers + if userSession?.user.permissions.items.canManageCollections == true { Toggle(L10n.editCollections, isOn: $enableCollectionManagement) } - /// Manage Item Lyrics - /* if userSession?.user.permissions.items.canManageLyrics ?? false { - Toggle(L10n.allowLyricsManagement isOn: $enableLyricsManagement) - } */ - /// Manage Item Subtitles - /* if userSession?.user.items.canManageSubtitles ?? false { - Toggle(L10n.allowSubtitleManagement, isOn: $enableSubtitleManagement) - } */ + /// Enabled Media Management when there are media elements that can be managed + if userSession?.user.permissions.items.canEditMetadata == true || + userSession?.user.permissions.items.canManageLyrics == true || + userSession?.user.permissions.items.canManageSubtitles == true + { + Toggle(L10n.editMedia, isOn: $enableItemEditing) + } + /// Enabled Media Deletion for valid deletion users + if userSession?.user.permissions.items.canDelete == true { + Toggle(L10n.deleteMedia, isOn: $enableItemDeletion) + } } } } diff --git a/Translations/en.lproj/Localizable.strings b/Translations/en.lproj/Localizable.strings index 598a4483d04e7819881cd8867cd70d1b93210b4d..ae61fb82d89bd9b6eecbc1ef241c0051658b7d6f 100644 GIT binary patch delta 34 pcmccjhxOhc)(zVxFeXptb&;HWVS*8m