diff --git a/core/library.go b/core/library.go index 0bf3be9fa..eb865cfc7 100644 --- a/core/library.go +++ b/core/library.go @@ -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 diff --git a/core/library_test.go b/core/library_test.go index 175d9c37d..0ff1ac077 100644 --- a/core/library_test.go +++ b/core/library_test.go @@ -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() { diff --git a/server/nativeapi/library_test.go b/server/nativeapi/library_test.go index ed5564a41..41982548d 100644 --- a/server/nativeapi/library_test.go +++ b/server/nativeapi/library_test.go @@ -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() { diff --git a/tests/mock_library_repo.go b/tests/mock_library_repo.go index 3f0e576e9..5feb8fb06 100644 --- a/tests/mock_library_repo.go +++ b/tests/mock_library_repo.go @@ -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 } diff --git a/ui/src/dataProvider/wrapperDataProvider.js b/ui/src/dataProvider/wrapperDataProvider.js index 268d3668d..23bb69587 100644 --- a/ui/src/dataProvider/wrapperDataProvider.js +++ b/ui/src/dataProvider/wrapperDataProvider.js @@ -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) } diff --git a/ui/src/user/UserCreate.jsx b/ui/src/user/UserCreate.jsx index ce69b6542..b6370ad9b 100644 --- a/ui/src/user/UserCreate.jsx +++ b/ui/src/user/UserCreate.jsx @@ -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 ( } {...props}> - + { [mutate, notify, permissions, redirect, refresh], ) - // Custom validation function - const validateForm = (values) => { - return validateUserForm(values, translate) - } - return ( } undoable={false} {...props}> } save={save} - validate={validateForm} > {permissions === 'admin' && ( { - 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 -} diff --git a/ui/src/user/userValidation.test.js b/ui/src/user/userValidation.test.js deleted file mode 100644 index 2ee473910..000000000 --- a/ui/src/user/userValidation.test.js +++ /dev/null @@ -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', - ) - }) - }) -})