diff --git a/src/api/index.js b/src/api/index.js index 638803030d..d98be04df3 100644 --- a/src/api/index.js +++ b/src/api/index.js @@ -111,8 +111,10 @@ export class Api { return this.sendAction(`/api/project/${project}`) } - getUsers({ domain = 'default', index = null, q = null, sort = null, desc = null, from = 0, size = 10, noRole = true } = {}) { - const params = omitBy({ domain, index, q, sort, desc, from, size, noRole }, isNull) + // uid is an exact match and takes precedence over q server-side: use it to resolve one known + // user, q to search. omitBy drops the nulls, so passing { uid } alone sends no q. + getUsers({ domain = 'default', index = null, uid = null, q = null, sort = null, desc = null, from = 0, size = 10, noRole = true } = {}) { + const params = omitBy({ domain, index, uid, q, sort, desc, from, size, noRole }, isNull) return this.sendAction('/api/users/admin', { method: Method.GET, params }) } diff --git a/src/views/Settings/SettingsView/SettingsViewUsers.vue b/src/views/Settings/SettingsView/SettingsViewUsers.vue index c050e31322..89b4e49372 100644 --- a/src/views/Settings/SettingsView/SettingsViewUsers.vue +++ b/src/views/Settings/SettingsView/SettingsViewUsers.vue @@ -88,9 +88,11 @@ async function fetchRoutedUser() { const uid = routedUid.value if (!uid) return try { - const { items } = await api.getUsers({ domain: DEFAULT_DOMAIN, noRole: true, q: uid, size: 100 }) + // An exact uid lookup: a q search would return every user whose uid merely contains this one, + // and on a large instance the user we want could fall outside the page. + const { items } = await api.getUsers({ domain: DEFAULT_DOMAIN, noRole: true, uid }) if (uid !== routedUid.value) return - routedUser.value = items?.find(user => user.uid === uid) ?? null + routedUser.value = items?.[0] ?? null routedUserUid.value = uid } catch (error) { diff --git a/tests/unit/specs/api/index.spec.js b/tests/unit/specs/api/index.spec.js index df242ac822..50bc3fe6de 100644 --- a/tests/unit/specs/api/index.spec.js +++ b/tests/unit/specs/api/index.spec.js @@ -515,6 +515,18 @@ describe('Datashare backend client', () => { expect(axios.request.mock.calls[0][0].params).not.toHaveProperty('user') expect(axios.request.mock.calls[0][0].params).not.toHaveProperty('project') }) + + it('sends an exact "uid" and no "q" when resolving a single user', async () => { + await api.getUsers({ uid: 'alice' }) + expect(axios.request).toBeCalledWith( + expect.objectContaining({ + url: Api.getFullUrl('/api/users/admin'), + method: 'GET', + params: expect.objectContaining({ uid: 'alice' }) + }) + ) + expect(axios.request.mock.calls[0][0].params).not.toHaveProperty('q') + }) }) it('should call deleteUser with userId (uid) and no body', async () => { diff --git a/tests/unit/specs/views/Settings/SettingsView/SettingsViewUsers.spec.js b/tests/unit/specs/views/Settings/SettingsView/SettingsViewUsers.spec.js index 308190424f..bbf8e7eb23 100644 --- a/tests/unit/specs/views/Settings/SettingsView/SettingsViewUsers.spec.js +++ b/tests/unit/specs/views/Settings/SettingsView/SettingsViewUsers.spec.js @@ -241,8 +241,13 @@ describe('SettingsViewUsers.vue', () => { describe('routed user modals', () => { const zoe = { uid: 'zoe@example.org', name: 'Zoe', email: 'zoe@example.org', permissions: [] } + // The backend resolves uid exactly, so an unknown uid comes back as an empty page beforeEach(() => { - api.getUsers.mockImplementation(async ({ q }) => (q === zoe.uid ? { items: [zoe], pagination: { total: 1 } } : usersResponse)) + api.getUsers.mockImplementation(async ({ uid }) => { + if (!uid) return usersResponse + const items = uid === zoe.uid ? [zoe] : [] + return { items, pagination: { total: items.length } } + }) }) it.each([ @@ -256,6 +261,17 @@ describe('SettingsViewUsers.vue', () => { expect(wrapper.findComponent(modal).props()).toMatchObject({ modelValue: true, user: zoe, notFound: false }) }) + // A uid search by q only matches a substring, so a short uid could fall outside the page and + // the modal would claim the user does not exist (ICIJ/datashare#2434). + it('looks the routed user up by exact uid, not with a q search over a page of hits', async () => { + await core.router.push(`/settings/users/edit/${zoe.uid}`) + shallowMountComponent() + await flushPromises() + // noRole stays on: a user holding no role in the scope must still resolve + expect(api.getUsers).toHaveBeenCalledWith(expect.objectContaining({ uid: zoe.uid, noRole: true })) + expect(api.getUsers).not.toHaveBeenCalledWith(expect.objectContaining({ q: zoe.uid })) + }) + it.each([ ['manage', SettingsViewUsersRolesModal], ['edit', SettingsViewUsersEditModal], @@ -310,8 +326,8 @@ describe('SettingsViewUsers.vue', () => { }) it('reports a failed lookup and goes back to the list, instead of saying the user does not exist', async () => { - api.getUsers.mockImplementation(async ({ q }) => { - if (q === zoe.uid) throw new Error('timeout') + api.getUsers.mockImplementation(async ({ uid }) => { + if (uid === zoe.uid) throw new Error('timeout') return usersResponse }) await core.router.push(`/settings/users/manage/${zoe.uid}`) @@ -414,13 +430,13 @@ describe('SettingsViewUsers.vue', () => { // Save then close: the refetch started by user:updated is dropped once the modal closes const renamed = { ...zoe, name: 'Zoe Renamed' } - api.getUsers.mockImplementation(async ({ q }) => (q === zoe.uid ? { items: [renamed] } : usersResponse)) + api.getUsers.mockImplementation(async ({ uid }) => (uid === zoe.uid ? { items: [renamed] } : usersResponse)) wrapper.findComponent(SettingsViewUsersEditModal).vm.$emit('user:updated', { uid: zoe.uid }) wrapper.findComponent(SettingsViewUsersEditModal).vm.$emit('update:modelValue', false) await flushPromises() let resolveLookup - api.getUsers.mockImplementation(({ q }) => (q === zoe.uid + api.getUsers.mockImplementation(({ uid }) => (uid === zoe.uid ? new Promise((resolve) => { resolveLookup = () => resolve({ items: [renamed] }) }) @@ -451,12 +467,12 @@ describe('SettingsViewUsers.vue', () => { await flushPromises() const granted = { ...zoe, permissions: [{ v1: 'PROJECT_MEMBER', v2: 'default::project-a' }] } - api.getUsers.mockImplementation(async ({ q }) => (q === zoe.uid ? { items: [granted] } : usersResponse)) + api.getUsers.mockImplementation(async ({ uid }) => (uid === zoe.uid ? { items: [granted] } : usersResponse)) api.getUsers.mockClear() wrapper.findComponent(SettingsViewUsersRolesModal).vm.$emit('user:updated', { uid: zoe.uid }) await flushPromises() - expect(api.getUsers).toHaveBeenCalledWith(expect.objectContaining({ q: zoe.uid })) + expect(api.getUsers).toHaveBeenCalledWith(expect.objectContaining({ uid: zoe.uid })) expect(api.getUsers).toHaveBeenCalledWith(expect.objectContaining({ q: null })) expect(wrapper.findComponent(SettingsViewUsersRolesModal).props('user')).toEqual(granted) })