diff options
author | zulan <git@zulan.net> | 2018-09-24 22:54:57 +0200 |
---|---|---|
committer | zulan <git@zulan.net> | 2018-10-15 19:01:46 +0200 |
commit | cf266ee00491bc0e70fbbb7dc9720a9e2f8c3708 (patch) | |
tree | 54778b79b3db543bd6d2fad3af02fe51afdb1ce9 | |
parent | 50a280338e22f34e66f03c1c069fffbf32c912b2 (diff) | |
download | nextcloud-server-cf266ee00491bc0e70fbbb7dc9720a9e2f8c3708.tar.gz nextcloud-server-cf266ee00491bc0e70fbbb7dc9720a9e2f8c3708.zip |
Get l10n within NewUserMailHelper to ensure it always uses the new user's language.
Some related tests had to be changed because they relied on internals, see also from the PHPUnit documentation:
"Exercise caution when using [the at] matcher as it can lead to brittle tests which are too closely tied to specific implementation details."
Signed-off-by: Zulan <git@zulan.net>
-rw-r--r-- | apps/provisioning_api/lib/AppInfo/Application.php | 2 | ||||
-rw-r--r-- | apps/provisioning_api/lib/Controller/UsersController.php | 8 | ||||
-rw-r--r-- | apps/provisioning_api/tests/Controller/UsersControllerTest.php | 69 | ||||
-rw-r--r-- | settings/Application.php | 2 | ||||
-rw-r--r-- | settings/Mailer/NewUserMailHelper.php | 45 | ||||
-rw-r--r-- | tests/Settings/Mailer/NewUserMailHelperTest.php | 30 |
6 files changed, 41 insertions, 115 deletions
diff --git a/apps/provisioning_api/lib/AppInfo/Application.php b/apps/provisioning_api/lib/AppInfo/Application.php index b7a93d6fb13..7e9e04978fe 100644 --- a/apps/provisioning_api/lib/AppInfo/Application.php +++ b/apps/provisioning_api/lib/AppInfo/Application.php @@ -43,7 +43,7 @@ class Application extends App { return new NewUserMailHelper( $server->query(Defaults::class), $server->getURLGenerator(), - $server->getL10N('settings'), + $server->getL10NFactory(), $server->getMailer(), $server->getSecureRandom(), new TimeFactory(), diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 9aa32e1186a..27d66a83c81 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -894,16 +894,8 @@ class UsersController extends AUserData { if ($email === '' || $email === null) { throw new OCSException('Email address not available', 101); } - $username = $targetUser->getUID(); - $lang = $this->config->getUserValue($username, 'core', 'lang', 'en'); - if (!$this->l10nFactory->languageExists('settings', $lang)) { - $lang = 'en'; - } - - $l10n = $this->l10nFactory->get('settings', $lang); try { - $this->newUserMailHelper->setL10N($l10n); $emailTemplate = $this->newUserMailHelper->generateTemplate($targetUser, false); $this->newUserMailHelper->sendMail($targetUser, $emailTemplate); } catch(\Exception $e) { diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php index adbdb9d8685..57283e11912 100644 --- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php +++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php @@ -3257,35 +3257,13 @@ class UsersControllerTest extends TestCase { ->expects($this->once()) ->method('getEmailAddress') ->will($this->returnValue('abc@example.org')); - $this->config - ->expects($this->at(0)) - ->method('getUserValue') - ->with('user-id', 'core', 'lang') - ->willReturn('es'); - $l10n = $this->getMockBuilder(IL10N::class) - ->disableOriginalConstructor() - ->getMock(); - $this->l10nFactory - ->expects($this->at(0)) - ->method('languageExists') - ->with('settings', 'es') - ->willReturn(true); - $this->l10nFactory - ->expects($this->at(1)) - ->method('get') - ->with('settings', 'es') - ->willReturn($l10n); $emailTemplate = $this->createMock(IEMailTemplate::class); $this->newUserMailHelper ->expects($this->at(0)) - ->method('setL10N') - ->willReturn($l10n); - $this->newUserMailHelper - ->expects($this->at(1)) ->method('generateTemplate') ->willReturn($emailTemplate); $this->newUserMailHelper - ->expects($this->at(2)) + ->expects($this->at(1)) ->method('sendMail') ->with($targetUser, $emailTemplate); @@ -3327,35 +3305,16 @@ class UsersControllerTest extends TestCase { ->expects($this->once()) ->method('getEmailAddress') ->will($this->returnValue('abc@example.org')); - $this->config - ->expects($this->at(0)) - ->method('getUserValue') - ->with('user-id', 'core', 'lang') - ->willReturn('es'); $l10n = $this->getMockBuilder(IL10N::class) ->disableOriginalConstructor() ->getMock(); - $this->l10nFactory - ->expects($this->at(0)) - ->method('languageExists') - ->with('settings', 'es') - ->willReturn(false); - $this->l10nFactory - ->expects($this->at(1)) - ->method('get') - ->with('settings', 'en') - ->willReturn($l10n); $emailTemplate = $this->createMock(IEMailTemplate::class); $this->newUserMailHelper ->expects($this->at(0)) - ->method('setL10N') - ->willReturn($l10n); - $this->newUserMailHelper - ->expects($this->at(1)) ->method('generateTemplate') ->willReturn($emailTemplate); $this->newUserMailHelper - ->expects($this->at(2)) + ->expects($this->at(1)) ->method('sendMail') ->with($targetUser, $emailTemplate); @@ -3402,35 +3361,13 @@ class UsersControllerTest extends TestCase { ->expects($this->once()) ->method('getEmailAddress') ->will($this->returnValue('abc@example.org')); - $this->config - ->expects($this->at(0)) - ->method('getUserValue') - ->with('user-id', 'core', 'lang') - ->willReturn('es'); - $l10n = $this->getMockBuilder(IL10N::class) - ->disableOriginalConstructor() - ->getMock(); - $this->l10nFactory - ->expects($this->at(0)) - ->method('languageExists') - ->with('settings', 'es') - ->willReturn(true); - $this->l10nFactory - ->expects($this->at(1)) - ->method('get') - ->with('settings', 'es') - ->willReturn($l10n); $emailTemplate = $this->createMock(IEMailTemplate::class); $this->newUserMailHelper ->expects($this->at(0)) - ->method('setL10N') - ->willReturn($l10n); - $this->newUserMailHelper - ->expects($this->at(1)) ->method('generateTemplate') ->willReturn($emailTemplate); $this->newUserMailHelper - ->expects($this->at(2)) + ->expects($this->at(1)) ->method('sendMail') ->with($targetUser, $emailTemplate) ->willThrowException(new \Exception()); diff --git a/settings/Application.php b/settings/Application.php index 5b2b606353f..4ad59a64d40 100644 --- a/settings/Application.php +++ b/settings/Application.php @@ -100,7 +100,7 @@ class Application extends App { return new NewUserMailHelper( $defaults, $server->getURLGenerator(), - $server->getL10N('settings'), + $server->getL10NFactory(), $server->getMailer(), $server->getSecureRandom(), new TimeFactory(), diff --git a/settings/Mailer/NewUserMailHelper.php b/settings/Mailer/NewUserMailHelper.php index bcd8ae0a538..d88f7300f84 100644 --- a/settings/Mailer/NewUserMailHelper.php +++ b/settings/Mailer/NewUserMailHelper.php @@ -26,6 +26,7 @@ namespace OC\Settings\Mailer; +use OCP\L10N\IFactory; use OCP\Mail\IEMailTemplate; use OCP\AppFramework\Utility\ITimeFactory; use OCP\Defaults; @@ -42,8 +43,8 @@ class NewUserMailHelper { private $themingDefaults; /** @var IURLGenerator */ private $urlGenerator; - /** @var IL10N */ - private $l10n; + /** @var IFactory */ + private $l10nFactory; /** @var IMailer */ private $mailer; /** @var ISecureRandom */ @@ -60,7 +61,7 @@ class NewUserMailHelper { /** * @param Defaults $themingDefaults * @param IURLGenerator $urlGenerator - * @param IL10N $l10n + * @param IFactory $l10nFactory * @param IMailer $mailer * @param ISecureRandom $secureRandom * @param ITimeFactory $timeFactory @@ -70,7 +71,7 @@ class NewUserMailHelper { */ public function __construct(Defaults $themingDefaults, IURLGenerator $urlGenerator, - IL10N $l10n, + IFactory $l10nFactory, IMailer $mailer, ISecureRandom $secureRandom, ITimeFactory $timeFactory, @@ -79,7 +80,7 @@ class NewUserMailHelper { $fromAddress) { $this->themingDefaults = $themingDefaults; $this->urlGenerator = $urlGenerator; - $this->l10n = $l10n; + $this->l10nFactory = $l10nFactory; $this->mailer = $mailer; $this->secureRandom = $secureRandom; $this->timeFactory = $timeFactory; @@ -89,20 +90,19 @@ class NewUserMailHelper { } /** - * Set the IL10N object - * - * @param IL10N $l10n - */ - public function setL10N(IL10N $l10n) { - $this->l10n = $l10n; - } - - /** * @param IUser $user * @param bool $generatePasswordResetToken * @return IEMailTemplate */ public function generateTemplate(IUser $user, $generatePasswordResetToken = false) { + $userId = $user->getUID(); + $lang = $this->config->getUserValue($userId, 'core', 'lang', 'en'); + if (!$this->l10nFactory->languageExists('settings', $lang)) { + $lang = 'en'; + } + + $l10n = $this->l10nFactory->get('settings', $lang); + if ($generatePasswordResetToken) { $token = $this->secureRandom->generate( 21, @@ -119,7 +119,6 @@ class NewUserMailHelper { $link = $this->urlGenerator->getAbsoluteURL('/'); } $displayName = $user->getDisplayName(); - $userId = $user->getUID(); $emailTemplate = $this->mailer->createEMailTemplate('settings.Welcome', [ 'link' => $link, @@ -129,24 +128,24 @@ class NewUserMailHelper { 'resetTokenGenerated' => $generatePasswordResetToken, ]); - $emailTemplate->setSubject($this->l10n->t('Your %s account was created', [$this->themingDefaults->getName()])); + $emailTemplate->setSubject($l10n->t('Your %s account was created', [$this->themingDefaults->getName()])); $emailTemplate->addHeader(); if ($displayName === $userId) { - $emailTemplate->addHeading($this->l10n->t('Welcome aboard')); + $emailTemplate->addHeading($l10n->t('Welcome aboard')); } else { - $emailTemplate->addHeading($this->l10n->t('Welcome aboard %s', [$displayName])); + $emailTemplate->addHeading($l10n->t('Welcome aboard %s', [$displayName])); } - $emailTemplate->addBodyText($this->l10n->t('Welcome to your %s account, you can add, protect, and share your data.', [$this->themingDefaults->getName()])); - $emailTemplate->addBodyText($this->l10n->t('Your username is: %s', [$userId])); + $emailTemplate->addBodyText($l10n->t('Welcome to your %s account, you can add, protect, and share your data.', [$this->themingDefaults->getName()])); + $emailTemplate->addBodyText($l10n->t('Your username is: %s', [$userId])); if ($generatePasswordResetToken) { - $leftButtonText = $this->l10n->t('Set your password'); + $leftButtonText = $l10n->t('Set your password'); } else { - $leftButtonText = $this->l10n->t('Go to %s', [$this->themingDefaults->getName()]); + $leftButtonText = $l10n->t('Go to %s', [$this->themingDefaults->getName()]); } $emailTemplate->addBodyButtonGroup( $leftButtonText, $link, - $this->l10n->t('Install Client'), + $l10n->t('Install Client'), 'https://nextcloud.com/install/#install-clients' ); $emailTemplate->addFooter(); diff --git a/tests/Settings/Mailer/NewUserMailHelperTest.php b/tests/Settings/Mailer/NewUserMailHelperTest.php index d59b371acfd..5eccc5bc9f4 100644 --- a/tests/Settings/Mailer/NewUserMailHelperTest.php +++ b/tests/Settings/Mailer/NewUserMailHelperTest.php @@ -22,6 +22,7 @@ namespace Tests\Settings\Mailer; use OC\Mail\EMailTemplate; +use OCP\L10N\IFactory; use OCP\Mail\IEMailTemplate; use OC\Mail\Message; use OC\Settings\Mailer\NewUserMailHelper; @@ -64,6 +65,7 @@ class NewUserMailHelperTest extends TestCase { ->willReturn('myLogo'); $this->urlGenerator = $this->createMock(IURLGenerator::class); $this->l10n = $this->createMock(IL10N::class); + $this->l10nFactory = $this->createMock(IFactory::class); $this->mailer = $this->createMock(IMailer::class); $template = new EMailTemplate( $this->defaults, @@ -82,11 +84,15 @@ class NewUserMailHelperTest extends TestCase { ->will($this->returnCallback(function ($text, $parameters = []) { return vsprintf($text, $parameters); })); + $this->l10nFactory->method('get') + ->will($this->returnCallback(function ($text, $lang) { + return $this->l10n; + })); $this->newUserMailHelper = new NewUserMailHelper( $this->defaults, $this->urlGenerator, - $this->l10n, + $this->l10nFactory, $this->mailer, $this->secureRandom, $this->timeFactory, @@ -113,15 +119,11 @@ class NewUserMailHelperTest extends TestCase { /** @var IUser|\PHPUnit_Framework_MockObject_MockObject $user */ $user = $this->createMock(IUser::class); $user - ->expects($this->at(0)) - ->method('getEmailAddress') - ->willReturn('recipient@example.com'); - $user - ->expects($this->at(1)) + ->expects($this->any()) ->method('getEmailAddress') ->willReturn('recipient@example.com'); $this->config - ->expects($this->at(0)) + ->expects($this->any()) ->method('getSystemValue') ->with('secret') ->willReturn('MyInstanceWideSecret'); @@ -131,24 +133,20 @@ class NewUserMailHelperTest extends TestCase { ->with('12345:MySuperLongSecureRandomToken', 'recipient@example.comMyInstanceWideSecret') ->willReturn('TokenCiphertext'); $user - ->expects($this->at(2)) + ->expects($this->any()) ->method('getUID') ->willReturn('john'); $this->config - ->expects($this->at(1)) + ->expects($this->once()) ->method('setUserValue') ->with('john', 'core', 'lostpassword', 'TokenCiphertext'); - $user - ->expects($this->at(3)) - ->method('getUID') - ->willReturn('john'); $this->urlGenerator ->expects($this->at(0)) ->method('linkToRouteAbsolute') ->with('core.lost.resetform', ['userId' => 'john', 'token' => 'MySuperLongSecureRandomToken']) ->willReturn('https://example.com/resetPassword/MySuperLongSecureRandomToken'); $user - ->expects($this->at(4)) + ->expects($this->any()) ->method('getDisplayName') ->willReturn('john'); $user @@ -385,11 +383,11 @@ EOF; /** @var IUser|\PHPUnit_Framework_MockObject_MockObject $user */ $user = $this->createMock(IUser::class); $user - ->expects($this->at(0)) + ->expects($this->any()) ->method('getDisplayName') ->willReturn('John Doe'); $user - ->expects($this->at(1)) + ->expects($this->any()) ->method('getUID') ->willReturn('john'); $this->defaults |