diff --git a/src/components/InternalAddress.vue b/src/components/InternalAddress.vue index 77bd6cf558..1653f7742e 100644 --- a/src/components/InternalAddress.vue +++ b/src/components/InternalAddress.vue @@ -62,8 +62,17 @@ v-model:open="openDialog" :buttons="buttons" :name="t('mail', 'Add internal address')" + :isForm="true" + :noClose="saving" @close="openDialog = false"> - + @@ -87,6 +96,7 @@ import IconAdd from 'vue-material-design-icons/Plus.vue' import IconDelete from 'vue-material-design-icons/TrashCanOutline.vue' import logger from '../logger.js' import useMainStore from '../store/mainStore.js' +import { parseInternalAddress } from '../util/internalAddress.js' const sortByAddress = sortBy(prop('address')) @@ -108,24 +118,36 @@ export default { return { openDialog: false, newAddress: '', - buttons: [ + saving: false, + errorMessage: '', + } + }, + + computed: { + ...mapStores(useMainStore), + buttons() { + return [ { - label: 'Cancel', + label: t('mail', 'Cancel'), icon: IconCancel, + disabled: this.saving, callback: () => { this.openDialog = false }, }, { - label: 'Ok', + label: t('mail', 'Ok'), + type: 'submit', variant: 'primary', icon: IconCheck, - callback: () => { this.addInternalAddress() }, + disabled: this.newEntry.address === '', + callback: () => this.addInternalAddress(), }, - ], - } - }, + ] + }, + + newEntry() { + return parseInternalAddress(this.newAddress) + }, - computed: { - ...mapStores(useMainStore), list() { return this.mainStore.getInternalAddresses }, @@ -139,6 +161,19 @@ export default { }, }, + watch: { + newAddress() { + this.errorMessage = '' + }, + + openDialog(open) { + if (!open) { + this.newAddress = '' + this.errorMessage = '' + } + }, + }, + methods: { async removeInternalAddress(sender) { // Remove the item immediately @@ -155,38 +190,35 @@ export default { }, async addInternalAddress() { - const type = this.checkType() + const { address, type } = this.newEntry + this.errorMessage = '' + this.saving = true try { await this.mainStore.addInternalAddress({ - address: this.newAddress, + address, type, - }).then(async () => { - this.newAddress = '' - this.openDialog = false }) + this.openDialog = false } catch (error) { - logger.error(`Could not add internal address ${this.newAddress}`, { + if (error.response?.status === 422) { + this.errorMessage = t('mail', 'Enter a valid email address or domain') + await this.$nextTick() + this.$refs.addressField?.focus() + return false + } + + logger.error(`Could not add internal address ${address}`, { error, }) showError(t('mail', 'Could not add internal address {address}', { - address: this.newAddress, + address, })) + return false + } finally { + this.saving = false } }, - checkType() { - const parts = this.newAddress.split('@') - if (parts.length !== 2) { - return 'domain' - } - // remove '@'' from domain if added by mistake - if (parts[0].length === 0) { - this.newAddress = parts[1] - return 'domain' - } - return 'individual' - }, - senderType(type) { switch (type) { case 'individual': diff --git a/src/store/mainStore/actions.js b/src/store/mainStore/actions.js index e088ee309e..18c9ac0fe0 100644 --- a/src/store/mainStore/actions.js +++ b/src/store/mainStore/actions.js @@ -1287,6 +1287,11 @@ export default function mainStoreActions() { }) { return handleHttpAuthErrors(async () => { const internalAddress = await addInternalAddress(address, type) + if (internalAddress === null) { + logger.debug('internal address is already internal') + return + } + this.addInternalAddressMutation(internalAddress) logger.debug('internal address added') }) diff --git a/src/tests/unit/components/InternalAddress.vue.spec.js b/src/tests/unit/components/InternalAddress.vue.spec.js new file mode 100644 index 0000000000..7d7a63719c --- /dev/null +++ b/src/tests/unit/components/InternalAddress.vue.spec.js @@ -0,0 +1,254 @@ +/** + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import { showError } from '@nextcloud/dialogs' +import { createTestingPinia } from '@pinia/testing' +import { enableAutoUnmount, flushPromises, shallowMount } from '@vue/test-utils' +import { setActivePinia } from 'pinia' +import { nextTick } from 'vue' +import NcButton from '@nextcloud/vue/components/NcButton' +import NcDialog from '@nextcloud/vue/components/NcDialog' +import NcTextField from '@nextcloud/vue/components/NcTextField' +import InternalAddress from '../../../components/InternalAddress.vue' +import Nextcloud from '../../../mixins/Nextcloud.js' +import useMainStore from '../../../store/mainStore.js' + +vi.mock('@nextcloud/dialogs', async (importOriginal) => ({ + ...await importOriginal(), + showError: vi.fn(), +})) + +describe('InternalAddress', () => { + enableAutoUnmount(afterEach) + + let store + + beforeEach(() => { + setActivePinia(createTestingPinia()) + store = useMainStore() + }) + + afterEach(() => { + vi.clearAllMocks() + }) + + async function mountWithAddress(address) { + const wrapper = shallowMount(InternalAddress, { + attachTo: document.body, + global: { + mixins: [Nextcloud], + stubs: { + NcInputField: false, + NcTextField: false, + }, + }, + }) + wrapper.findComponent(NcButton).vm.$emit('click') + await wrapper.findComponent(NcTextField).setValue(address) + return wrapper + } + + function findOkButton(wrapper) { + return wrapper.findComponent(NcDialog).props('buttons').find((button) => button.variant === 'primary') + } + + function findCancelButton(wrapper) { + return wrapper.findComponent(NcDialog).props('buttons').find((button) => button.variant !== 'primary') + } + + function enteredAddress(wrapper) { + return wrapper.findComponent(NcTextField).props('modelValue') + } + + it('submits the dialog form with Ok, so Enter adds the address', async () => { + const wrapper = await mountWithAddress('example.test') + + expect(wrapper.findComponent(NcDialog).props('isForm')).toBe(true) + expect(findOkButton(wrapper).type).toBe('submit') + expect(findCancelButton(wrapper).type).not.toBe('submit') + }) + + it('translates the button labels', async () => { + await mountWithAddress('example.test') + + expect(t).toHaveBeenCalledWith('mail', 'Cancel') + expect(t).toHaveBeenCalledWith('mail', 'Ok') + }) + + it.each([ + [true, ''], + [true, ' '], + [true, '@'], + [true, ' @ '], + [false, 'example.test'], + ])('sets Ok disabled to %s for "%s"', async (disabled, address) => { + const wrapper = await mountWithAddress(address) + + expect(findOkButton(wrapper).disabled).toBe(disabled) + }) + + it.each([ + [' example.test ', 'example.test', 'domain'], + ['@example.test', 'example.test', 'domain'], + [' @ example.test', 'example.test', 'domain'], + [' alice@example.test ', 'alice@example.test', 'individual'], + ])('adds "%s" as "%s" of type %s', async (input, address, type) => { + store.addInternalAddress.mockResolvedValue() + const wrapper = await mountWithAddress(input) + + await findOkButton(wrapper).callback() + + expect(store.addInternalAddress).toHaveBeenCalledWith({ address, type }) + }) + + it('keeps the Ok button busy until the internal address is added', async () => { + let finishAdding + store.addInternalAddress.mockReturnValue(new Promise((resolve) => { + finishAdding = resolve + })) + const wrapper = await mountWithAddress('example.test') + + const result = findOkButton(wrapper).callback() + await flushPromises() + + expect(await Promise.race([result, 'still saving'])).toBe('still saving') + finishAdding() + expect(await result).not.toBe(false) + await nextTick() + expect(store.addInternalAddress).toHaveBeenCalledWith({ address: 'example.test', type: 'domain' }) + expect(enteredAddress(wrapper)).toBe('') + }) + + it('blocks cancelling and closing while the internal address is saved', async () => { + let finishAdding + store.addInternalAddress.mockReturnValue(new Promise((resolve) => { + finishAdding = resolve + })) + const wrapper = await mountWithAddress('example.test') + + const result = findOkButton(wrapper).callback() + await nextTick() + + expect(findCancelButton(wrapper).disabled).toBe(true) + expect(wrapper.findComponent(NcDialog).props('noClose')).toBe(true) + expect(wrapper.find('input').attributes('readonly')).toBeDefined() + finishAdding() + await result + await nextTick() + expect(findCancelButton(wrapper).disabled).toBe(false) + expect(wrapper.findComponent(NcDialog).props('noClose')).toBe(false) + expect(wrapper.find('input').attributes('readonly')).toBeUndefined() + }) + + it('keeps Ok enabled while saving, so it keeps the focus', async () => { + store.addInternalAddress.mockReturnValue(new Promise(() => {})) + const wrapper = await mountWithAddress('example.test') + + findOkButton(wrapper).callback() + await nextTick() + + expect(findOkButton(wrapper).disabled).toBe(false) + }) + + it('keeps the dialog open if the internal address could not be added', async () => { + store.addInternalAddress.mockRejectedValue(new Error('Request failed')) + const wrapper = await mountWithAddress('example.test') + + const result = await findOkButton(wrapper).callback() + await nextTick() + + expect(result).toBe(false) + expect(showError).toHaveBeenCalledOnce() + expect(enteredAddress(wrapper)).toBe('example.test') + expect(findCancelButton(wrapper).disabled).toBe(false) + expect(findOkButton(wrapper).disabled).toBe(false) + expect(wrapper.findComponent(NcDialog).props('noClose')).toBe(false) + expect(wrapper.findComponent(NcTextField).props('error')).toBe(false) + }) + + describe('when the server rejects the address', () => { + const validationError = () => Object.assign(new Error('Request failed with status code 422'), { + response: { status: 422 }, + }) + + async function addRejectedAddress() { + store.addInternalAddress.mockRejectedValue(validationError()) + const wrapper = await mountWithAddress('not valid') + const result = await findOkButton(wrapper).callback() + await nextTick() + return { wrapper, result } + } + + it('shows the error at the text field instead of a toast', async () => { + const { wrapper, result } = await addRejectedAddress() + + expect(result).toBe(false) + expect(showError).not.toHaveBeenCalled() + expect(t).toHaveBeenCalledWith('mail', 'Enter a valid email address or domain') + expect(wrapper.findComponent(NcTextField).props('error')).toBe(true) + expect(wrapper.findComponent(NcTextField).props('helperText')).not.toBe('') + expect(enteredAddress(wrapper)).toBe('not valid') + expect(document.activeElement).toBe(wrapper.find('input').element) + }) + + it('moves the focus to the field once the error is rendered', async () => { + store.addInternalAddress.mockRejectedValue(validationError()) + const wrapper = await mountWithAddress('not valid') + const input = wrapper.find('input').element + let describedByOnFocus = null + input.addEventListener('focus', () => { + describedByOnFocus = input.getAttribute('aria-describedby') + }) + + await findOkButton(wrapper).callback() + + expect(document.activeElement).toBe(input) + expect(describedByOnFocus).toContain('helper-text') + }) + + it('hides the previous error while the address is sent again', async () => { + const { wrapper } = await addRejectedAddress() + store.addInternalAddress.mockReturnValue(new Promise(() => {})) + + findOkButton(wrapper).callback() + await nextTick() + + expect(wrapper.findComponent(NcTextField).props('error')).toBe(false) + }) + + it('ignores a rejection that arrives after the dialog is gone', async () => { + let rejectAdding + store.addInternalAddress.mockReturnValue(new Promise((resolve, reject) => { + rejectAdding = reject + })) + const wrapper = await mountWithAddress('not valid') + const result = findOkButton(wrapper).callback() + + wrapper.unmount() + rejectAdding(validationError()) + + expect(await result).toBe(false) + }) + + it('clears the error when the address is edited', async () => { + const { wrapper } = await addRejectedAddress() + + await wrapper.findComponent(NcTextField).setValue('example.test') + + expect(wrapper.findComponent(NcTextField).props('error')).toBe(false) + expect(wrapper.findComponent(NcTextField).props('helperText')).toBe('') + }) + + it('resets the address and the error when the dialog is closed', async () => { + const { wrapper } = await addRejectedAddress() + + findCancelButton(wrapper).callback() + await nextTick() + + expect(enteredAddress(wrapper)).toBe('') + expect(wrapper.findComponent(NcTextField).props('error')).toBe(false) + }) + }) +}) diff --git a/src/tests/unit/store/actions.spec.js b/src/tests/unit/store/actions.spec.js index ce7f10cc76..2e9eb319b3 100644 --- a/src/tests/unit/store/actions.spec.js +++ b/src/tests/unit/store/actions.spec.js @@ -6,6 +6,7 @@ import { createPinia, setActivePinia } from 'pinia' import { curry, range, reverse } from 'ramda' import * as AccountService from '../../../service/AccountService.js' +import * as InternalAddressService from '../../../service/InternalAddressService.js' import * as MailboxService from '../../../service/MailboxService.js' import * as MessageService from '../../../service/MessageService.js' import * as NotificationService from '../../../service/NotificationService.js' @@ -14,6 +15,7 @@ import useMainStore from '../../../store/mainStore.js' import { normalizedEnvelopeListId } from '../../../util/normalization.js' vi.mock('../../../service/AccountService.js') +vi.mock('../../../service/InternalAddressService.js') vi.mock('../../../service/MailboxService.js') vi.mock('../../../service/MessageService.js') vi.mock('../../../service/NotificationService.js') @@ -796,6 +798,28 @@ describe('Vuex store actions', () => { }) }) + describe('addInternalAddress', () => { + it('adds the created internal address', async () => { + const internalAddress = { id: 3, address: 'example.test', type: 'domain' } + InternalAddressService.addInternalAddress.mockResolvedValue(internalAddress) + + await store.addInternalAddress({ address: 'example.test', type: 'domain' }) + + expect(InternalAddressService.addInternalAddress).toHaveBeenCalledWith('example.test', 'domain') + expect(store.getInternalAddresses).toEqual([internalAddress]) + }) + + it('keeps the internal addresses unchanged if the address already exists', async () => { + const internalAddress = { id: 3, address: 'example.test', type: 'domain' } + store.addInternalAddressMutation(internalAddress) + InternalAddressService.addInternalAddress.mockResolvedValue(null) + + await store.addInternalAddress({ address: 'example.test', type: 'domain' }) + + expect(store.getInternalAddresses).toEqual([internalAddress]) + }) + }) + describe('startComposerSession reply-to resolution', () => { const account = { id: 1, diff --git a/src/tests/unit/util/internalAddress.spec.js b/src/tests/unit/util/internalAddress.spec.js new file mode 100644 index 0000000000..ff799b676a --- /dev/null +++ b/src/tests/unit/util/internalAddress.spec.js @@ -0,0 +1,22 @@ +/** + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import { parseInternalAddress } from '../../../util/internalAddress.js' + +describe('parseInternalAddress', () => { + it.each([ + ['example.test', 'example.test', 'domain'], + [' example.test ', 'example.test', 'domain'], + ['@example.test', 'example.test', 'domain'], + [' @ example.test ', 'example.test', 'domain'], + ['alice@example.test', 'alice@example.test', 'individual'], + [' alice@example.test ', 'alice@example.test', 'individual'], + ['', '', 'domain'], + [' ', '', 'domain'], + ['@', '', 'domain'], + ])('parses "%s" as "%s" of type %s', (input, address, type) => { + expect(parseInternalAddress(input)).toEqual({ address, type }) + }) +}) diff --git a/src/util/internalAddress.js b/src/util/internalAddress.js new file mode 100644 index 0000000000..dcf604ca3b --- /dev/null +++ b/src/util/internalAddress.js @@ -0,0 +1,23 @@ +/** + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +/** + * Split the text entered for a new internal address into the address and its type. + * Spaces around the text and a leading "@" before a domain are removed. + * + * @param {string} input The entered text + * @return {{ address: string, type: 'domain'|'individual' }} + */ +export function parseInternalAddress(input) { + const address = input.trim() + const parts = address.split('@') + if (parts.length !== 2) { + return { address, type: 'domain' } + } + if (parts[0].length === 0) { + return { address: parts[1].trim(), type: 'domain' } + } + return { address, type: 'individual' } +}