Compare commits

...
Author SHA1 Message Date
Deluan 02ef4037df fix: only send library association on create when libraryIds is defined
Guard the createUser library association call with
libraryIds !== undefined, matching the updateUser pattern. This prevents
accidentally clearing backend default library assignments when the form
field hasn't been initialized yet.
2026-03-29 12:11:04 -04:00
Deluan ef3dddd164 refactor: remove dead user validation code and simplify mock
Removed the now-empty userValidation.js and its test file since the
validation function was reduced to a no-op. Cleaned up the callers in
UserCreate and UserEdit that referenced it. Simplified the mock
GetUserLibraries fallback to delegate to GetAll instead of duplicating
its logic, and removed redundant comments.
2026-03-29 11:48:05 -04:00
Deluan 08b269071f 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.
2026-03-29 11:09:41 -04:00
9 changed files with 52 additions and 143 deletions

No files matched your search

+2 -9
View File
@@ -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
+4 -3
View File
@@ -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() {
+19 -4
View File
@@ -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() {
+24 -16
View File
@@ -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
}
+2 -6
View File
@@ -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)
}
+1 -9
View File
@@ -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"
-7
View File
@@ -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
-19
View File
@@ -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
}
-70
View File
@@ -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',
)
})
})
})