Skip to content
This repository was archived by the owner on Jul 13, 2021. It is now read-only.

Commit d1a7ca0

Browse files
ptsavdarptsavdar
authored
[CoreBundle] 8.x fix administration code issue (#1790)
* Added option is_user_admin_code_unique and fixed override issue * Added administration code constraint * Administration code constraint in class, not property * Cleaned userscontroller for csv import * Added admin code constraint to CsvUserValidator
1 parent d380038 commit d1a7ca0

File tree

15 files changed

+276
-86
lines changed

15 files changed

+276
-86
lines changed

‎main/core/Controller/Administration/UsersController.php

Lines changed: 26 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -283,17 +283,24 @@ public function userWorkspaceListAction(User $user, $page, $max)
283283
* @EXT\Method("POST")
284284
* @EXT\Template("ClarolineCoreBundle:Administration/Users:importForm.html.twig")
285285
*
286+
* @param Request $request
287+
*
286288
* @return Response
289+
*
290+
* @throws \Claroline\CoreBundle\Manager\Exception\AddRoleException
287291
*/
288-
public function importAction()
292+
public function importAction(Request $request)
289293
{
290294
$form = $this->formFactory->create(new ImportUserType(true));
291-
$form->handleRequest($this->request);
295+
$form->handleRequest($request);
292296
$mode = $form->get('mode')->getData();
297+
$options = ['ignore-update' => true];
293298

294299
if ($mode === 'update') {
295300
$form = $this->formFactory->create(new ImportUserType(true, 1));
296301
$form->handleRequest($this->request);
302+
$options['ignore-update'] = false;
303+
} else {
297304
}
298305

299306
if ($form->isValid()) {
@@ -304,31 +311,10 @@ public function importAction()
304311
$data = $this->container->get('claroline.utilities.misc')->formatCsvOutput($data);
305312
$lines = str_getcsv($data, PHP_EOL);
306313
$users = [];
307-
$toUpdate = [];
308314
$sessionFlashBag = $this->session->getFlashBag();
309315

310316
foreach ($lines as $line) {
311-
if (trim($line) !== '') {
312-
if ($mode === 'update') {
313-
$datas = str_getcsv($line, ';');
314-
$username = $datas[2];
315-
$email = $datas[4];
316-
$code = $datas[5];
317-
$existingUser = $this->userManager->getUserByUsernameOrMailOrCode(
318-
$username,
319-
$email,
320-
$code
321-
);
322-
323-
if (is_null($existingUser)) {
324-
$users[] = $datas;
325-
} else {
326-
$toUpdate[] = $datas;
327-
}
328-
} else {
329-
$users[] = str_getcsv($line, ';');
330-
}
331-
}
317+
$users[] = str_getcsv($line, ';');
332318
}
333319

334320
$roleUser = $this->roleManager->getRoleByName('ROLE_USER');
@@ -353,44 +339,31 @@ public function importAction()
353339
}
354340
}
355341

356-
if (count($toUpdate) > 0) {
357-
$updatedNames = $this->userManager->importUsers(
358-
$toUpdate,
359-
$sendMail,
360-
null,
361-
$additionalRoles,
362-
$enableEmailNotification
363-
);
342+
$logs = $this->userManager->importUsers(
343+
$users,
344+
$sendMail,
345+
null,
346+
$additionalRoles,
347+
$enableEmailNotification,
348+
$options
349+
);
364350

365-
foreach ($updatedNames as $name) {
351+
foreach ($logs as $key => $names) {
352+
$msgClass = 'success';
353+
if ($key === 'skipped') {
354+
$msgClass = 'error';
355+
}
356+
foreach ($names as $name) {
366357
$msg = '<'.$name.'> ';
367358
$msg .= $this->translator->trans(
368-
'has_been_updated',
359+
'has_been_'.$key,
369360
[],
370361
'platform'
371362
);
372-
$sessionFlashBag->add('success', $msg);
363+
$sessionFlashBag->add($msgClass, $msg);
373364
}
374365
}
375366

376-
$createdNames = $this->userManager->importUsers(
377-
$users,
378-
$sendMail,
379-
null,
380-
$additionalRoles,
381-
$enableEmailNotification
382-
);
383-
384-
foreach ($createdNames as $name) {
385-
$msg = '<'.$name.'> ';
386-
$msg .= $this->translator->trans(
387-
'has_been_created',
388-
[],
389-
'platform'
390-
);
391-
$sessionFlashBag->add('success', $msg);
392-
}
393-
394367
return new RedirectResponse($this->router->generate('claro_admin_users_index'));
395368
}
396369

‎main/core/Controller/ProfileController.php

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,6 @@ public function editProfileAction(User $loggedUser, User $user = null)
236236
throw new AccessDeniedException();
237237
}
238238

239-
$userRole = $this->roleManager->getUserRoleByUser($user);
240239
$roles = $this->roleManager->getPlatformRoles($user);
241240
$accesses = $this->profilePropertyManager->getAccessesForCurrentUser();
242241

@@ -278,7 +277,6 @@ public function editProfileAction(User $loggedUser, User $user = null)
278277

279278
$user = $form->getData();
280279
$this->userManager->rename($user, $user->getUsername());
281-
$this->roleManager->renameUserRole($userRole, $user->getUsername());
282280

283281
$successMessage = $translator->trans('edit_profile_success', [], 'platform');
284282
$errorMessage = $translator->trans('edit_profile_error', [], 'platform');

‎main/core/Entity/User.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
* @DoctrineAssert\UniqueEntity("mail")
4646
* @Assert\Callback(methods={"isPublicUrlValid"})
4747
* @ClaroAssert\Username()
48+
* @ClaroAssert\UserAdministrativeCode()
4849
*/
4950
class User extends AbstractRoleSubject implements Serializable, AdvancedUserInterface, EquatableInterface, OrderableInterface
5051
{

‎main/core/Library/Configuration/PlatformConfiguration.php

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ class PlatformConfiguration
8989
private $loginTargetRoute;
9090
private $enableOpengraph;
9191
private $tmpDir;
92+
private $isUserAdminCodeUnique;
9293
/**
9394
* @param mixed $sessionDbDataCol
9495
*/
@@ -810,4 +811,24 @@ public function getTmpDir()
810811
{
811812
return $this->tmpDir;
812813
}
814+
815+
/**
816+
* @return mixed
817+
*/
818+
public function getIsUserAdminCodeUnique()
819+
{
820+
return $this->isUserAdminCodeUnique;
821+
}
822+
823+
/**
824+
* @param mixed $isUserAdminCodeUnique
825+
*
826+
* @return $this
827+
*/
828+
public function setIsUserAdminCodeUnique($isUserAdminCodeUnique)
829+
{
830+
$this->isUserAdminCodeUnique = $isUserAdminCodeUnique;
831+
832+
return $this;
833+
}
813834
}

‎main/core/Library/Configuration/PlatformConfigurationHandler.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ class PlatformConfigurationHandler
9999
'enable_rich_text_file_import' => false,
100100
'login_target_route' => 'claro_security_login',
101101
'enable_opengraph' => true,
102+
'is_user_admin_code_unique' => true,
102103
];
103104
private $lockedParameters;
104105

