diff options
author | Christoph Wurst <christoph@winzerhof-wurst.at> | 2022-02-28 20:35:09 +0300 |
---|---|---|
committer | Christoph Wurst <christoph@winzerhof-wurst.at> | 2022-04-01 13:41:28 +0300 |
commit | e99758216b0a6a02087143ff7a8e3e37ff097752 (patch) | |
tree | 29841b1d92de5bfc2dbfe50e697e52e531f572a1 | |
parent | 69fdf4d15ff150523a0946850c3f91e4209ae167 (diff) |
Improve account setup error reporting
Co-authored-by: julia.kirschenheuter <julia.kirschenheuter@nextcloud.com>
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
-rw-r--r-- | lib/Controller/AccountsController.php | 32 | ||||
-rw-r--r-- | lib/Controller/SieveController.php | 6 | ||||
-rw-r--r-- | lib/Exception/CouldNotConnectException.php | 65 | ||||
-rw-r--r-- | lib/Service/SetupService.php | 9 | ||||
-rw-r--r-- | src/components/AccountForm.vue | 153 | ||||
-rw-r--r-- | src/views/Setup.vue | 41 | ||||
-rw-r--r-- | tests/Unit/Controller/AccountsControllerTest.php | 165 | ||||
-rw-r--r-- | tests/Unit/Controller/SieveControllerTest.php | 3 |
8 files changed, 322 insertions, 152 deletions
diff --git a/lib/Controller/AccountsController.php b/lib/Controller/AccountsController.php index e86c24f74..eb5c11d19 100644 --- a/lib/Controller/AccountsController.php +++ b/lib/Controller/AccountsController.php @@ -35,6 +35,7 @@ use OCA\Mail\Contracts\IMailManager; use OCA\Mail\Contracts\IMailTransmission; use OCA\Mail\Db\Mailbox; use OCA\Mail\Exception\ClientException; +use OCA\Mail\Exception\CouldNotConnectException; use OCA\Mail\Exception\ManyRecipientsException; use OCA\Mail\Exception\ServiceException; use OCA\Mail\Http\JsonResponse as MailJsonResponse; @@ -339,28 +340,37 @@ class AccountsController extends Controller { * @throws ClientException */ public function create(string $accountName, string $emailAddress, string $password = null, string $imapHost = null, int $imapPort = null, string $imapSslMode = null, string $imapUser = null, string $imapPassword = null, string $smtpHost = null, int $smtpPort = null, string $smtpSslMode = null, string $smtpUser = null, string $smtpPassword = null, bool $autoDetect = true): JSONResponse { - $account = null; - $errorMessage = null; try { if ($autoDetect) { $account = $this->setup->createNewAutoConfiguredAccount($accountName, $emailAddress, $password); } else { $account = $this->setup->createNewAccount($accountName, $emailAddress, $imapHost, $imapPort, $imapSslMode, $imapUser, $imapPassword, $smtpHost, $smtpPort, $smtpSslMode, $smtpUser, $smtpPassword, $this->currentUserId); } - } catch (Exception $ex) { - $errorMessage = $ex->getMessage(); + } catch (CouldNotConnectException $e) { + $this->logger->info('Creating account failed: ' . $e->getMessage(), [ + 'exception' => $e, + ]); + return \OCA\Mail\Http\JsonResponse::fail([ + 'error' => $e->getReason(), + 'service' => $e->getService(), + 'host' => $e->getHost(), + 'port' => $e->getPort(), + ]); + } catch (ServiceException $e) { + $this->logger->error('Creating account failed: ' . $e->getMessage(), [ + 'exception' => $e, + ]); + return \OCA\Mail\Http\JsonResponse::error('Could not create account'); } if (is_null($account)) { - if ($autoDetect) { - throw new ClientException($this->l10n->t('Auto detect failed. Please use manual mode.')); - } else { - $this->logger->error('Creating account failed: ' . $errorMessage); - throw new ClientException($this->l10n->t('Creating account failed: ') . $errorMessage); - } + return \OCA\Mail\Http\JsonResponse::fail([ + 'error' => 'AUTOCONFIG_FAILED', + 'message' => $this->l10n->t('Auto detect failed. Please use manual mode.'), + ]); } - return new JSONResponse($account, Http::STATUS_CREATED); + return \OCA\Mail\Http\JsonResponse::success($account, Http::STATUS_CREATED); } /** diff --git a/lib/Controller/SieveController.php b/lib/Controller/SieveController.php index 11fc5b186..ca1bfd4ce 100644 --- a/lib/Controller/SieveController.php +++ b/lib/Controller/SieveController.php @@ -179,7 +179,7 @@ class SieveController extends Controller { try { $this->sieveClientFactory->createClient($sieveHost, $sievePort, $sieveUser, $sievePassword, $sieveSslMode); } catch (ManagesieveException $e) { - throw CouldNotConnectException::create($e, 'ManageSieve', $sieveHost, $sievePort); + throw new CouldNotConnectException($e, 'ManageSieve', $sieveHost, $sievePort); } $mailAccount->setSieveEnabled(true); @@ -205,13 +205,13 @@ class SieveController extends Controller { $account = $this->accountService->find($this->currentUserId, $id); if (!$account->getMailAccount()->isSieveEnabled()) { - throw new CouldNotConnectException('ManageSieve is disabled.'); + throw new ClientException('ManageSieve is disabled.'); } try { $sieve = $this->sieveClientFactory->getClient($account); } catch (ManagesieveException $e) { - throw CouldNotConnectException::create($e, 'ManageSieve', $account->getMailAccount()->getSieveHost(), $account->getMailAccount()->getSievePort()); + throw new CouldNotConnectException($e, 'ManageSieve', $account->getMailAccount()->getSieveHost(), $account->getMailAccount()->getSievePort()); } return $sieve; diff --git a/lib/Exception/CouldNotConnectException.php b/lib/Exception/CouldNotConnectException.php index c996095ee..e6759e94e 100644 --- a/lib/Exception/CouldNotConnectException.php +++ b/lib/Exception/CouldNotConnectException.php @@ -23,14 +23,69 @@ declare(strict_types=1); namespace OCA\Mail\Exception; +use Horde_Imap_Client_Exception; use Throwable; class CouldNotConnectException extends ServiceException { - public static function create(Throwable $exception, string $service, string $host, int $port): self { - return new self( - "Connection to {$service} at {$host}:{$port} failed. {$exception->getMessage()}", - (int)$exception->getCode(), - $exception + + /** @var string */ + private $service; + + /** @var string */ + private $host; + + /** @var int */ + private $port; + + /** @var Throwable */ + private $previous; + + public function __construct(Throwable $previous, string $service, string $host, int $port) { + parent::__construct( + "Connection to {$service} at {$host}:{$port} failed. {$previous->getMessage()}", + (int)$previous->getCode(), + $previous ); + $this->service = $service; + $this->host = $host; + $this->port = $port; + $this->previous = $previous; + } + + /** + * @return string + */ + public function getService(): string { + return $this->service; + } + + /** + * @return string + */ + public function getHost(): string { + return $this->host; + } + + /** + * @return int + */ + public function getPort(): int { + return $this->port; + } + + public function getReason(): string { + if (!($this->previous instanceof Horde_Imap_Client_Exception)) { + return 'OTHER'; + } + + switch ($this->previous->getCode()) { + case Horde_Imap_Client_Exception::LOGIN_AUTHENTICATIONFAILED: + return 'AUTHENTICATION'; + case Horde_Imap_Client_Exception::SERVER_CONNECT: + case Horde_Imap_Client_Exception::SERVER_READERROR: + return 'CONNECTION_ERROR'; + default: + return 'OTHER'; + } } } diff --git a/lib/Service/SetupService.php b/lib/Service/SetupService.php index 78b423425..89152eb96 100644 --- a/lib/Service/SetupService.php +++ b/lib/Service/SetupService.php @@ -117,11 +117,12 @@ class SetupService { * @param string $uid * @param int|null $accountId * + * @throws CouldNotConnectException * @throws ServiceException * - * @return Account|null + * @return Account */ - public function createNewAccount($accountName, $emailAddress, $imapHost, $imapPort, $imapSslMode, $imapUser, $imapPassword, $smtpHost, $smtpPort, $smtpSslMode, $smtpUser, $smtpPassword, $uid, ?int $accountId = null): ?Account { + public function createNewAccount($accountName, $emailAddress, $imapHost, $imapPort, $imapSslMode, $imapUser, $imapPassword, $smtpHost, $smtpPort, $smtpSslMode, $smtpUser, $smtpPassword, $uid, ?int $accountId = null): Account { $this->logger->info('Setting up manually configured account'); $newAccount = new MailAccount([ 'accountId' => $accountId, @@ -165,7 +166,7 @@ class SetupService { try { $imapClient->login(); } catch (Horde_Imap_Client_Exception $e) { - throw CouldNotConnectException::create($e, 'IMAP', $mailAccount->getInboundHost(), $mailAccount->getInboundPort()); + throw new CouldNotConnectException($e, 'IMAP', $mailAccount->getInboundHost(), $mailAccount->getInboundPort()); } $transport = $this->smtpClientFactory->create($account); @@ -173,7 +174,7 @@ class SetupService { try { $transport->getSMTPObject(); } catch (Horde_Mail_Exception $e) { - throw CouldNotConnectException::create($e, 'SMTP', $mailAccount->getOutboundHost(), $mailAccount->getOutboundPort()); + throw new CouldNotConnectException($e, 'SMTP', $mailAccount->getOutboundHost(), $mailAccount->getOutboundPort()); } } } diff --git a/src/components/AccountForm.vue b/src/components/AccountForm.vue index 2fa0fc265..5e231dc55 100644 --- a/src/components/AccountForm.vue +++ b/src/components/AccountForm.vue @@ -13,15 +13,19 @@ :placeholder="t('mail', 'Name')" :disabled="loading" autofocus> - <label for="auto-address">{{ t('mail', 'Mail Address') }}</label> + <label for="auto-address" class="account-form__label--required">{{ t('mail', 'Mail address') }}</label> <input id="auto-address" - v-model="autoConfig.emailAddress" + v-model.lazy="autoConfig.emailAddress" type="email" - :placeholder="t('mail', 'Mail Address')" + :placeholder="t('mail', 'name@example.org')" :disabled="loading" - required> - <label for="auto-password">{{ t('mail', 'Password') }}</label> + required + @blur="isValidEmail(autoConfig.emailAddress)"> + <p v-if="!isValidEmail(autoConfig.emailAddress)" class="account-form--error"> + {{ t('mail', 'Please enter an email of the format name@example.com') }} + </p> + <label for="auto-password" class="account-form__label--required">{{ t('mail', 'Password') }}</label> <input id="auto-password" v-model="autoConfig.password" @@ -38,17 +42,21 @@ type="text" :placeholder="t('mail', 'Name')" :disabled="loading"> - <label for="man-address">{{ t('mail', 'Mail Address') }}</label> + <label for="man-address" class="account-form__label--required">{{ t('mail', 'Mail address') }}</label> <input id="man-address" - v-model="manualConfig.emailAddress" + v-model.lazy="manualConfig.emailAddress" type="email" - :placeholder="t('mail', 'Mail Address')" + :placeholder="t('mail', 'name@example.org')" :disabled="loading" - required> + required + @blur="isValidEmail(manualConfig.emailAddress)"> + <p v-if="!isValidEmail(manualConfig.emailAddress)" class="account-form--error"> + {{ t('mail', 'Please enter an email of the format name@example.com') }} + </p> <h3>{{ t('mail', 'IMAP Settings') }}</h3> - <label for="man-imap-host">{{ t('mail', 'IMAP Host') }}</label> + <label for="man-imap-host" class="account-form__label--required">{{ t('mail', 'IMAP Host') }}</label> <input id="man-imap-host" v-model="manualConfig.imapHost" @@ -56,7 +64,9 @@ :placeholder="t('mail', 'IMAP Host')" :disabled="loading" required> - <h4>{{ t('mail', 'IMAP Security') }}</h4> + <h4 class="account-form__heading--required"> + {{ t('mail', 'IMAP Security') }} + </h4> <div class="flex-row"> <input id="man-imap-sec-none" @@ -95,7 +105,7 @@ for="man-imap-sec-tls" :class="{primary: manualConfig.imapSslMode === 'tls'}">{{ t('mail', 'STARTTLS') }}</label> </div> - <label for="man-imap-port">{{ t('mail', 'IMAP Port') }}</label> + <label for="man-imap-port" class="account-form__label--required">{{ t('mail', 'IMAP Port') }}</label> <input id="man-imap-port" v-model="manualConfig.imapPort" @@ -103,7 +113,7 @@ :placeholder="t('mail', 'IMAP Port')" :disabled="loading" required> - <label for="man-imap-user">{{ t('mail', 'IMAP User') }}</label> + <label for="man-imap-user" class="account-form__label--required">{{ t('mail', 'IMAP User') }}</label> <input id="man-imap-user" v-model="manualConfig.imapUser" @@ -111,7 +121,7 @@ :placeholder="t('mail', 'IMAP User')" :disabled="loading" required> - <label for="man-imap-password">{{ t('mail', 'IMAP Password') }}</label> + <label for="man-imap-password" class="account-form__label--required">{{ t('mail', 'IMAP Password') }}</label> <input id="man-imap-password" v-model="manualConfig.imapPassword" @@ -121,7 +131,7 @@ required> <h3>{{ t('mail', 'SMTP Settings') }}</h3> - <label for="man-smtp-host">{{ t('mail', 'SMTP Host') }}</label> + <label for="man-smtp-host" class="account-form__label--required">{{ t('mail', 'SMTP Host') }}</label> <input id="man-smtp-host" ref="smtpHost" @@ -131,7 +141,9 @@ :placeholder="t('mail', 'SMTP Host')" :disabled="loading" required> - <h4>{{ t('mail', 'SMTP Security') }}</h4> + <h4 class="account-form__heading--required"> + {{ t('mail', 'SMTP Security') }} + </h4> <div class="flex-row"> <input id="man-smtp-sec-none" @@ -170,7 +182,7 @@ for="man-smtp-sec-tls" :class="{primary: manualConfig.smtpSslMode === 'tls'}">{{ t('mail', 'STARTTLS') }}</label> </div> - <label for="man-smtp-port">{{ t('mail', 'SMTP Port') }}</label> + <label for="man-smtp-port" class="account-form__label--required">{{ t('mail', 'SMTP Port') }}</label> <input id="man-smtp-port" v-model="manualConfig.smtpPort" @@ -178,7 +190,7 @@ :placeholder="t('mail', 'SMTP Port')" :disabled="loading" required> - <label for="man-smtp-user">{{ t('mail', 'SMTP User') }}</label> + <label for="man-smtp-user" class="account-form__label--required">{{ t('mail', 'SMTP User') }}</label> <input id="man-smtp-user" v-model="manualConfig.smtpUser" @@ -186,7 +198,7 @@ :placeholder="t('mail', 'SMTP User')" :disabled="loading" required> - <label for="man-smtp-password">{{ t('mail', 'SMTP Password') }}</label> + <label for="man-smtp-password" class="account-form__label--required">{{ t('mail', 'SMTP Password') }}</label> <input id="man-smtp-password" v-model="manualConfig.smtpPassword" @@ -196,12 +208,28 @@ required> </Tab> </Tabs> - <slot name="feedback" /> - <input type="submit" - class="primary" - :disabled="loading" - :value="submitButtonText" - @click.prevent="onSubmit"> + <div class="account-form__submit-buttons"> + <button v-if="mode === 'auto'" + class="primary account-form__submit-button" + type="submit" + :disabled="isDisabledAuto" + @click.prevent="onSubmit"> + <span v-if="loading" class="icon-loading-small account-form__submit-button__spinner" /> + {{ submitButtonText }} + </button> + + <button v-else-if="mode === 'manual'" + class="primary account-form__submit-button" + type="submit" + :disabled="isDisabledManual" + @click.prevent="onSubmit"> + <span v-if="loading" class="icon-loading-small account-form__submit-button__spinner" /> + {{ submitButtonText }} + </button> + </div> + <div class="account-form--feedback"> + <slot name="feedback" /> + </div> </form> </template> @@ -266,13 +294,39 @@ export default { smtpUser: fromAccountOr('smtpUser', ''), smtpPassword: '', }, - submitButtonText: this.account ? t('mail', 'Save') : t('mail', 'Connect'), } }, computed: { settingsPage() { return this.account !== undefined }, + + isDisabledAuto() { + switch (this.mode === 'auto') { + case !this.autoConfig.emailAddress || !this.isValidEmail(this.autoConfig.emailAddress) || !this.autoConfig.password : + return true + } + return this.loading + }, + + isDisabledManual() { + switch (this.mode === 'manual') { + case !this.manualConfig.emailAddress || !this.isValidEmail(this.manualConfig.emailAddress) + || !this.manualConfig.imapHost || !this.manualConfig.imapPort + || !this.manualConfig.imapUser || !this.manualConfig.imapPassword + || !this.manualConfig.smtpHost || !this.manualConfig.smtpPort + || !this.manualConfig.smtpUser || !this.manualConfig.smtpPassword: + return true + } + return this.loading + }, + + submitButtonText() { + if (this.loading) { + return t('mail', 'Connecting') + } + return this.account ? t('mail', 'Save') : t('mail', 'Connect') + }, }, methods: { onModeChanged(e) { @@ -339,13 +393,17 @@ export default { } }, onSubmit(event) { - console.debug('account form submitted', { event }) - - this.loading = true - - this.saveChanges() - .catch((error) => logger.error('could not save account details', { error })) - .then(() => (this.loading = false)) + if (!this.isDisabledManual || !this.isDisabledAuto) { + console.debug('account form submitted', { event }) + this.loading = true + this.saveChanges() + .catch((error) => logger.error('could not save account details', { error })) + .then(() => (this.loading = false)) + } + }, + isValidEmail(email) { + const regExpEmail = /^(([^<>()[\]\\.,;:\s@"]+(\.[^<>()[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/ + return regExpEmail.test(email) }, }, } @@ -413,4 +471,33 @@ input[type='radio'][disabled] + label { cursor: default; opacity: 0.5; } +.account-form__label--required:after { + content:" *"; +} +.account-form__heading--required:after { + content:" *"; +} +.account-form__submit-buttons { + display: flex; + justify-content: center; + margin-top: 5px; +} +.account-form__submit-button { + display: flex; + align-items: center; +} +.account-form__submit-button__spinner { + margin: 0 10px 0 0; + height: auto; + width: auto; +} +.account-form--feedback { + color: var(--color-text-maxcontrast); + margin-top: 5px; + text-align: center; +} +.account-form--error { + text-align: left; + font-size: 14px; +} </style> diff --git a/src/views/Setup.vue b/src/views/Setup.vue index 647a50e5e..31285a9d7 100644 --- a/src/views/Setup.vue +++ b/src/views/Setup.vue @@ -1,14 +1,17 @@ <template> <Content app-name="mail"> <Navigation v-if="hasAccounts" /> - <div id="emptycontent"> - <div class="icon-mail" /> - <h2>{{ t('mail', 'Connect your mail account') }}</h2> - <AccountForm :display-name="displayName" :email="email" :save="onSave"> - <template v-if="error" #feedback class="warning"> - {{ error }} + <div class="mail-empty-content"> + <EmptyContent icon="icon-mail"> + <h2>{{ t('mail', 'Connect your mail account') }}</h2> + <template #desc> + <AccountForm :display-name="displayName" :email="email" :save="onSave"> + <template v-if="error" #feedback class="warning"> + {{ error }} + </template> + </AccountForm> </template> - </AccountForm> + </EmptyContent> </div> </Content> </template> @@ -19,6 +22,7 @@ import { loadState } from '@nextcloud/initial-state' import { translate as t } from '@nextcloud/l10n' import AccountForm from '../components/AccountForm' +import EmptyContent from '@nextcloud/vue/dist/Components/EmptyContent' import Navigation from '../components/Navigation' import logger from '../logger' @@ -27,6 +31,7 @@ export default { components: { AccountForm, Content, + EmptyContent, Navigation, }, data() { @@ -58,10 +63,22 @@ export default { .catch((error) => { logger.error('Could not create account', { error }) - if (error?.data?.message) { - this.error = error.data.message + if (error.data?.error === 'AUTOCONFIG_FAILED') { + this.error = t('mail', 'Auto detect failed. Please try manual mode.') + } else if (error.data?.error === 'CONNECTION_ERROR') { + if (error.data.service === 'IMAP') { + this.error = t('mail', 'Manual config failed. IMAP server is not reachable.') + } else if (error.data.service === 'SMTP') { + this.error = t('mail', 'Manual config failed. SMTP server is not reachable.') + } + } else if (error.data?.error === 'AUTHENTICATION') { + if (error.data.service === 'IMAP') { + this.error = t('mail', 'Manual config failed. IMAP username or password is wrong.') + } else if (error.data.service === 'SMTP') { + this.error = t('mail', 'Manual config failed. SMTP username or password is wrong.') + } } else { - this.error = t('mail', 'Unexpected error during account creation') + this.error = t('mail', 'There was an error while setting up your account. Please try again.') } }) }, @@ -70,7 +87,7 @@ export default { </script> <style> -#emptycontent { - margin-top: 10vh; +.mail-empty-content { + margin: 0 auto; } </style> diff --git a/tests/Unit/Controller/AccountsControllerTest.php b/tests/Unit/Controller/AccountsControllerTest.php index 5f8e8fe36..18e2a4f2b 100644 --- a/tests/Unit/Controller/AccountsControllerTest.php +++ b/tests/Unit/Controller/AccountsControllerTest.php @@ -25,7 +25,6 @@ declare(strict_types=1); namespace OCA\Mail\Tests\Unit\Controller; use ChristophWurst\Nextcloud\Testing\TestCase; -use Exception; use Horde_Exception; use OCA\Mail\Account; use OCA\Mail\Contracts\IMailManager; @@ -112,7 +111,7 @@ class AccountsControllerTest extends TestCase { $this->request = $this->createMock(IRequest::class); $this->accountService = $this->createMock(AccountService::class); $this->groupsIntegration = $this->createMock(GroupsIntegration::class); - $this->groupsIntegration->expects($this->any()) + $this->groupsIntegration->expects(self::any()) ->method('expand') ->will($this->returnArgument(0)); $this->userId = 'manfred'; @@ -144,20 +143,20 @@ class AccountsControllerTest extends TestCase { $this->accountId = 123; } - public function testIndex() { - $this->account->expects($this->once()) + public function testIndex(): void { + $this->account->expects(self::once()) ->method('jsonSerialize') - ->will($this->returnValue([ + ->will(self::returnValue([ 'accountId' => 123, ])); - $this->accountService->expects($this->once()) + $this->accountService->expects(self::once()) ->method('findByUserId') - ->with($this->equalTo($this->userId)) - ->will($this->returnValue([$this->account])); - $this->aliasesService->expects($this->any()) + ->with(self::equalTo($this->userId)) + ->will(self::returnValue([$this->account])); + $this->aliasesService->expects(self::any()) ->method('findAll') - ->with($this->equalTo($this->accountId), $this->equalTo($this->userId)) - ->will($this->returnValue(['a1', 'a2'])); + ->with(self::equalTo($this->accountId), self::equalTo($this->userId)) + ->will(self::returnValue(['a1', 'a2'])); $response = $this->controller->index(); @@ -170,80 +169,80 @@ class AccountsControllerTest extends TestCase { ], ] ]); - $this->assertEquals($expectedResponse, $response); + self::assertEquals($expectedResponse, $response); } - public function testShow() { - $this->accountService->expects($this->once()) + public function testShow(): void { + $this->accountService->expects(self::once()) ->method('find') - ->with($this->equalTo($this->userId), $this->equalTo($this->accountId)) - ->will($this->returnValue($this->account)); + ->with(self::equalTo($this->userId), self::equalTo($this->accountId)) + ->will(self::returnValue($this->account)); $response = $this->controller->show($this->accountId); $expectedResponse = new JSONResponse($this->account); - $this->assertEquals($expectedResponse, $response); + self::assertEquals($expectedResponse, $response); } - public function testShowDoesNotExist() { - $this->accountService->expects($this->once()) + public function testShowDoesNotExist(): void { + $this->accountService->expects(self::once()) ->method('find') - ->with($this->equalTo($this->userId), $this->equalTo($this->accountId)) - ->will($this->throwException(new DoesNotExistException('test123'))); + ->with(self::equalTo($this->userId), self::equalTo($this->accountId)) + ->will(self::throwException(new DoesNotExistException('test123'))); $this->expectException(DoesNotExistException::class); $this->controller->show($this->accountId); } - public function testUpdateSignature() { - $this->accountService->expects($this->once()) + public function testUpdateSignature(): void { + $this->accountService->expects(self::once()) ->method('updateSignature') - ->with($this->equalTo($this->accountId), $this->equalTo($this->userId), 'sig'); + ->with(self::equalTo($this->accountId), self::equalTo($this->userId), 'sig'); $response = $this->controller->updateSignature($this->accountId, 'sig'); $expectedResponse = new JSONResponse(); - $this->assertEquals($expectedResponse, $response); + self::assertEquals($expectedResponse, $response); } - public function testDeleteSignature() { - $this->accountService->expects($this->once()) + public function testDeleteSignature(): void { + $this->accountService->expects(self::once()) ->method('updateSignature') - ->with($this->equalTo($this->accountId), $this->equalTo($this->userId), null); + ->with(self::equalTo($this->accountId), self::equalTo($this->userId), null); $response = $this->controller->updateSignature($this->accountId, null); $expectedResponse = new JSONResponse(); - $this->assertEquals($expectedResponse, $response); + self::assertEquals($expectedResponse, $response); } - public function testDestroy() { - $this->accountService->expects($this->once()) + public function testDestroy(): void { + $this->accountService->expects(self::once()) ->method('delete') - ->with($this->equalTo($this->userId), $this->equalTo($this->accountId)); + ->with(self::equalTo($this->userId), self::equalTo($this->accountId)); $response = $this->controller->destroy($this->accountId); $expectedResponse = new JSONResponse(); - $this->assertEquals($expectedResponse, $response); + self::assertEquals($expectedResponse, $response); } - public function testDestroyDoesNotExist() { - $this->accountService->expects($this->once()) + public function testDestroyDoesNotExist(): void { + $this->accountService->expects(self::once()) ->method('delete') - ->with($this->equalTo($this->userId), $this->equalTo($this->accountId)) - ->will($this->throwException(new DoesNotExistException('test'))); + ->with(self::equalTo($this->userId), self::equalTo($this->accountId)) + ->will(self::throwException(new DoesNotExistException('test'))); $this->expectException(DoesNotExistException::class); $this->controller->destroy($this->accountId); } - public function testCreateAutoDetectSuccess() { + public function testCreateAutoDetectSuccess(): void { $email = 'john@example.com'; $password = '123456'; $accountName = 'John Doe'; $account = $this->createMock(Account::class); - $this->setupService->expects($this->once()) + $this->setupService->expects(self::once()) ->method('createNewAutoconfiguredAccount') ->with($accountName, $email, $password) ->willReturn($account); @@ -251,31 +250,31 @@ class AccountsControllerTest extends TestCase { $response = $this->controller->create($accountName, $email, $password, null, null, null, null, null, null, null, null, null, null, true); - $expectedResponse = new JSONResponse($account, Http::STATUS_CREATED); + $expectedResponse = \OCA\Mail\Http\JsonResponse::success($account, Http::STATUS_CREATED); - $this->assertEquals($expectedResponse, $response); + self::assertEquals($expectedResponse, $response); } - public function testCreateAutoDetectFailure() { + public function testCreateAutoDetectFailure(): void { $email = 'john@example.com'; $password = '123456'; $accountName = 'John Doe'; - $this->setupService->expects($this->once()) + $this->setupService->expects(self::once()) ->method('createNewAutoconfiguredAccount') ->with($accountName, $email, $password) - ->willThrowException(new \Exception()); + ->willThrowException(new ClientException()); $this->expectException(ClientException::class); $this->controller->create($accountName, $email, $password, null, null, null, null, null, null, null, null, null, null, true); } - public function testUpdateAutoDetectSuccess() { + public function testUpdateAutoDetectSuccess(): void { $email = 'john@example.com'; $password = '123456'; $accountName = 'John Doe'; $account = $this->createMock(Account::class); - $this->setupService->expects($this->once()) + $this->setupService->expects(self::once()) ->method('createNewAutoconfiguredAccount') ->with($accountName, $email, $password) ->willReturn($account); @@ -283,26 +282,26 @@ class AccountsControllerTest extends TestCase { $response = $this->controller->create($accountName, $email, $password, null, null, null, null, null, null, null, null, null, null, true); - $expectedResponse = new JSONResponse($account, Http::STATUS_CREATED); + $expectedResponse = \OCA\Mail\Http\JsonResponse::success($account, Http::STATUS_CREATED); - $this->assertEquals($expectedResponse, $response); + self::assertEquals($expectedResponse, $response); } - public function testUpdateAutoDetectFailure() { + public function testUpdateAutoDetectFailure(): void { $email = 'john@example.com'; $password = '123456'; $accountName = 'John Doe'; - $this->setupService->expects($this->once()) + $this->setupService->expects(self::once()) ->method('createNewAutoconfiguredAccount') ->with($accountName, $email, $password) - ->willThrowException(new Exception()); + ->willThrowException(new ClientException()); $this->expectException(ClientException::class); $this->controller->create($accountName, $email, $password, null, null, null, null, null, null, null, null, null, null, true); } - public function testCreateManualSuccess() { + public function testCreateManualSuccess(): void { $autoDetect = false; $email = 'user@domain.tld'; $password = 'mypassword'; @@ -318,19 +317,19 @@ class AccountsControllerTest extends TestCase { $smtpUser = 'user@domain.tld'; $smtpPassword = 'mypassword'; $account = $this->createMock(Account::class); - $this->setupService->expects($this->once()) + $this->setupService->expects(self::once()) ->method('createNewAccount') ->with($accountName, $email, $imapHost, $imapPort, $imapSslMode, $imapUser, $imapPassword, $smtpHost, $smtpPort, $smtpSslMode, $smtpUser, $smtpPassword, $this->userId) ->willReturn($account); $response = $this->controller->create($accountName, $email, $password, $imapHost, $imapPort, $imapSslMode, $imapUser, $imapPassword, $smtpHost, $smtpPort, $smtpSslMode, $smtpUser, $smtpPassword, $autoDetect); - $expectedResponse = new JSONResponse($account, Http::STATUS_CREATED); + $expectedResponse = \OCA\Mail\Http\JsonResponse::success($account, Http::STATUS_CREATED); - $this->assertEquals($expectedResponse, $response); + self::assertEquals($expectedResponse, $response); } - public function testCreateManualFailure() { + public function testCreateManualFailure(): void { $autoDetect = false; $email = 'user@domain.tld'; $password = 'mypassword'; @@ -345,16 +344,16 @@ class AccountsControllerTest extends TestCase { $smtpSslMode = 'none'; $smtpUser = 'user@domain.tld'; $smtpPassword = 'mypassword'; - $this->setupService->expects($this->once()) + $this->setupService->expects(self::once()) ->method('createNewAccount') ->with($accountName, $email, $imapHost, $imapPort, $imapSslMode, $imapUser, $imapPassword, $smtpHost, $smtpPort, $smtpSslMode, $smtpUser, $smtpPassword, $this->userId) - ->willThrowException(new Exception()); + ->willThrowException(new ClientException()); $this->expectException(ClientException::class); $this->controller->create($accountName, $email, $password, $imapHost, $imapPort, $imapSslMode, $imapUser, $imapPassword, $smtpHost, $smtpPort, $smtpSslMode, $smtpUser, $smtpPassword, $autoDetect); } - public function testUpdateManualSuccess() { + public function testUpdateManualSuccess(): void { $autoDetect = false; $id = 135; $email = 'user@domain.tld'; @@ -371,7 +370,7 @@ class AccountsControllerTest extends TestCase { $smtpUser = 'user@domain.tld'; $smtpPassword = 'mypassword'; $account = $this->createMock(Account::class); - $this->setupService->expects($this->once()) + $this->setupService->expects(self::once()) ->method('createNewAccount') ->with($accountName, $email, $imapHost, $imapPort, $imapSslMode, $imapUser, $imapPassword, $smtpHost, $smtpPort, $smtpSslMode, $smtpUser, $smtpPassword, $this->userId, $id) ->willReturn($account); @@ -380,10 +379,10 @@ class AccountsControllerTest extends TestCase { $expectedResponse = new JSONResponse($account); - $this->assertEquals($expectedResponse, $response); + self::assertEquals($expectedResponse, $response); } - public function testUpdateManualFailure() { + public function testUpdateManualFailure(): void { $autoDetect = false; $id = 135; $email = 'user@domain.tld'; @@ -399,38 +398,38 @@ class AccountsControllerTest extends TestCase { $smtpSslMode = 'none'; $smtpUser = 'user@domain.tld'; $smtpPassword = 'mypassword'; - $this->setupService->expects($this->once()) + $this->setupService->expects(self::once()) ->method('createNewAccount') ->with($accountName, $email, $imapHost, $imapPort, $imapSslMode, $imapUser, $imapPassword, $smtpHost, $smtpPort, $smtpSslMode, $smtpUser, $smtpPassword, $this->userId, $id) - ->willThrowException(new Exception()); + ->willThrowException(new ClientException()); $this->expectException(ClientException::class); $this->controller->update($id, $autoDetect, $accountName, $email, $password, $imapHost, $imapPort, $imapSslMode, $imapUser, $imapPassword, $smtpHost, $smtpPort, $smtpSslMode, $smtpUser, $smtpPassword); } - public function testSendNewMessage() { + public function testSendNewMessage(): void { $account = $this->createMock(Account::class); - $this->accountService->expects($this->once()) + $this->accountService->expects(self::once()) ->method('find') ->willReturn($account); $messageData = NewMessageData::fromRequest($account, 'to@d.com', '', '', 'sub', 'bod', []); - $this->transmission->expects($this->once()) + $this->transmission->expects(self::once()) ->method('sendMessage') ->with($messageData, null, null, null); $expected = new JSONResponse(); $resp = $this->controller->send(13, 'sub', 'bod', 'to@d.com', '', ''); - $this->assertEquals($expected, $resp); + self::assertEquals($expected, $resp); } - public function testSendingError() { + public function testSendingError(): void { $account = $this->createMock(Account::class); - $this->accountService->expects($this->once()) + $this->accountService->expects(self::once()) ->method('find') ->willReturn($account); $messageData = NewMessageData::fromRequest($account, 'to@d.com', '', '', 'sub', 'bod', []); - $this->transmission->expects($this->once()) + $this->transmission->expects(self::once()) ->method('sendMessage') ->with($messageData, null, null, null) ->willThrowException(new Horde_Exception('error')); @@ -439,7 +438,7 @@ class AccountsControllerTest extends TestCase { $this->controller->send(13, 'sub', 'bod', 'to@d.com', '', ''); } - public function testSendingManyRecipientsError() { + public function testSendingManyRecipientsError(): void { $this->expectException(ManyRecipientsException::class); $recipients = []; @@ -451,7 +450,7 @@ class AccountsControllerTest extends TestCase { $this->controller->send(13, 'sub', 'bod', $recipients, '', ''); } - public function testSendingManyRecipientsCcError() { + public function testSendingManyRecipientsCcError(): void { $this->expectException(ManyRecipientsException::class); $recipients = []; @@ -463,20 +462,20 @@ class AccountsControllerTest extends TestCase { $this->controller->send(13, 'sub', 'bod', '', $recipients, ''); } - public function testSendReply() { + public function testSendReply(): void { $account = $this->createMock(Account::class); $replyMessage = new Message(); $messageId = 1234; - $this->accountService->expects($this->once()) + $this->accountService->expects(self::once()) ->method('find') ->willReturn($account); - $this->mailManager->expects($this->once()) + $this->mailManager->expects(self::once()) ->method('getMessage') ->with($this->userId, $messageId) ->willReturn($replyMessage); $messageData = NewMessageData::fromRequest($account, 'to@d.com', '', '', 'sub', 'bod', []); $replyData = new RepliedMessageData($account, $replyMessage); - $this->transmission->expects($this->once()) + $this->transmission->expects(self::once()) ->method('sendMessage') ->with($messageData, $replyData, null, null); $expected = new JSONResponse(); @@ -495,7 +494,7 @@ class AccountsControllerTest extends TestCase { [], null); - $this->assertEquals($expected, $resp); + self::assertEquals($expected, $resp); } public function draftDataProvider(): array { @@ -518,14 +517,14 @@ class AccountsControllerTest extends TestCase { $newUid = 124; $account = $this->createMock(Account::class); $mailbox = new Mailbox(); - $this->accountService->expects($this->once()) + $this->accountService->expects(self::once()) ->method('find') ->with($this->userId, $this->accountId) - ->will($this->returnValue($this->account)); - $this->transmission->expects($this->once()) + ->will(self::returnValue($this->account)); + $this->transmission->expects(self::once()) ->method('saveDraft') ->willReturn([$account, $mailbox, $newUid]); - $this->mailManager->expects($this->once()) + $this->mailManager->expects(self::once()) ->method('getMessageIdForUid') ->willReturn($newId); @@ -534,6 +533,6 @@ class AccountsControllerTest extends TestCase { $expected = new JSONResponse([ 'id' => $newId, ]); - $this->assertEquals($expected, $actual); + self::assertEquals($expected, $actual); } } diff --git a/tests/Unit/Controller/SieveControllerTest.php b/tests/Unit/Controller/SieveControllerTest.php index 2d80afcbb..bad9ff61d 100644 --- a/tests/Unit/Controller/SieveControllerTest.php +++ b/tests/Unit/Controller/SieveControllerTest.php @@ -28,6 +28,7 @@ use Horde\ManageSieve\Exception; use OCA\Mail\Account; use OCA\Mail\Controller\SieveController; use OCA\Mail\Db\MailAccount; +use OCA\Mail\Exception\ClientException; use OCA\Mail\Exception\CouldNotConnectException; use OCA\Mail\Tests\Integration\TestCase; @@ -130,7 +131,7 @@ class SieveControllerTest extends TestCase { } public function testGetActiveScriptNoSieve(): void { - $this->expectException(CouldNotConnectException::class); + $this->expectException(ClientException::class); $this->expectExceptionMessage('ManageSieve is disabled'); $mailAccount = new MailAccount(); |