fix: keep create user and group dialogs open while the name check runs and on errors

packages/web-app-admin-settings/src/components/Groups/CreateGroupModal.vue

@@ -68,6 +68,7 @@ async function validateDisplayName() {
     return setDisplayNameError($gettext('Group name cannot exceed 255 characters'))
   }
 
+  formData.value.displayName.valid = false
   const exists = await clientService.graphAuthenticated.groups.getGroup(displayName).then(
     () => true,
     () => false
@@ -102,6 +103,7 @@ async function onConfirm() {
       title: $gettext('Failed to create group'),
       errors: [error]
     })
+    throw error
   }
 }
 

packages/web-app-admin-settings/src/components/Users/CreateUserModal.vue

@@ -117,6 +117,7 @@ async function validateUserName() {
     return setFieldError('userName', error)
   }
 
+  formData.value.userName.valid = false
   // the user name is taken if fetching a user with it succeeds
   const exists = await clientService.graphAuthenticated.users.getUser(userName).then(
     () => true,
@@ -171,16 +172,30 @@ async function onConfirm() {
     return Promise.reject()
   }
 
+  const client = clientService.graphAuthenticated
+  let createdUserId: string
+
+  try {
+    const createdUser = await client.users.createUser(unref(user))
+    createdUserId = createdUser.id
+  } catch (error) {
+    console.error(error)
+    showErrorMessage({
+      title: $gettext('Failed to create user'),
+      errors: [error]
+    })
+    throw error
+  }
+
+  showMessage({ title: $gettext('User was created successfully') })
+
   try {
-    const client = clientService.graphAuthenticated
-    const { id: createdUserId } = await client.users.createUser(unref(user))
     const createdUser = await client.users.getUser(createdUserId)
-    showMessage({ title: $gettext('User was created successfully') })
     userSettingsStore.upsertUser(createdUser)
   } catch (error) {
     console.error(error)
     showErrorMessage({
-      title: $gettext('Failed to create user'),
+      title: $gettext('Failed to load user'),
       errors: [error]
     })
   }

packages/web-app-admin-settings/tests/unit/components/Groups/CreateGroupModal.spec.ts

@@ -57,6 +57,15 @@ describe('CreateGroupModal', () => {
         'Group "admins" already exists'
       )
     })
+    it('disables the confirm button while the name check is running', async () => {
+      const { wrapper, mocks } = getWrapper()
+      const { getGroup } = mocks.$clientService.graphAuthenticated.groups
+      getGroup.mockRejectedValueOnce(new Error(''))
+      await setDisplayName(wrapper, 'users')
+      getGroup.mockReturnValueOnce(new Promise(() => undefined))
+      await setDisplayName(wrapper, 'admins')
+      expect(wrapper.emitted('update:confirmDisabled').at(-1)).toEqual([true])
+    })
     it('enables the confirm button when the display name is valid', async () => {
       const { wrapper, mocks } = getWrapper()
       mocks.$clientService.graphAuthenticated.groups.getGroup.mockRejectedValue(new Error(''))
@@ -87,15 +96,15 @@ describe('CreateGroupModal', () => {
       const { upsertGroup } = useGroupSettingsStore()
       expect(upsertGroup).toHaveBeenCalled()
     })
-    it('shows an error message on failure', async () => {
+    it('shows an error message and keeps the dialog open on failure', async () => {
       vi.spyOn(console, 'error').mockImplementation(() => undefined)
       const { wrapper, mocks } = getWrapper()
       const { groups } = mocks.$clientService.graphAuthenticated
       groups.getGroup.mockRejectedValue(new Error(''))
       groups.createGroup.mockRejectedValue(mockAxiosResolve({ id: '1' }))
       await setDisplayName(wrapper, 'foo bar')
 
-      await wrapper.vm.onConfirm()
+      await expect(wrapper.vm.onConfirm()).rejects.toBeDefined()
 
       const { showErrorMessage } = useMessages()
       expect(showErrorMessage).toHaveBeenCalled()

packages/web-app-admin-settings/tests/unit/components/Users/CreateUserModal.spec.ts

@@ -59,6 +59,15 @@ describe('CreateUserModal', () => {
         'User "jan" already exists'
       )
     })
+    it('disables the confirm button while the name check is running', async () => {
+      const { wrapper, mocks } = getWrapper()
+      const { getUser } = mocks.$clientService.graphAuthenticated.users
+      getUser.mockRejectedValueOnce(new Error(''))
+      await fillForm(wrapper)
+      getUser.mockReturnValueOnce(new Promise(() => undefined))
+      await setInput(wrapper, selectors.userName, 'jan')
+      expect(wrapper.emitted('update:confirmDisabled').at(-1)).toEqual([true])
+    })
     it.each(['jana', 'sk@domain.tld'])('accepts "%s" as user name', async (userName) => {
       const { wrapper, mocks } = getWrapper()
       mocks.$clientService.graphAuthenticated.users.getUser.mockRejectedValue(new Error(''))
@@ -129,18 +138,47 @@ describe('CreateUserModal', () => {
       const { showMessage } = useMessages()
       expect(showMessage).toHaveBeenCalled()
     })
-    it('shows an error message on failure', async () => {
+    it('shows an error message and keeps the dialog open when creating the user fails', async () => {
       vi.spyOn(console, 'error').mockImplementation(() => undefined)
       const { wrapper, mocks } = getWrapper()
       const { users } = mocks.$clientService.graphAuthenticated
-      users.getUser.mockRejectedValue(new Error(''))
+      users.getUser.mockRejectedValueOnce(new Error(''))
       await fillForm(wrapper)
 
-      users.createUser.mockResolvedValue(mock<User>({ id: '1' }))
-      await wrapper.vm.onConfirm()
+      const error = new Error('Could not create user')
+      users.createUser.mockRejectedValueOnce(error)
+      await expect(wrapper.vm.onConfirm()).rejects.toBe(error)
+
+      const { showMessage, showErrorMessage } = useMessages()
+      expect(showErrorMessage).toHaveBeenCalledWith({
+        title: 'Failed to create user',
+        errors: [error]
+      })
+      expect(showMessage).not.toHaveBeenCalled()
+      expect(users.getUser).toHaveBeenCalledExactlyOnceWith('foo')
+      const { upsertUser } = useUserSettingsStore()
+      expect(upsertUser).not.toHaveBeenCalled()
+    })
+    it('allows the dialog to close when the user is created but loading their details fails', async () => {
+      vi.spyOn(console, 'error').mockImplementation(() => undefined)
+      const { wrapper, mocks } = getWrapper()
+      const { users } = mocks.$clientService.graphAuthenticated
+      users.getUser.mockRejectedValueOnce(new Error(''))
+      await fillForm(wrapper)
+
+      const error = new Error('Could not load user')
+      users.createUser.mockResolvedValueOnce(mock<User>({ id: '1' }))
+      users.getUser.mockRejectedValueOnce(error)
 
-      const { showErrorMessage } = useMessages()
-      expect(showErrorMessage).toHaveBeenCalled()
+      await expect(wrapper.vm.onConfirm()).resolves.toBeUndefined()
+
+      expect(users.getUser).toHaveBeenLastCalledWith('1')
+      const { showMessage, showErrorMessage } = useMessages()
+      expect(showMessage).toHaveBeenCalledWith({ title: 'User was created successfully' })
+      expect(showErrorMessage).toHaveBeenCalledExactlyOnceWith({
+        title: 'Failed to load user',
+        errors: [error]
+      })
       const { upsertUser } = useUserSettingsStore()
       expect(upsertUser).not.toHaveBeenCalled()
     })