mirror of
https://github.com/navidrome/navidrome.git
synced 2026-05-03 06:51:16 +00:00
Merge 02ef4037df81a699f3b923ed68ae759154821d7e into 94eb6c522b63198bdc4565442d86918ad43156e5
This commit is contained in:
commit
e2b27dcf31
@ -78,16 +78,9 @@ func (s *libraryService) SetUserLibraries(ctx context.Context, userID string, li
|
||||
return fmt.Errorf("%w: cannot manually assign libraries to admin users", model.ErrValidation)
|
||||
}
|
||||
|
||||
// Regular users must have at least one library
|
||||
if len(libraryIDs) == 0 {
|
||||
return fmt.Errorf("%w: at least one library must be assigned to non-admin users", model.ErrValidation)
|
||||
}
|
||||
|
||||
// Validate all library IDs exist
|
||||
if len(libraryIDs) > 0 {
|
||||
if err := s.validateLibraryIDs(ctx, libraryIDs); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := s.validateLibraryIDs(ctx, libraryIDs); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
// Set user libraries
|
||||
|
||||
@ -535,11 +535,12 @@ var _ = Describe("Library Service", func() {
|
||||
Expect(err.Error()).To(ContainSubstring("cannot manually assign libraries to admin users"))
|
||||
})
|
||||
|
||||
It("fails when no libraries provided for regular user", func() {
|
||||
It("allows setting empty libraries for regular user", func() {
|
||||
err := service.SetUserLibraries(ctx, "user1", []int{})
|
||||
|
||||
Expect(err).To(HaveOccurred())
|
||||
Expect(err.Error()).To(ContainSubstring("at least one library must be assigned to non-admin users"))
|
||||
Expect(err).NotTo(HaveOccurred())
|
||||
libraries := userRepo.UserLibraries["user1"]
|
||||
Expect(libraries).To(BeEmpty())
|
||||
})
|
||||
|
||||
It("fails when library doesn't exist", func() {
|
||||
|
||||
@ -350,9 +350,20 @@ var _ = Describe("Library API", func() {
|
||||
Expect(w.Body.String()).To(ContainSubstring("library ID 999 does not exist"))
|
||||
})
|
||||
|
||||
It("requires at least one library for regular users", func() {
|
||||
It("allows removing all libraries from regular users", func() {
|
||||
// First assign some libraries
|
||||
setupRequest := map[string][]int{
|
||||
"libraryIds": {1, 2},
|
||||
}
|
||||
setupBody, _ := json.Marshal(setupRequest)
|
||||
setupReq := createAuthenticatedRequest("PUT", fmt.Sprintf("/user/%s/library", regularUser.ID), bytes.NewBuffer(setupBody), adminToken)
|
||||
setupW := httptest.NewRecorder()
|
||||
router.ServeHTTP(setupW, setupReq)
|
||||
Expect(setupW.Code).To(Equal(http.StatusOK))
|
||||
|
||||
// Then remove all libraries
|
||||
request := map[string][]int{
|
||||
"libraryIds": {}, // Empty libraries
|
||||
"libraryIds": {},
|
||||
}
|
||||
body, _ := json.Marshal(request)
|
||||
req := createAuthenticatedRequest("PUT", fmt.Sprintf("/user/%s/library", regularUser.ID), bytes.NewBuffer(body), adminToken)
|
||||
@ -360,8 +371,12 @@ var _ = Describe("Library API", func() {
|
||||
|
||||
router.ServeHTTP(w, req)
|
||||
|
||||
Expect(w.Code).To(Equal(http.StatusBadRequest))
|
||||
Expect(w.Body.String()).To(ContainSubstring("at least one library must be assigned"))
|
||||
Expect(w.Code).To(Equal(http.StatusOK))
|
||||
|
||||
var libraries []model.Library
|
||||
err := json.Unmarshal(w.Body.Bytes(), &libraries)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(libraries).To(BeEmpty())
|
||||
})
|
||||
|
||||
It("prevents manual assignment to admin users", func() {
|
||||
|
||||
@ -14,9 +14,10 @@ import (
|
||||
|
||||
type MockLibraryRepo struct {
|
||||
model.LibraryRepository
|
||||
Data map[int]model.Library
|
||||
Err error
|
||||
PutFn func(*model.Library) error // Allow custom Put behavior for testing
|
||||
Data map[int]model.Library
|
||||
UserLibraries map[string][]int // per-user library ID assignments
|
||||
Err error
|
||||
PutFn func(*model.Library) error // Allow custom Put behavior for testing
|
||||
}
|
||||
|
||||
func (m *MockLibraryRepo) SetData(data model.Libraries) {
|
||||
@ -266,16 +267,23 @@ func (m *MockLibraryRepo) GetUserLibraries(ctx context.Context, userID string) (
|
||||
if userID == "non-existent" {
|
||||
return nil, model.ErrNotFound
|
||||
}
|
||||
// Convert map to slice for return
|
||||
var libraries model.Libraries
|
||||
for _, lib := range m.Data {
|
||||
libraries = append(libraries, lib)
|
||||
if m.UserLibraries != nil {
|
||||
ids, ok := m.UserLibraries[userID]
|
||||
if !ok {
|
||||
return nil, nil
|
||||
}
|
||||
var libraries model.Libraries
|
||||
for _, id := range ids {
|
||||
if lib, exists := m.Data[id]; exists {
|
||||
libraries = append(libraries, lib)
|
||||
}
|
||||
}
|
||||
slices.SortFunc(libraries, func(a, b model.Library) int {
|
||||
return a.ID - b.ID
|
||||
})
|
||||
return libraries, nil
|
||||
}
|
||||
// Sort by ID for predictable order
|
||||
slices.SortFunc(libraries, func(a, b model.Library) int {
|
||||
return a.ID - b.ID
|
||||
})
|
||||
return libraries, nil
|
||||
return m.GetAll()
|
||||
}
|
||||
|
||||
func (m *MockLibraryRepo) SetUserLibraries(ctx context.Context, userID string, libraryIDs []int) error {
|
||||
@ -288,15 +296,15 @@ func (m *MockLibraryRepo) SetUserLibraries(ctx context.Context, userID string, l
|
||||
if userID == "admin-1" {
|
||||
return fmt.Errorf("%w: cannot manually assign libraries to admin users", model.ErrValidation)
|
||||
}
|
||||
if len(libraryIDs) == 0 {
|
||||
return fmt.Errorf("%w: at least one library must be assigned to non-admin users", model.ErrValidation)
|
||||
}
|
||||
// Validate all library IDs exist
|
||||
for _, id := range libraryIDs {
|
||||
if _, exists := m.Data[id]; !exists {
|
||||
return fmt.Errorf("%w: library ID %d does not exist", model.ErrValidation, id)
|
||||
}
|
||||
}
|
||||
if m.UserLibraries == nil {
|
||||
m.UserLibraries = make(map[string][]int)
|
||||
}
|
||||
m.UserLibraries[userID] = libraryIDs
|
||||
return nil
|
||||
}
|
||||
|
||||
|
||||
@ -93,14 +93,10 @@ const callDeleteMany = (resource, params) => {
|
||||
|
||||
// Helper function to handle user-library associations
|
||||
const handleUserLibraryAssociation = async (userId, libraryIds) => {
|
||||
if (!libraryIds || libraryIds.length === 0) {
|
||||
return // Admin users or users without library assignments
|
||||
}
|
||||
|
||||
try {
|
||||
await httpClient(`${REST_URL}/user/${userId}/library`, {
|
||||
method: 'PUT',
|
||||
body: JSON.stringify({ libraryIds }),
|
||||
body: JSON.stringify({ libraryIds: libraryIds || [] }),
|
||||
})
|
||||
} catch (error) {
|
||||
console.error('Error setting user libraries:', error) //eslint-disable-line no-console
|
||||
@ -118,7 +114,7 @@ const createUser = async (params) => {
|
||||
const userId = userResponse.data.id
|
||||
|
||||
// Then set library associations for non-admin users
|
||||
if (!userData.isAdmin && libraryIds && libraryIds.length > 0) {
|
||||
if (!userData.isAdmin && libraryIds !== undefined) {
|
||||
await handleUserLibraryAssociation(userId, libraryIds)
|
||||
}
|
||||
|
||||
|
||||
@ -51,17 +51,9 @@ const UserCreate = (props) => {
|
||||
[mutate, notify, redirect],
|
||||
)
|
||||
|
||||
// Custom validation function
|
||||
const validateUserForm = (values) => {
|
||||
const errors = {}
|
||||
// Library selection is optional for non-admin users since they will be auto-assigned to default libraries
|
||||
// No validation required for library selection
|
||||
return errors
|
||||
}
|
||||
|
||||
return (
|
||||
<Create title={<Title subTitle={title} />} {...props}>
|
||||
<SimpleForm save={save} validate={validateUserForm} variant={'outlined'}>
|
||||
<SimpleForm save={save} variant={'outlined'}>
|
||||
<TextInput
|
||||
spellCheck={false}
|
||||
source="userName"
|
||||
|
||||
@ -24,7 +24,6 @@ import { Typography } from '@material-ui/core'
|
||||
import { Title } from '../common'
|
||||
import DeleteUserButton from './DeleteUserButton'
|
||||
import { LibrarySelectionField } from './LibrarySelectionField.jsx'
|
||||
import { validateUserForm } from './userValidation'
|
||||
|
||||
const useStyles = makeStyles({
|
||||
toolbar: {
|
||||
@ -104,18 +103,12 @@ const UserEdit = (props) => {
|
||||
[mutate, notify, permissions, redirect, refresh],
|
||||
)
|
||||
|
||||
// Custom validation function
|
||||
const validateForm = (values) => {
|
||||
return validateUserForm(values, translate)
|
||||
}
|
||||
|
||||
return (
|
||||
<Edit title={<UserTitle />} undoable={false} {...props}>
|
||||
<SimpleForm
|
||||
variant={'outlined'}
|
||||
toolbar={<UserToolbar showDelete={canDelete} />}
|
||||
save={save}
|
||||
validate={validateForm}
|
||||
>
|
||||
{permissions === 'admin' && (
|
||||
<TextInput
|
||||
|
||||
@ -1,19 +0,0 @@
|
||||
// User form validation utilities
|
||||
export const validateUserForm = (values, translate) => {
|
||||
const errors = {}
|
||||
|
||||
// Only require library selection for non-admin users
|
||||
if (!values.isAdmin) {
|
||||
// Check both libraryIds (array of IDs) and libraries (array of objects)
|
||||
const hasLibraryIds = values.libraryIds && values.libraryIds.length > 0
|
||||
const hasLibraries = values.libraries && values.libraries.length > 0
|
||||
|
||||
if (!hasLibraryIds && !hasLibraries) {
|
||||
errors.libraryIds = translate(
|
||||
'resources.user.validation.librariesRequired',
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
return errors
|
||||
}
|
||||
@ -1,70 +0,0 @@
|
||||
import { describe, it, expect, vi } from 'vitest'
|
||||
import { validateUserForm } from './userValidation'
|
||||
|
||||
describe('User Validation Utilities', () => {
|
||||
const mockTranslate = vi.fn((key) => key)
|
||||
|
||||
describe('validateUserForm', () => {
|
||||
it('should not return errors for admin users', () => {
|
||||
const values = {
|
||||
isAdmin: true,
|
||||
libraryIds: [],
|
||||
}
|
||||
const errors = validateUserForm(values, mockTranslate)
|
||||
expect(errors).toEqual({})
|
||||
})
|
||||
|
||||
it('should not return errors for non-admin users with libraries', () => {
|
||||
const values = {
|
||||
isAdmin: false,
|
||||
libraryIds: [1, 2, 3],
|
||||
}
|
||||
const errors = validateUserForm(values, mockTranslate)
|
||||
expect(errors).toEqual({})
|
||||
})
|
||||
|
||||
it('should return error for non-admin users without libraries', () => {
|
||||
const values = {
|
||||
isAdmin: false,
|
||||
libraryIds: [],
|
||||
}
|
||||
const errors = validateUserForm(values, mockTranslate)
|
||||
expect(errors.libraryIds).toBe(
|
||||
'resources.user.validation.librariesRequired',
|
||||
)
|
||||
})
|
||||
|
||||
it('should return error for non-admin users with undefined libraryIds', () => {
|
||||
const values = {
|
||||
isAdmin: false,
|
||||
}
|
||||
const errors = validateUserForm(values, mockTranslate)
|
||||
expect(errors.libraryIds).toBe(
|
||||
'resources.user.validation.librariesRequired',
|
||||
)
|
||||
})
|
||||
|
||||
it('should not return errors for non-admin users with libraries array', () => {
|
||||
const values = {
|
||||
isAdmin: false,
|
||||
libraries: [
|
||||
{ id: 1, name: 'Library 1' },
|
||||
{ id: 2, name: 'Library 2' },
|
||||
],
|
||||
}
|
||||
const errors = validateUserForm(values, mockTranslate)
|
||||
expect(errors).toEqual({})
|
||||
})
|
||||
|
||||
it('should return error for non-admin users with empty libraries array', () => {
|
||||
const values = {
|
||||
isAdmin: false,
|
||||
libraries: [],
|
||||
}
|
||||
const errors = validateUserForm(values, mockTranslate)
|
||||
expect(errors.libraryIds).toBe(
|
||||
'resources.user.validation.librariesRequired',
|
||||
)
|
||||
})
|
||||
})
|
||||
})
|
||||
Loading…
x
Reference in New Issue
Block a user