mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-31 07:30:32 +00:00
fix: allow users to have no libraries assigned
Previously, three layers enforced that non-admin users must have at least one library: the backend service validation, the frontend data provider (which silently skipped the API call for empty lists), and the frontend form validation. This prevented both creating users without libraries and removing all libraries from existing users. Removed the "at least one library" constraint across all layers so that library assignment is fully optional for non-admin users.
This commit is contained in:
parent
049fc78177
commit
08b269071f
@ -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,12 +267,28 @@ func (m *MockLibraryRepo) GetUserLibraries(ctx context.Context, userID string) (
|
||||
if userID == "non-existent" {
|
||||
return nil, model.ErrNotFound
|
||||
}
|
||||
// Convert map to slice for return
|
||||
// If per-user tracking is set, return only the assigned libraries
|
||||
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
|
||||
}
|
||||
// Fallback: return all libraries
|
||||
var libraries model.Libraries
|
||||
for _, lib := range m.Data {
|
||||
libraries = append(libraries, lib)
|
||||
}
|
||||
// Sort by ID for predictable order
|
||||
slices.SortFunc(libraries, func(a, b model.Library) int {
|
||||
return a.ID - b.ID
|
||||
})
|
||||
@ -288,15 +305,17 @@ 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)
|
||||
}
|
||||
}
|
||||
// Store per-user assignments
|
||||
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) {
|
||||
await handleUserLibraryAssociation(userId, libraryIds)
|
||||
}
|
||||
|
||||
|
||||
@ -1,19 +1,5 @@
|
||||
// User form validation utilities
|
||||
export const validateUserForm = (values, translate) => {
|
||||
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
|
||||
}
|
||||
|
||||
@ -23,48 +23,21 @@ describe('User Validation Utilities', () => {
|
||||
expect(errors).toEqual({})
|
||||
})
|
||||
|
||||
it('should return error for non-admin users without libraries', () => {
|
||||
it('should not return errors for non-admin users without libraries', () => {
|
||||
const values = {
|
||||
isAdmin: false,
|
||||
libraryIds: [],
|
||||
}
|
||||
const errors = validateUserForm(values, mockTranslate)
|
||||
expect(errors.libraryIds).toBe(
|
||||
'resources.user.validation.librariesRequired',
|
||||
)
|
||||
expect(errors).toEqual({})
|
||||
})
|
||||
|
||||
it('should return error for non-admin users with undefined libraryIds', () => {
|
||||
it('should not return errors 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