diff --git a/ui/src/dataProvider/wrapperDataProvider.js b/ui/src/dataProvider/wrapperDataProvider.js index 268d3668d..f5004308b 100644 --- a/ui/src/dataProvider/wrapperDataProvider.js +++ b/ui/src/dataProvider/wrapperDataProvider.js @@ -137,8 +137,9 @@ const updateUser = async (params) => { data: userData, }) - // Then handle library associations for non-admin users - if (!userData.isAdmin && libraryIds !== undefined) { + // Then handle library associations for non-admin users. Only admins can call + // this endpoint; for self-edits the server manages library assignments + if (isAdmin() && !userData.isAdmin && libraryIds !== undefined) { await handleUserLibraryAssociation(userId, libraryIds) } diff --git a/ui/src/dataProvider/wrapperDataProvider.test.js b/ui/src/dataProvider/wrapperDataProvider.test.js new file mode 100644 index 000000000..fbc82f969 --- /dev/null +++ b/ui/src/dataProvider/wrapperDataProvider.test.js @@ -0,0 +1,90 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import wrapperDataProvider from './wrapperDataProvider' + +const { mockProvider, mockHttpClient } = vi.hoisted(() => ({ + mockProvider: { + update: vi.fn(), + create: vi.fn(), + getOne: vi.fn(), + }, + mockHttpClient: vi.fn(), +})) + +vi.mock('ra-data-json-server', () => ({ default: () => mockProvider })) +vi.mock('./httpClient', () => ({ default: mockHttpClient })) + +describe('wrapperDataProvider', () => { + beforeEach(() => { + vi.clearAllMocks() + localStorage.clear() + mockProvider.update.mockResolvedValue({ data: { id: 'u1' } }) + mockProvider.create.mockResolvedValue({ data: { id: 'u1' } }) + mockHttpClient.mockResolvedValue({ json: [] }) + }) + + describe('update user', () => { + it('sets library associations when an admin edits a non-admin user', async () => { + localStorage.setItem('role', 'admin') + + await wrapperDataProvider.update('user', { + id: 'u1', + data: { name: 'Sam', isAdmin: false, libraryIds: [1] }, + }) + + expect(mockProvider.update).toHaveBeenCalledWith( + 'user', + expect.objectContaining({ id: 'u1' }), + ) + expect(mockHttpClient).toHaveBeenCalledWith('/api/user/u1/library', { + method: 'PUT', + body: JSON.stringify({ libraryIds: [1] }), + }) + }) + + it('does not call the admin-only library endpoint when a non-admin edits their own profile', async () => { + localStorage.setItem('role', 'regular') + + await wrapperDataProvider.update('user', { + id: 'u1', + data: { + name: 'Sam', + isAdmin: false, + libraryIds: [1], + currentPassword: 'old', + password: 'new', + }, + }) + + expect(mockProvider.update).toHaveBeenCalled() + expect(mockHttpClient).not.toHaveBeenCalled() + }) + + it('does not set library associations when the edited user is an admin', async () => { + localStorage.setItem('role', 'admin') + + await wrapperDataProvider.update('user', { + id: 'u1', + data: { name: 'Sam', isAdmin: true, libraryIds: [1] }, + }) + + expect(mockProvider.update).toHaveBeenCalled() + expect(mockHttpClient).not.toHaveBeenCalled() + }) + + it('strips libraryIds from the user update payload', async () => { + localStorage.setItem('role', 'admin') + + await wrapperDataProvider.update('user', { + id: 'u1', + data: { name: 'Sam', isAdmin: false, libraryIds: [1] }, + }) + + expect(mockProvider.update).toHaveBeenCalledWith( + 'user', + expect.objectContaining({ + data: { name: 'Sam', isAdmin: false }, + }), + ) + }) + }) +}) diff --git a/ui/src/user/UserEdit.jsx b/ui/src/user/UserEdit.jsx index 2283dd8bc..d8302a9f9 100644 --- a/ui/src/user/UserEdit.jsx +++ b/ui/src/user/UserEdit.jsx @@ -96,9 +96,10 @@ const UserEdit = (props) => { }) permissions === 'admin' ? redirect('/user') : refresh() } catch (error) { - if (error.body.errors) { + if (error?.body?.errors) { return error.body.errors } + notify('ra.page.error', 'warning') } }, [mutate, notify, permissions, redirect, refresh], diff --git a/ui/src/user/UserEdit.test.jsx b/ui/src/user/UserEdit.test.jsx index 75a9a1ada..1d8290569 100644 --- a/ui/src/user/UserEdit.test.jsx +++ b/ui/src/user/UserEdit.test.jsx @@ -27,6 +27,14 @@ const adminUser = { isAdmin: true, } +const hooks = vi.hoisted(() => ({ + save: null, + mutate: vi.fn(), + notify: vi.fn(), + redirect: vi.fn(), + refresh: vi.fn(), +})) + // Mock React-Admin completely with simpler implementations vi.mock('react-admin', () => ({ Edit: ({ children, title }) => ( @@ -35,9 +43,10 @@ vi.mock('react-admin', () => ({ {children} ), - SimpleForm: ({ children }) => ( -
{children}
- ), + SimpleForm: ({ children, save }) => { + hooks.save = save + return
{children}
+ }, TextInput: ({ source }) => , BooleanInput: ({ source }) => ( @@ -54,10 +63,10 @@ vi.mock('react-admin', () => ({ Typography: ({ children }) =>

{children}

, required: () => () => null, email: () => () => null, - useMutation: () => [vi.fn()], - useNotify: () => vi.fn(), - useRedirect: () => vi.fn(), - useRefresh: () => vi.fn(), + useMutation: () => [hooks.mutate], + useNotify: () => hooks.notify, + useRedirect: () => hooks.redirect, + useRefresh: () => hooks.refresh, usePermissions: () => ({ permissions: 'admin' }), useTranslate: () => (key) => key, })) @@ -127,4 +136,60 @@ describe('', () => { expect(screen.getByTestId('text-input-name')).toBeInTheDocument() expect(screen.getByTestId('text-input-email')).toBeInTheDocument() }) + + describe('save', () => { + beforeEach(() => { + vi.clearAllMocks() + hooks.save = null + }) + + it('notifies success and redirects when the update succeeds', async () => { + hooks.mutate.mockResolvedValue({ data: defaultUser }) + render() + + await hooks.save({ id: 'user1', name: 'New Name' }) + + expect(hooks.notify).toHaveBeenCalledWith( + 'resources.user.notifications.updated', + 'info', + { smart_count: 1 }, + ) + expect(hooks.redirect).toHaveBeenCalledWith('/user') + }) + + it('returns field errors when the update fails validation', async () => { + const fieldErrors = { currentPassword: 'ra.validation.required' } + hooks.mutate.mockRejectedValue({ body: { errors: fieldErrors } }) + render() + + const result = await hooks.save({ id: 'user1' }) + + expect(result).toEqual(fieldErrors) + expect(hooks.notify).not.toHaveBeenCalledWith( + 'resources.user.notifications.updated', + 'info', + { smart_count: 1 }, + ) + }) + + it('notifies an error when the update fails without field errors', async () => { + hooks.mutate.mockRejectedValue(new Error('Forbidden')) + render() + + await hooks.save({ id: 'user1' }) + + expect(hooks.notify).toHaveBeenCalledWith('ra.page.error', 'warning') + expect(hooks.redirect).not.toHaveBeenCalled() + }) + + it('notifies an error when the update rejects with a non-object error', async () => { + hooks.mutate.mockRejectedValue(undefined) + render() + + await hooks.save({ id: 'user1' }) + + expect(hooks.notify).toHaveBeenCalledWith('ra.page.error', 'warning') + expect(hooks.redirect).not.toHaveBeenCalled() + }) + }) })