Merge pull request #21533 from nextcloud/techdebt/noid/dedicated-method-to-get-a-users-language

Techdebt/noid/dedicated method to get a users language
This commit is contained in:
Roeland Jago Douma 2020-07-04 08:56:13 +02:00 committed by GitHub
commit 4d8b4ef3cc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 138 additions and 200 deletions

View File

@ -54,7 +54,7 @@ abstract class AbstractProvider implements INotificationProvider {
protected $logger;
/** @var L10NFactory */
private $l10nFactory;
protected $l10nFactory;
/** @var IL10N[] */
private $l10ns;

View File

@ -35,7 +35,6 @@ use OCP\IConfig;
use OCP\IL10N;
use OCP\ILogger;
use OCP\IURLGenerator;
use OCP\IUser;
use OCP\L10N\IFactory as L10NFactory;
use OCP\Mail\IEMailTemplate;
use OCP\Mail\IMailer;
@ -349,7 +348,7 @@ class EmailProvider extends AbstractProvider {
foreach ($users as $user) {
$emailAddress = $user->getEMailAddress();
if ($emailAddress) {
$lang = $this->getLangForUser($user);
$lang = $this->l10nFactory->getUserLanguage($user);
if ($lang) {
$emailAddresses[$emailAddress] = [
'LANG' => $lang,
@ -363,14 +362,6 @@ class EmailProvider extends AbstractProvider {
return $emailAddresses;
}
/**
* @param IUser $user
* @return string
*/
private function getLangForUser(IUser $user): ?string {
return $this->config->getUserValue($user->getUID(), 'core', 'lang', null);
}
/**
* @param IL10N $l10n
* @param VEvent $vevent

View File

@ -40,27 +40,28 @@ use OCP\L10N\IFactory as L10NFactory;
use OCP\Mail\IEMailTemplate;
use OCP\Mail\IMailer;
use OCP\Mail\IMessage;
use PHPUnit\Framework\MockObject\MockObject;
use Sabre\VObject\Component\VCalendar;
class EmailProviderTest extends AbstractNotificationProviderTest {
public const USER_EMAIL = 'frodo@hobb.it';
/** @var ILogger|\PHPUnit\Framework\MockObject\MockObject */
/** @var ILogger|MockObject */
protected $logger;
/** @var L10NFactory|\PHPUnit\Framework\MockObject\MockObject */
/** @var L10NFactory|MockObject */
protected $l10nFactory;
/** @var IL10N|\PHPUnit\Framework\MockObject\MockObject */
/** @var IL10N|MockObject */
protected $l10n;
/** @var IURLGenerator|\PHPUnit\Framework\MockObject\MockObject */
/** @var IURLGenerator|MockObject */
protected $urlGenerator;
/** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */
/** @var IConfig|MockObject */
protected $config;
/** @var IMailer|\PHPUnit\Framework\MockObject\MockObject */
/** @var IMailer|MockObject */
private $mailer;
protected function setUp(): void {
@ -101,19 +102,6 @@ class EmailProviderTest extends AbstractNotificationProviderTest {
$users = [$user1, $user2, $user3, $user4];
$this->config->expects($this->at(0))
->method('getUserValue')
->with('uid1', 'core', 'lang', null)
->willReturn(null);
$this->config->expects($this->at(1))
->method('getUserValue')
->with('uid2', 'core', 'lang', null)
->willReturn('de');
$this->config->expects($this->at(2))
->method('getUserValue')
->with('uid3', 'core', 'lang', null)
->willReturn('de');
$enL10N = $this->createMock(IL10N::class);
$enL10N->method('t')
->willReturnArgument(0);
@ -126,30 +114,31 @@ class EmailProviderTest extends AbstractNotificationProviderTest {
$deL10N->method('l')
->willReturnArgument(0);
$this->l10nFactory->expects($this->at(0))
$this->l10nFactory
->method('getUserLanguage')
->willReturnMap([
[$user1, 'en'],
[$user2, 'de'],
[$user3, 'de'],
]);
$this->l10nFactory
->method('findLanguage')
->with()
->willReturn('en');
$this->l10nFactory->expects($this->at(1))
$this->l10nFactory
->method('languageExists')
->with('dav', 'en')
->willReturn(true);
->willReturnMap([
['dav', 'en', true],
['dav', 'de', true],
]);
$this->l10nFactory->expects($this->at(2))
$this->l10nFactory
->method('get')
->with('dav', 'en')
->willReturn($enL10N);
$this->l10nFactory->expects($this->at(3))
->method('languageExists')
->with('dav', 'de')
->willReturn(true);
$this->l10nFactory->expects($this->at(4))
->method('get')
->with('dav', 'de')
->willReturn($deL10N);
->willReturnMap([
['dav', 'en', null, $enL10N],
['dav', 'de', null, $deL10N],
]);
$template1 = $this->getTemplateMock();
$message11 = $this->getMessageMock('uid1@example.com', $template1);
@ -223,19 +212,6 @@ class EmailProviderTest extends AbstractNotificationProviderTest {
$users = [$user1, $user2, $user3, $user4];
$this->config->expects($this->at(0))
->method('getUserValue')
->with('uid1', 'core', 'lang', null)
->willReturn(null);
$this->config->expects($this->at(1))
->method('getUserValue')
->with('uid2', 'core', 'lang', null)
->willReturn('de');
$this->config->expects($this->at(2))
->method('getUserValue')
->with('uid3', 'core', 'lang', null)
->willReturn('de');
$enL10N = $this->createMock(IL10N::class);
$enL10N->method('t')
->willReturnArgument(0);
@ -248,30 +224,31 @@ class EmailProviderTest extends AbstractNotificationProviderTest {
$deL10N->method('l')
->willReturnArgument(0);
$this->l10nFactory->expects($this->at(0))
$this->l10nFactory
->method('getUserLanguage')
->willReturnMap([
[$user1, 'en'],
[$user2, 'de'],
[$user3, 'de'],
]);
$this->l10nFactory
->method('findLanguage')
->with()
->willReturn('en');
$this->l10nFactory->expects($this->at(1))
$this->l10nFactory
->method('languageExists')
->with('dav', 'de')
->willReturn(true);
->willReturnMap([
['dav', 'en', true],
['dav', 'de', true],
]);
$this->l10nFactory->expects($this->at(2))
$this->l10nFactory
->method('get')
->with('dav', 'de')
->willReturn($enL10N);
$this->l10nFactory->expects($this->at(3))
->method('languageExists')
->with('dav', 'en')
->willReturn(true);
$this->l10nFactory->expects($this->at(4))
->method('get')
->with('dav', 'en')
->willReturn($deL10N);
->willReturnMap([
['dav', 'en', null, $enL10N],
['dav', 'de', null, $deL10N],
]);
$template1 = $this->getTemplateMock();
$message11 = $this->getMessageMock('foo1@example.org', $template1);

View File

@ -44,6 +44,7 @@ use OCP\IGroupManager;
use OCP\IRequest;
use OCP\IUserManager;
use OCP\IUserSession;
use OCP\L10N\IFactory;
use OCP\User\Backend\ISetDisplayNameBackend;
use OCP\User\Backend\ISetPasswordBackend;
@ -59,23 +60,17 @@ abstract class AUserData extends OCSController {
protected $userSession;
/** @var AccountManager */
protected $accountManager;
/** @var IFactory */
protected $l10nFactory;
/**
* @param string $appName
* @param IRequest $request
* @param IUserManager $userManager
* @param IConfig $config
* @param IGroupManager $groupManager
* @param IUserSession $userSession
* @param AccountManager $accountManager
*/
public function __construct(string $appName,
IRequest $request,
IUserManager $userManager,
IConfig $config,
IGroupManager $groupManager,
IUserSession $userSession,
AccountManager $accountManager) {
AccountManager $accountManager,
IFactory $l10nFactory) {
parent::__construct($appName, $request);
$this->userManager = $userManager;
@ -83,6 +78,7 @@ abstract class AUserData extends OCSController {
$this->groupManager = $groupManager;
$this->userSession = $userSession;
$this->accountManager = $accountManager;
$this->l10nFactory = $l10nFactory;
}
/**
@ -146,7 +142,7 @@ abstract class AUserData extends OCSController {
$data[AccountManager::PROPERTY_WEBSITE] = $userAccount[AccountManager::PROPERTY_WEBSITE]['value'];
$data[AccountManager::PROPERTY_TWITTER] = $userAccount[AccountManager::PROPERTY_TWITTER]['value'];
$data['groups'] = $gids;
$data['language'] = $this->config->getSystemValue('force_language', $this->config->getUserValue($targetUserObject->getUID(), 'core', 'lang'));
$data['language'] = $this->l10nFactory->getUserLanguage($targetUserObject);
$data['locale'] = $this->config->getUserValue($targetUserObject->getUID(), 'core', 'locale');
$backend = $targetUserObject->getBackend();

View File

@ -47,23 +47,13 @@ use OCP\IRequest;
use OCP\IUser;
use OCP\IUserManager;
use OCP\IUserSession;
use OCP\L10N\IFactory;
class GroupsController extends AUserData {
/** @var ILogger */
private $logger;
/**
* @param string $appName
* @param IRequest $request
* @param IUserManager $userManager
* @param IConfig $config
* @param IGroupManager $groupManager
* @param IUserSession $userSession
* @param AccountManager $accountManager
* @param ILogger $logger
* @param UsersController $userController
*/
public function __construct(string $appName,
IRequest $request,
IUserManager $userManager,
@ -71,6 +61,7 @@ class GroupsController extends AUserData {
IGroupManager $groupManager,
IUserSession $userSession,
AccountManager $accountManager,
IFactory $l10nFactory,
ILogger $logger) {
parent::__construct($appName,
$request,
@ -78,7 +69,9 @@ class GroupsController extends AUserData {
$config,
$groupManager,
$userSession,
$accountManager);
$accountManager,
$l10nFactory
);
$this->logger = $logger;
}

View File

@ -67,7 +67,7 @@ class UsersController extends AUserData {
/** @var ILogger */
private $logger;
/** @var IFactory */
private $l10nFactory;
protected $l10nFactory;
/** @var NewUserMailHelper */
private $newUserMailHelper;
/** @var FederatedFileSharingFactory */
@ -77,21 +77,6 @@ class UsersController extends AUserData {
/** @var RemoteWipe */
private $remoteWipe;
/**
* @param string $appName
* @param IRequest $request
* @param IUserManager $userManager
* @param IConfig $config
* @param IAppManager $appManager
* @param IGroupManager $groupManager
* @param IUserSession $userSession
* @param AccountManager $accountManager
* @param ILogger $logger
* @param IFactory $l10nFactory
* @param NewUserMailHelper $newUserMailHelper
* @param FederatedFileSharingFactory $federatedFileSharingFactory
* @param ISecureRandom $secureRandom
*/
public function __construct(string $appName,
IRequest $request,
IUserManager $userManager,
@ -112,7 +97,8 @@ class UsersController extends AUserData {
$config,
$groupManager,
$userSession,
$accountManager);
$accountManager,
$l10nFactory);
$this->appManager = $appManager;
$this->logger = $logger;

View File

@ -41,6 +41,7 @@ use OCP\IRequest;
use OCP\IUser;
use OCP\IUserManager;
use OCP\IUserSession;
use OCP\L10N\IFactory;
use OCP\UserInterface;
class GroupsControllerTest extends \Test\TestCase {
@ -75,6 +76,7 @@ class GroupsControllerTest extends \Test\TestCase {
$this->groupManager = $this->createMock(Manager::class);
$this->userSession = $this->createMock(IUserSession::class);
$this->accountManager = $this->createMock(AccountManager::class);
$this->l10nFactory = $this->createMock(IFactory::class);
$this->logger = $this->createMock(ILogger::class);
$this->subAdminManager = $this->createMock(SubAdmin::class);
@ -92,6 +94,7 @@ class GroupsControllerTest extends \Test\TestCase {
$this->groupManager,
$this->userSession,
$this->accountManager,
$this->l10nFactory,
$this->logger
])
->setMethods(['fillStorageInfo'])

View File

@ -953,16 +953,6 @@ class UsersControllerTest extends TestCase {
->method('getUserValue')
->with('UID', 'core', 'enabled', 'true')
->willReturn('true');
$this->config
->expects($this->at(1))
->method('getUserValue')
->with('UID', 'core', 'lang')
->willReturn('de');
$this->config
->expects($this->once())
->method('getSystemValue')
->with('force_language', 'de')
->willReturn('de');
$this->api
->expects($this->once())
->method('fillStorageInfo')
@ -995,10 +985,15 @@ class UsersControllerTest extends TestCase {
->method('getBackend')
->willReturn($backend);
$targetUser
->expects($this->exactly(6))
->method('getUID')
->willReturn('UID');
$this->l10nFactory
->expects($this->once())
->method('getUserLanguage')
->with($targetUser)
->willReturn('de');
$expected = [
'id' => 'UID',
'enabled' => true,
@ -1078,16 +1073,6 @@ class UsersControllerTest extends TestCase {
->method('getUserValue')
->with('UID', 'core', 'enabled', 'true')
->willReturn('true');
$this->config
->expects($this->once())
->method('getSystemValue')
->with('force_language', 'da')
->willReturn('da');
$this->config
->expects($this->at(1))
->method('getUserValue')
->with('UID', 'core', 'lang')
->willReturn('da');
$this->api
->expects($this->once())
->method('fillStorageInfo')
@ -1120,7 +1105,6 @@ class UsersControllerTest extends TestCase {
->method('getBackend')
->willReturn($backend);
$targetUser
->expects($this->exactly(6))
->method('getUID')
->willReturn('UID');
$this->accountManager->expects($this->any())->method('getUser')
@ -1134,6 +1118,12 @@ class UsersControllerTest extends TestCase {
]
);
$this->l10nFactory
->expects($this->once())
->method('getUserLanguage')
->with($targetUser)
->willReturn('da');
$expected = [
'id' => 'UID',
'enabled' => true,
@ -1255,11 +1245,6 @@ class UsersControllerTest extends TestCase {
->method('fillStorageInfo')
->with('UID')
->willReturn(['DummyValue']);
$this->config
->expects($this->once())
->method('getSystemValue')
->with('force_language', 'ru')
->willReturn('ru');
$backend = $this->createMock(UserInterface::class);
$backend->expects($this->atLeastOnce())
@ -1275,7 +1260,6 @@ class UsersControllerTest extends TestCase {
->method('getEMailAddress')
->willReturn('subadmin@nextcloud.com');
$targetUser
->expects($this->exactly(6))
->method('getUID')
->willReturn('UID');
$targetUser
@ -1294,11 +1278,6 @@ class UsersControllerTest extends TestCase {
->expects($this->once())
->method('getBackend')
->willReturn($backend);
$this->config
->expects($this->at(0))
->method('getUserValue')
->with('UID', 'core', 'lang')
->willReturn('ru');
$this->accountManager->expects($this->any())->method('getUser')
->with($targetUser)
->willReturn(
@ -1310,6 +1289,12 @@ class UsersControllerTest extends TestCase {
]
);
$this->l10nFactory
->expects($this->once())
->method('getUserLanguage')
->with($targetUser)
->willReturn('ru');
$expected = [
'id' => 'UID',
'storageLocation' => '/var/www/newtcloud/data/UID',
@ -2911,8 +2896,7 @@ class UsersControllerTest extends TestCase {
$subAdminManager
->expects($this->once())
->method('createSubAdmin')
->with($targetUser, $targetGroup)
->willReturn(true);
->with($targetUser, $targetGroup);
$this->groupManager
->expects($this->once())
->method('getSubAdmin')
@ -3014,8 +2998,7 @@ class UsersControllerTest extends TestCase {
$subAdminManager
->expects($this->once())
->method('deleteSubAdmin')
->with($targetUser, $targetGroup)
->willReturn(true);
->with($targetUser, $targetGroup);
$this->groupManager
->expects($this->once())
->method('getSubAdmin')

View File

@ -34,7 +34,6 @@ use OCP\Activity\IManager as IActivityManager;
use OCP\IConfig;
use OCP\IGroup;
use OCP\IGroupManager;
use OCP\IL10N;
use OCP\IURLGenerator;
use OCP\IUser;
use OCP\IUserManager;
@ -60,8 +59,6 @@ class Hooks {
protected $config;
/** @var IFactory */
protected $languageFactory;
/** @var IL10N */
protected $l;
public function __construct(IActivityManager $activityManager,
IGroupManager $groupManager,
@ -70,8 +67,7 @@ class Hooks {
IURLGenerator $urlGenerator,
IMailer $mailer,
IConfig $config,
IFactory $languageFactory,
IL10N $l) {
IFactory $languageFactory) {
$this->activityManager = $activityManager;
$this->groupManager = $groupManager;
$this->userManager = $userManager;
@ -80,7 +76,6 @@ class Hooks {
$this->mailer = $mailer;
$this->config = $config;
$this->languageFactory = $languageFactory;
$this->l = $l;
}
/**
@ -103,36 +98,30 @@ class Hooks {
->setAffectedUser($user->getUID());
$instanceUrl = $this->urlGenerator->getAbsoluteURL('/');
$language = $this->languageFactory->getUserLanguage($user);
$l = $this->languageFactory->get('settings', $language);
$actor = $this->userSession->getUser();
if ($actor instanceof IUser) {
if ($actor->getUID() !== $user->getUID()) {
// Admin changed the password through the user panel
$this->l = $this->languageFactory->get(
'settings',
$this->config->getUserValue(
$user->getUID(), 'core', 'lang',
$this->config->getSystemValue('default_language', 'en')
)
);
$text = $this->l->t('%1$s changed your password on %2$s.', [$actor->getDisplayName(), $instanceUrl]);
$text = $l->t('%1$s changed your password on %2$s.', [$actor->getDisplayName(), $instanceUrl]);
$event->setAuthor($actor->getUID())
->setSubject(Provider::PASSWORD_CHANGED_BY, [$actor->getUID()]);
} else {
// User changed their password themselves through settings
$text = $this->l->t('Your password on %s was changed.', [$instanceUrl]);
$text = $l->t('Your password on %s was changed.', [$instanceUrl]);
$event->setAuthor($actor->getUID())
->setSubject(Provider::PASSWORD_CHANGED_SELF);
}
} else {
if (\OC::$CLI) {
// Admin used occ to reset the password
$text = $this->l->t('Your password on %s was reset by an administrator.', [$instanceUrl]);
$text = $l->t('Your password on %s was reset by an administrator.', [$instanceUrl]);
$event->setSubject(Provider::PASSWORD_RESET);
} else {
// User reset their password from Lost page
$text = $this->l->t('Your password on %s was reset.', [$instanceUrl]);
$text = $l->t('Your password on %s was reset.', [$instanceUrl]);
$event->setSubject(Provider::PASSWORD_RESET_SELF);
}
}
@ -146,10 +135,10 @@ class Hooks {
'instanceUrl' => $instanceUrl,
]);
$template->setSubject($this->l->t('Password for %1$s changed on %2$s', [$user->getDisplayName(), $instanceUrl]));
$template->setSubject($l->t('Password for %1$s changed on %2$s', [$user->getDisplayName(), $instanceUrl]));
$template->addHeader();
$template->addHeading($this->l->t('Password changed for %s', [$user->getDisplayName()]), false);
$template->addBodyText($text . ' ' . $this->l->t('If you did not request this, please contact an administrator.'));
$template->addHeading($l->t('Password changed for %s', [$user->getDisplayName()]), false);
$template->addBodyText($text . ' ' . $l->t('If you did not request this, please contact an administrator.'));
$template->addFooter();
@ -180,25 +169,20 @@ class Hooks {
->setAffectedUser($user->getUID());
$instanceUrl = $this->urlGenerator->getAbsoluteURL('/');
$language = $this->languageFactory->getUserLanguage($user);
$l = $this->languageFactory->get('settings', $language);
$actor = $this->userSession->getUser();
if ($actor instanceof IUser) {
$subject = Provider::EMAIL_CHANGED_SELF;
if ($actor->getUID() !== $user->getUID()) {
$this->l = $this->languageFactory->get(
'settings',
$this->config->getUserValue(
$user->getUID(), 'core', 'lang',
$this->config->getSystemValue('default_language', 'en')
)
);
$subject = Provider::EMAIL_CHANGED;
}
$text = $this->l->t('Your email address on %s was changed.', [$instanceUrl]);
$text = $l->t('Your email address on %s was changed.', [$instanceUrl]);
$event->setAuthor($actor->getUID())
->setSubject($subject);
} else {
$text = $this->l->t('Your email address on %s was changed by an administrator.', [$instanceUrl]);
$text = $l->t('Your email address on %s was changed by an administrator.', [$instanceUrl]);
$event->setSubject(Provider::EMAIL_CHANGED);
}
$this->activityManager->publish($event);
@ -212,12 +196,12 @@ class Hooks {
'instanceUrl' => $instanceUrl,
]);
$template->setSubject($this->l->t('Email address for %1$s changed on %2$s', [$user->getDisplayName(), $instanceUrl]));
$template->setSubject($l->t('Email address for %1$s changed on %2$s', [$user->getDisplayName(), $instanceUrl]));
$template->addHeader();
$template->addHeading($this->l->t('Email address changed for %s', [$user->getDisplayName()]), false);
$template->addBodyText($text . ' ' . $this->l->t('If you did not request this, please contact an administrator.'));
$template->addHeading($l->t('Email address changed for %s', [$user->getDisplayName()]), false);
$template->addBodyText($text . ' ' . $l->t('If you did not request this, please contact an administrator.'));
if ($user->getEMailAddress()) {
$template->addBodyText($this->l->t('The new email address is %s', [$user->getEMailAddress()]));
$template->addBodyText($l->t('The new email address is %s', [$user->getEMailAddress()]));
}
$template->addFooter();

View File

@ -99,11 +99,7 @@ class NewUserMailHelper {
*/
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';
}
$lang = $this->l10nFactory->getUserLanguage($user);
$l10n = $this->l10nFactory->get('settings', $lang);
if ($generatePasswordResetToken) {

View File

@ -358,6 +358,29 @@ class Factory implements IFactory {
return new LanguageIterator($user, $this->config);
}
/**
* Return the language to use when sending something to a user
*
* @param IUser|null $user
* @return string
* @since 20.0.0
*/
public function getUserLanguage(IUser $user = null): string {
$language = $this->config->getSystemValue('force_language', false);
if ($language !== false) {
return $language;
}
if ($user instanceof IUser) {
$language = $this->config->getUserValue($user->getUID(), 'core', 'lang', null);
if ($language !== null) {
return $language;
}
}
return $this->config->getSystemValue('default_language', 'en');
}
/**
* @param string $locale
* @return bool

View File

@ -1401,10 +1401,7 @@ class DefaultShareProvider implements IShareProvider {
/** @var IUser $recipient */
$email = $recipient->getEMailAddress();
if ($email) {
$language = $this->config->getSystemValue('force_language', false);
$language = \is_string($language) ? $language : $this->config->getUserValue($recipient->getUID(), 'core', 'lang', null);
$language = $language ?? $this->config->getSystemValue('default_language', 'en');
$language = $this->l10nFactory->getUserLanguage($recipient);
if (!isset($toListByLanguage[$language])) {
$toListByLanguage[$language] = [];
}

View File

@ -805,7 +805,7 @@ class Manager implements IManager {
if ($user !== null) {
$emailAddress = $user->getEMailAddress();
if ($emailAddress !== null && $emailAddress !== '') {
$userLang = $this->config->getUserValue($share->getSharedWith(), 'core', 'lang', null);
$userLang = $this->l10nFactory->getUserLanguage($user);
$l = $this->l10nFactory->get('lib', $userLang);
$this->sendMailNotification(
$l,

View File

@ -126,4 +126,13 @@ interface IFactory {
* @since 14.0.0
*/
public function getLanguageIterator(IUser $user = null): ILanguageIterator;
/**
* Return the language to use when sending something to a user
*
* @param IUser|null $user
* @return string
* @since 20.0.0
*/
public function getUserLanguage(IUser $user = null): string;
}