Skip to content

fix no sent mailbox configured #8311

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
3 changes: 2 additions & 1 deletion lib/Contracts/IMailManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,13 @@
/**
* @param Account $account
* @param string $name
* @param array<Horde_Imap_Client::SPECIAL_USE_*> $specialUseAttributes

Check failure on line 48 in lib/Contracts/IMailManager.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

UndefinedDocblockClass

lib/Contracts/IMailManager.php:48:12: UndefinedDocblockClass: Docblock-defined class, interface or enum named OCA\Mail\Contracts\Horde_Imap_Client does not exist (see https://psalm.dev/200)
*
* @return Mailbox
*
* @throws ServiceException
*/
public function createMailbox(Account $account, string $name): Mailbox;
public function createMailbox(Account $account, string $name, array $specialUseAttributes = []): Mailbox;

/**
* @param Mailbox $mailbox
Expand Down
28 changes: 24 additions & 4 deletions lib/Controller/MailboxesController.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,16 @@
private ?string $currentUserId;
private IMailManager $mailManager;
private SyncService $syncService;


private const SUPPORTED_SPECIAL_USE_ATTRIBUTES = [
Horde_Imap_Client::SPECIALUSE_ALL,
Horde_Imap_Client::SPECIALUSE_ARCHIVE,
Horde_Imap_Client::SPECIALUSE_DRAFTS,
Horde_Imap_Client::SPECIALUSE_FLAGGED,
Horde_Imap_Client::SPECIALUSE_JUNK,
Horde_Imap_Client::SPECIALUSE_SENT,
Horde_Imap_Client::SPECIALUSE_TRASH,
];
/**
* @param string $appName
* @param IRequest $request
Expand Down Expand Up @@ -246,10 +255,21 @@
* @throws ClientException
*/
#[TrapError]
public function create(int $accountId, string $name): JSONResponse {
$account = $this->accountService->find($this->currentUserId, $accountId);
public function create(int $accountId, string $name, array $specialUseAttributes = []): JSONResponse {
$diff = array_filter($specialUseAttributes, static function ($attribute) {
return !in_array($attribute, self::SUPPORTED_SPECIAL_USE_ATTRIBUTES, true);

Check warning on line 260 in lib/Controller/MailboxesController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/MailboxesController.php#L260

Added line #L260 was not covered by tests
});
if (!empty($diff)) {
throw new ServiceException('Unsupported special use attribute: ' . implode(', ', $diff));

Check warning on line 263 in lib/Controller/MailboxesController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/MailboxesController.php#L263

Added line #L263 was not covered by tests
}

return new JSONResponse($this->mailManager->createMailbox($account, $name));
$account = $this->accountService->find($this->currentUserId, $accountId);
try {
$mailbox = $this->mailManager->createMailbox($account, $name, $specialUseAttributes);
} catch (ServiceException $e) {
return new JSONResponse([], Http::STATUS_BAD_REQUEST);

Check warning on line 270 in lib/Controller/MailboxesController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/MailboxesController.php#L269-L270

Added lines #L269 - L270 were not covered by tests
}
return new JSONResponse($mailbox);
}

/**
Expand Down
16 changes: 14 additions & 2 deletions lib/IMAP/FolderMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,22 @@ public function getFolders(Account $account, Horde_Imap_Client_Socket $client,
}, $toPersist);
}

/**
* @param Horde_Imap_Client_Socket $client
* @param Account $account
* @param string $name
* @param array<Horde_Imap_Client::SPECIAL_USE_*> $specialUseAttributes
*
* @return Folder
* @throws Horde_Imap_Client_Exception
* @throws ServiceException
*/

public function createFolder(Horde_Imap_Client_Socket $client,
Account $account,
string $name): Folder {
$client->createMailbox($name);
string $name,
array $specialUseAttributes = []): Folder {
$client->createMailbox($name, ['special_use' => $specialUseAttributes]);

$list = $client->listMailboxes($name, Horde_Imap_Client::MBOX_ALL_SUBSCRIBED, [
'delimiter' => true,
Expand Down
7 changes: 4 additions & 3 deletions lib/Service/MailManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -144,19 +144,20 @@
/**
* @param Account $account
* @param string $name
* @param array<Horde_Imap_Client::SPECIAL_USE_*> $specialUseAttributes
*
* @return Mailbox
* @throws ServiceException
*/
#[\Override]
public function createMailbox(Account $account, string $name): Mailbox {
public function createMailbox(Account $account, string $name, array $specialUseAttributes = []): Mailbox {

Check failure on line 153 in lib/Service/MailManager.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

ImplementedParamTypeMismatch

lib/Service/MailManager.php:153:70: ImplementedParamTypeMismatch: Argument 3 of OCA\Mail\Service\MailManager::createMailbox has wrong type 'array<array-key, Horde_Imap_Client::SPECIAL_USE_*>', expecting 'array<array-key, OCA\Mail\Contracts\Horde_Imap_Client::SPECIAL_USE_*>' as defined by OCA\Mail\Contracts\IMailManager::createMailbox (see https://psalm.dev/199)
$client = $this->imapClientFactory->getClient($account);
try {
$folder = $this->folderMapper->createFolder($client, $account, $name);
$folder = $this->folderMapper->createFolder($client, $account, $name, $specialUseAttributes);
$this->folderMapper->fetchFolderAcls([$folder], $client);
} catch (Horde_Imap_Client_Exception $e) {
throw new ServiceException(
'Could not get mailbox status: ' . $e->getMessage(),
'Could not create Mailbox: ' . $e->getMessage(),

Check warning on line 160 in lib/Service/MailManager.php

View check run for this annotation

Codecov / codecov/patch

lib/Service/MailManager.php#L160

Added line #L160 was not covered by tests
$e->getCode(),
$e
);
Expand Down
59 changes: 57 additions & 2 deletions src/components/NewMessageModal.vue
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ import {
NcEmptyContent as EmptyContent,
NcModal as Modal,
} from '@nextcloud/vue'
import { showError, showSuccess } from '@nextcloud/dialogs'
import { showError, showSuccess, showWarning } from '@nextcloud/dialogs'
import { translate as t } from '@nextcloud/l10n'

import logger from '../logger.js'
Expand Down Expand Up @@ -286,6 +286,57 @@ export default {
handleShow(element) {
this.additionalTrapElements.push(element)
},
async onNewSentMailbox(data, account) {
showWarning(t('mail', 'Setting Sent default folder...'))
let newSentMailboxId = null
const mailboxes = this.mainStore.getMailboxes(data.accountId)
const sentMailboxId = mailboxes.find((mailbox) => (mailbox.name === (account.personalNamespace ?? '') + 'Sent') || (mailbox.name === (account.personalNamespace ?? '') + t('mail', 'Sent')))?.databaseId
if (sentMailboxId) {
try {
await this.setSentMailboxAndResend(account, sentMailboxId, data)
showSuccess(t('mail', 'Default sent folder set'))
this.onSend(data)

} catch (error) {
logger.error('could not set sent mailbox', { error })
showError(t('mail', 'Couldn\'t set sent default folder, please try manually before sending a new message'))
}
return

}
logger.info(`creating ${t('mail', 'Sent')} mailbox`)
try {
const newSentMailbox = await this.mainStore.createMailbox({ account, name: (account.personalNamespace ?? '') + t('mail', 'Sent'), specialUseAttributes: ['\\Sent'] })
showSuccess(t('mail', 'Default sent folder set'))
logger.info(`mailbox ${(account.personalNamespace ?? '') + t('mail', 'Sent')} created`)
newSentMailboxId = newSentMailbox.databaseId
} catch (error) {
showError(t('mail', 'Could not create new mailbox, please try setting a sent mailbox manually'))
logger.error('could not create mailbox', { error })
this.$emit('close')
return
}

try {
await this.setSentMailboxAndResend(account, newSentMailboxId, data)
this.onSend(data)
} catch (error) {
logger.error('could not set sent mailbox', { error })
showError(t('mail', 'Couldn\'t set sent default folder, please try manually before sending a new message'))
this.$emit('close')
}

},

async setSentMailboxAndResend(account, id) {
logger.debug('setting sent mailbox to ' + id)
await this.mainStore.patchAccount({
account,
data: {
sentMailboxId: id,
},
})
},
/**
* @param data Message data
* @param {object=} opts Options
Expand Down Expand Up @@ -385,6 +436,11 @@ export default {
.catch((error) => logger.error('could not upload attachments', { error }))
},
async onSend(data, force = false) {
const account = this.mainStore.getAccount(data.accountId)
if (!account?.sentMailboxId) {
this.onNewSentMailbox(data, account)
return
}
logger.debug('sending message', { data })

if (this.sending) {
Expand Down Expand Up @@ -498,7 +554,6 @@ export default {
}

// Sync sent mailbox when it's currently open
const account = this.mainStore.getAccount(data.accountId)
if (account && parseInt(this.$route.params.mailboxId, 10) === account.sentMailboxId) {
setTimeout(() => {
this.mainStore.syncEnvelopes({
Expand Down
3 changes: 2 additions & 1 deletion src/service/MailboxService.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,13 @@ export async function fetchAll(accountId) {
return resp.data.mailboxes
}

export function create(accountId, name) {
export function create(accountId, name, specialUseAttributes) {
const url = generateUrl('/apps/mail/api/mailboxes')

const data = {
accountId,
name,
specialUseAttributes,
}
return axios.post(url, data).then((resp) => resp.data)
}
Expand Down
3 changes: 2 additions & 1 deletion src/store/mainStore/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -303,12 +303,13 @@ export default function mainStoreActions() {
async createMailbox({
account,
name,
specialUseAttributes = [],
}) {
return handleHttpAuthErrors(async () => {
const prefixed = (account.personalNamespace && !name.startsWith(account.personalNamespace))
? account.personalNamespace + name
: name
const mailbox = await createMailbox(account.id, prefixed)
const mailbox = await createMailbox(account.id, prefixed, specialUseAttributes)
console.debug(`mailbox ${prefixed} created for account ${account.id}`, { mailbox })
this.addMailboxMutation({
account,
Expand Down
8 changes: 4 additions & 4 deletions src/tests/unit/store/actions.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ describe('Vuex store actions', () => {
const result = await store.createMailbox({ account, name })

expect(result).toEqual(mailbox)
expect(MailboxService.create).toHaveBeenCalledWith(13, 'Important')
expect(MailboxService.create).toHaveBeenCalledWith(13, 'Important', [])
})

it('creates a sub-mailbox', async () => {
Expand All @@ -84,7 +84,7 @@ describe('Vuex store actions', () => {
const result = await store.createMailbox({ account, name })

expect(result).toEqual(mailbox)
expect(MailboxService.create).toHaveBeenCalledWith(13, 'Archive.2020')
expect(MailboxService.create).toHaveBeenCalledWith(13, 'Archive.2020', [])
})

it('adds a prefix to new mailboxes if the account has a personal namespace', async () => {
Expand All @@ -105,7 +105,7 @@ describe('Vuex store actions', () => {
const result = await store.createMailbox({ account, name })

expect(result).toEqual(mailbox)
expect(MailboxService.create).toHaveBeenCalledWith(13, 'INBOX.Important')
expect(MailboxService.create).toHaveBeenCalledWith(13, 'INBOX.Important', [])
})

it('adds no prefix to new sub-mailboxes if the account has a personal namespace', async () => {
Expand All @@ -126,7 +126,7 @@ describe('Vuex store actions', () => {
const result = await store.createMailbox({ account, name })

expect(result).toEqual(mailbox)
expect(MailboxService.create).toHaveBeenCalledWith(13, 'INBOX.Archive.2020')
expect(MailboxService.create).toHaveBeenCalledWith(13, 'INBOX.Archive.2020', [])
})

it('combines unified inbox even if no inboxes are present', async() => {
Expand Down
Loading