@@ -229,6 +230,7 @@ public function getPlatformConfig()
229230
$config->setLoginTargetRoute($this->parameters['login_target_route']);
230231
$config->setEnableOpengraph($this->parameters['enable_opengraph']);
231232
$config->setTmpDir($this->parameters['tmp_dir']);
233+
$config->setIsUserAdminCodeUnique($this->parameters['is_user_admin_code_unique']);
232234

233235
return $config;
234236
}

‎main/core/Manager/UserManager.php

Lines changed: 40 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -373,11 +373,19 @@ public function deleteUser(User $user)
373373
* )
374374
*
375375
* @param array $users
376-
* @param string $authentication an authentication source
377-
* @param bool $mail do the users need to be mailed
378-
* @param \Closure $logger an anonymous function allowing to log actions
376+
* @param bool $sendMail
377+
* @param \Closure $logger an anonymous function allowing to log actions
378+
* @param array $additionalRoles
379+
* @param bool $enableEmailNotifaction
380+
* @param array $options
379381
*
380382
* @return array
383+
*
384+
* @throws AddRoleException
385+
* @throws \Claroline\CoreBundle\Persistence\NoFlushSuiteStartedException
386+
*
387+
* @internal param string $authentication an authentication source
388+
* @internal param bool $mail do the users need to be mailed
381389
*/
382390
public function importUsers(
383391
array $users,
@@ -392,8 +400,16 @@ public function importUsers(
392400
$options['ignore-update'] = false;
393401
}
394402

395-
$returnValues = [];
403+
if (!isset($options['single-validate'])) {
404+
$options['single-validate'] = false;
405+
}
406+
407+
// Return values
408+
$created = [];
409+
$updated = [];
396410
$skipped = [];
411+
// Skipped users table
412+
$skippedUsers = [];
397413
//keep these roles before the clear() will mess everything up. It's not what we want.
398414
$tmpRoles = $additionalRoles;
399415
$additionalRoles = [];
@@ -424,6 +440,7 @@ public function importUsers(
424440
foreach ($users as $user) {
425441
$firstName = $user[0];
426442
$lastName = $user[1];
443+
$fullName = $firstName.' '.$lastName;
427444
$username = $user[2];
428445
$pwd = $user[3];
429446
$email = trim($user[4]);
@@ -484,7 +501,6 @@ public function importUsers(
484501
$organizations = [];
485502
}
486503

487-
$group = $groupName ? $this->groupManager->getGroupByName($groupName) : null;
488504
if ($groupName) {
489505
$group = $this->groupManager->getGroupByNameAndScheduledForInsert($groupName);
490506

@@ -497,20 +513,13 @@ public function importUsers(
497513
$group = null;
498514
}
499515

500-
$userEntity = $this->userRepo->findOneByMail($email);
501-
502-
if (!$userEntity) {
503-
$userEntity = $this->userRepo->findOneByUsername($username);
504-
if (!$userEntity && $code !== null) {
505-
//the code isn't required afaik
506-
$userEntity = $this->userRepo->findOneByAdministrativeCode($code);
507-
}
508-
}
516+
$userEntity = $this->getUserByUsernameOrMailOrCode($username, $email, $code);
509517

510518
if ($userEntity && $options['ignore-update']) {
511519
if ($logger) {
512520
$logger(" Skipping {$userEntity->getUsername()}...");
513521
}
522+
$skipped[] = $fullName;
514523
continue;
515524
}
516525

@@ -539,7 +548,8 @@ public function importUsers(
539548
if ($options['single-validate']) {
540549
$errors = $this->validator->validate($userEntity);
541550
if (count($errors) > 0) {
542-
$skipped[$i] = $userEntity;
551+
$skippedUsers[$i] = $userEntity;
552+
$skipped[] = $fullName;
543553
if ($isNew) {
544554
--$countCreated;
545555
} else {
@@ -572,7 +582,11 @@ public function importUsers(
572582
}
573583

574584
$this->objectManager->persist($userEntity);
575-
$returnValues[] = $firstName.' '.$lastName;
585+
if ($isNew) {
586+
$created[] = $fullName;
587+
} else {
588+
$updated[] = $fullName;
589+
}
576590

577591
if ($group) {
578592
$this->groupManager->addUsersToGroup($group, [$userEntity]);
@@ -616,11 +630,15 @@ public function importUsers(
616630
$logger($countUpdated.' users updated.');
617631
}
618632

619-
foreach ($skipped as $key => $user) {
633+
foreach ($skippedUsers as $key => $user) {
620634
$logger('The user '.$user.' was skipped at line '.$key.' because it failed the validation pass.');
621635
}
622636

623-
return $returnValues;
637+
return [
638+
'created' => $created,
639+
'updated' => $updated,
640+
'skipped' => $skipped,
641+
];
624642
}
625643

626644
/**
@@ -1282,6 +1300,10 @@ public function getUserByUsernameOrMail($username, $mail, $executeQuery = true)
12821300

12831301
public function getUserByUsernameOrMailOrCode($username, $mail, $code)
12841302
{
1303+
if (empty($code) || !$this->platformConfigHandler->getParameter('is_user_admin_code_unique')) {
1304+
return $this->getUserByUsernameOrMail($username, $mail, true);
1305+
}
1306+
12851307
return $this->userRepo->findUserByUsernameOrMailOrCode($username, $mail, $code);
12861308
}
12871309

‎main/core/Repository/UserRepository.php

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,28 +17,56 @@
1717
use Claroline\CoreBundle\Entity\Role;
1818
use Claroline\CoreBundle\Entity\User;
1919
use Claroline\CoreBundle\Entity\Workspace\Workspace;
20+
use Claroline\CoreBundle\Library\Configuration\PlatformConfigurationHandler;
2021
use Doctrine\ORM\EntityRepository;
2122
use Doctrine\ORM\NoResultException;
2223
use Doctrine\ORM\Query;
24+
use JMS\DiExtraBundle\Annotation as DI;
2325
use Symfony\Component\Security\Core\Exception\UnsupportedUserException;
2426
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
2527
use Symfony\Component\Security\Core\User\UserInterface;
2628
use Symfony\Component\Security\Core\User\UserProviderInterface;
2729

30+
/**
31+
* Class UserRepository.
32+
*/
2833
class UserRepository extends EntityRepository implements UserProviderInterface
2934
{
35+
/**
36+
* @var PlatformConfigurationHandler
37+
*/
38+
private $platformConfigHandler;
39+
40+
/**
41+
* @param PlatformConfigurationHandler $platformConfigHandler
42+
*
43+
* @DI\InjectParams({
44+
* "platformConfigHandler" = @DI\Inject("claroline.config.platform_config_handler")
45+
* })
46+
*/
47+
public function setPlatformConfigurationHandler(PlatformConfigurationHandler $platformConfigHandler)
48+
{
49+
$this->platformConfigHandler = $platformConfigHandler;
50+
}
51+
3052
/**
3153
* {@inheritdoc}
3254
*/
3355
public function loadUserByUsername($username)
3456
{
57+
$isUserAdminCodeUnique = $this->platformConfigHandler->getParameter('is_user_admin_code_unique');
58+
3559
$dql = '
3660
SELECT u FROM Claroline\CoreBundle\Entity\User u
3761
WHERE u.username LIKE :username
38-
OR u.mail LIKE :username
39-
OR u.administrativeCode LIKE :username
40-
AND u.isEnabled = true
41-
';
62+
OR u.mail LIKE :username';
63+
64+
if ($isUserAdminCodeUnique) {
65+
$dql .= '
66+
OR u.administrativeCode LIKE :username';
67+
}
68+
$dql .= '
69+
AND u.isEnabled = true';
4270
$query = $this->_em->createQuery($dql);
4371
$query->setParameter('username', $username);
4472

‎main/core/Resources/translations/platform.de.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,7 @@ has_been_emptied: 'a été vidé'
403403
has_been_put_in_category: 'a été placé dans la catégorie'
404404
has_been_registered_in_workspace: 'a été inscrit dans l''espace d''activités'
405405
has_been_registered_to_group: 'a été inscrit dans le groupe'
406+
has_been_skipped: 'n''a pas été créé'
406407
has_been_suscribed_with_role: 'a été inscrit(e) avec le rôle'
407408
has_been_suscribed_with_role_group: 'a été inscrit avec le rôle'
408409
has_been_unregistered_from_group: 'a été désincrit du groupe'

0 commit comments

Comments
 (0)