diff --git a/src/Controller/ProfileController.php b/src/Controller/ProfileController.php index e6f35431..e603652f 100644 --- a/src/Controller/ProfileController.php +++ b/src/Controller/ProfileController.php @@ -21,7 +21,6 @@ use App\Form\UserTeamsType; use App\Repository\TeamRepository; use App\Repository\TimesheetRepository; use App\Utils\LocaleSettings; -use Doctrine\Common\Collections\ArrayCollection; use Sensio\Bundle\FrameworkExtraBundle\Configuration\Security; use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Symfony\Component\Form\FormInterface; @@ -212,13 +211,6 @@ class ProfileController extends AbstractController $event = new PrepareUserEvent($profile); $this->dispatcher->dispatch($event); - /** @var \ArrayIterator $iterator */ - $iterator = $profile->getPreferences()->getIterator(); - $iterator->uasort(function (UserPreference $a, UserPreference $b) { - return ($a->getOrder() < $b->getOrder()) ? -1 : 1; - }); - $profile->setPreferences(new ArrayCollection(iterator_to_array($iterator))); - $original = []; foreach ($profile->getPreferences() as $preference) { $original[$preference->getName()] = $preference; @@ -227,48 +219,59 @@ class ProfileController extends AbstractController $form = $this->createPreferencesForm($profile); $form->handleRequest($request); - if ($form->isSubmitted() && $form->isValid()) { - $entityManager = $this->getDoctrine()->getManager(); - $preferences = $profile->getPreferences(); + if ($form->isSubmitted()) { + if ($form->isValid()) { + $entityManager = $this->getDoctrine()->getManager(); + $preferences = $profile->getPreferences(); - // do not allow to add unknown preferences - foreach ($preferences as $preference) { - if (!isset($original[$preference->getName()])) { - $preferences->removeElement($preference); + // do not allow to add unknown preferences + foreach ($preferences as $preference) { + if (!isset($original[$preference->getName()])) { + $preferences->removeElement($preference); + } } - } - // but allow to delete already saved settings - foreach ($original as $name => $preference) { - if (false === $profile->getPreferences()->contains($preference)) { - $entityManager->remove($preference); + // but allow to delete already saved settings + foreach ($original as $name => $preference) { + if (false === $profile->getPreferences()->contains($preference)) { + $entityManager->remove($preference); + } } + + $profile->setPreferences($preferences); + $entityManager->persist($profile); + $entityManager->flush(); + + $this->flashSuccess('action.update.success'); + + // switch locale ONLY if updated profile is the current user + $locale = $request->getLocale(); + if ($this->getUser()->getId() === $profile->getId()) { + $locale = $profile->getPreferenceValue('language', $locale); + } + + return $this->redirectToRoute('user_profile_preferences', [ + '_locale' => $locale, + 'username' => $profile->getUsername() + ]); + } else { + $this->flashError('action.update.error', ['%reason%' => 'Validation failed']); } - - $profile->setPreferences($preferences); - $entityManager->persist($profile); - $entityManager->flush(); - - $this->flashSuccess('action.update.success'); - - // switch locale ONLY if updated profile is the current user - $locale = $request->getLocale(); - if ($this->getUser()->getId() === $profile->getId()) { - $locale = $profile->getPreferenceValue('language', $locale); - } - - return $this->redirectToRoute('user_profile_preferences', [ - '_locale' => $locale, - 'username' => $profile->getUsername() - ]); } + // prepare ordered preferences $sections = []; + /** @var \ArrayIterator $iterator */ + $iterator = $profile->getPreferences()->getIterator(); + $iterator->uasort(function (UserPreference $a, UserPreference $b) { + return ($a->getOrder() < $b->getOrder()) ? -1 : 1; + }); + /** @var UserPreference $pref */ - foreach ($profile->getPreferences() as $pref) { + foreach ($iterator as $pref) { if ($pref->isEnabled()) { - $sections[$pref->getSection()] = $pref->getSection(); + $sections[$pref->getSection()][] = $pref->getName(); } } diff --git a/src/Entity/User.php b/src/Entity/User.php index bbb5211e..7de5fc9e 100644 --- a/src/Entity/User.php +++ b/src/Entity/User.php @@ -336,7 +336,7 @@ class User extends BaseUser implements UserInterface } foreach ($this->preferences as $preference) { - if ($preference->getName() == $name) { + if ($preference->getName() === $name) { return $preference; } } diff --git a/src/Event/UserPreferenceEvent.php b/src/Event/UserPreferenceEvent.php index 0512faf0..45eabbe3 100644 --- a/src/Event/UserPreferenceEvent.php +++ b/src/Event/UserPreferenceEvent.php @@ -14,7 +14,7 @@ use App\Entity\UserPreference; use Symfony\Contracts\EventDispatcher\Event; /** - * This event should be used, if further user preferences should added dynamically + * This event should be used, if further user preferences should be added dynamically. */ final class UserPreferenceEvent extends Event { @@ -26,11 +26,11 @@ final class UserPreferenceEvent extends Event /** * @var User */ - protected $user; + private $user; /** * @var UserPreference[] */ - protected $preferences; + private $preferences = []; /** * @param User $user @@ -44,9 +44,10 @@ final class UserPreferenceEvent extends Event /** * Do not set the preferences directly to the user object, but ONLY via addPreference() + * * @return User */ - public function getUser() + public function getUser(): User { return $this->user; } @@ -54,7 +55,7 @@ final class UserPreferenceEvent extends Event /** * @return UserPreference[] */ - public function getPreferences() + public function getPreferences(): array { return $this->preferences; } diff --git a/src/EventSubscriber/UserProfileSubscriber.php b/src/EventSubscriber/UserProfileSubscriber.php index b675d318..30eff558 100644 --- a/src/EventSubscriber/UserProfileSubscriber.php +++ b/src/EventSubscriber/UserProfileSubscriber.php @@ -17,31 +17,23 @@ use Symfony\Component\HttpKernel\Event\KernelEvent; use Symfony\Component\HttpKernel\KernelEvents; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; -class UserProfileSubscriber implements EventSubscriberInterface +final class UserProfileSubscriber implements EventSubscriberInterface { /** * @var EventDispatcherInterface */ - protected $eventDispatcher; - + private $eventDispatcher; /** * @var TokenStorageInterface */ - protected $storage; + private $storage; - /** - * @param EventDispatcherInterface $dispatcher - * @param TokenStorageInterface $storage - */ public function __construct(EventDispatcherInterface $dispatcher, TokenStorageInterface $storage) { $this->eventDispatcher = $dispatcher; $this->storage = $storage; } - /** - * @return array - */ public static function getSubscribedEvents(): array { return [ @@ -49,10 +41,7 @@ class UserProfileSubscriber implements EventSubscriberInterface ]; } - /** - * @param KernelEvent $event - */ - public function prepareUserProfile(KernelEvent $event) + public function prepareUserProfile(KernelEvent $event): void { if (!$this->canHandleEvent($event)) { return; @@ -65,11 +54,7 @@ class UserProfileSubscriber implements EventSubscriberInterface $this->eventDispatcher->dispatch($event); } - /** - * @param KernelEvent $event - * @return bool - */ - protected function canHandleEvent(KernelEvent $event): bool + private function canHandleEvent(KernelEvent $event): bool { // Ignore sub-requests if (!$event->isMasterRequest()) { diff --git a/src/Form/Type/UserPreferenceType.php b/src/Form/Type/UserPreferenceType.php index e23a730e..8a568374 100644 --- a/src/Form/Type/UserPreferenceType.php +++ b/src/Form/Type/UserPreferenceType.php @@ -34,10 +34,6 @@ class UserPreferenceType extends AbstractType $this->translate = $translator; } - /** - * @param FormBuilderInterface $builder - * @param array $options - */ public function buildForm(FormBuilderInterface $builder, array $options) { $builder->addEventListener( @@ -56,7 +52,7 @@ class UserPreferenceType extends AbstractType } $required = true; - if (CheckboxType::class == $preference->getType()) { + if (CheckboxType::class === $preference->getType()) { $required = false; } diff --git a/templates/user/form.html.twig b/templates/user/form.html.twig index 3a75585c..9eb24948 100644 --- a/templates/user/form.html.twig +++ b/templates/user/form.html.twig @@ -2,11 +2,11 @@ {% block main %} {{ form_start(form) }} - {% for section, counter in sections %} + {% for section, entries in sections %} {% embed '@AdminLTE/Widgets/box-widget.html.twig' %} {% block box_body %} {% for pref in form.children.preferences %} - {% if pref.vars.data.section == section %} + {% if pref.vars.data.name in entries %} {{ form_row(pref) }} {% endif %} {% endfor %} diff --git a/tests/Controller/ProfileControllerTest.php b/tests/Controller/ProfileControllerTest.php index f1b1d3ea..b6e84407 100644 --- a/tests/Controller/ProfileControllerTest.php +++ b/tests/Controller/ProfileControllerTest.php @@ -410,10 +410,14 @@ class ProfileControllerTest extends ControllerBaseTest return [ // assert that the user doesn't have the "hourly-rate_own_profile" permission [User::ROLE_USER, UserFixtures::USERNAME_USER, 82, 82, 'ar', null], - // admins are allowed to update their own hourly rate + // teamleads are allowed to update their own hourly rate, but not other peoples hourly rate + [User::ROLE_TEAMLEAD, UserFixtures::USERNAME_TEAMLEAD, 35, 37.5, 'ar', 19.54], + // admins are allowed to update their own hourly rate, but not other peoples hourly rate [User::ROLE_ADMIN, UserFixtures::USERNAME_ADMIN, 81, 37.5, 'ar', 19.54], - // admins are allowed to update other peoples hourly rate - [User::ROLE_SUPER_ADMIN, UserFixtures::USERNAME_USER, 82, 37.5, 'en', 19.54], + // super-admins are allowed to update other peoples hourly rate + [User::ROLE_SUPER_ADMIN, UserFixtures::USERNAME_ADMIN, 81, 37.5, 'en', 19.54], + // super-admins are allowed to update their own hourly rate + [User::ROLE_SUPER_ADMIN, UserFixtures::USERNAME_SUPER_ADMIN, 46, 37.5, 'ar', 19.54], ]; } @@ -431,21 +435,16 @@ class ProfileControllerTest extends ControllerBaseTest $this->assertEquals($hourlyRateOriginal, $user->getPreferenceValue(UserPreference::HOURLY_RATE)); $this->assertNull($user->getPreferenceValue(UserPreference::INTERNAL_RATE)); $this->assertNull($user->getPreferenceValue(UserPreference::SKIN)); - $this->assertEquals(false, $user->getPreferenceValue('theme.collapsed_sidebar')); - $this->assertEquals('month', $user->getPreferenceValue('calendar.initial_view')); $form = $client->getCrawler()->filter('form[name=user_preferences_form]')->form(); $client->submit($form, [ 'user_preferences_form' => [ 'preferences' => [ - ['name' => UserPreference::HOURLY_RATE, 'value' => 37.5], - ['name' => UserPreference::INTERNAL_RATE, 'value' => 19.54], - ['name' => 'timezone', 'value' => 'America/Creston'], - ['name' => 'language', 'value' => 'ar'], - ['name' => UserPreference::SKIN, 'value' => 'blue'], - ['name' => 'theme.layout', 'value' => 'fixed'], - ['name' => 'theme.collapsed_sidebar', 'value' => true], - ['name' => 'calendar.initial_view', 'value' => 'agendaDay'], + 0 => ['name' => UserPreference::HOURLY_RATE, 'value' => 37.5], + 1 => ['name' => UserPreference::INTERNAL_RATE, 'value' => 19.54], + 2 => ['name' => UserPreference::TIMEZONE, 'value' => 'America/Creston'], + 3 => ['name' => UserPreference::LOCALE, 'value' => 'ar'], + 4 => ['name' => UserPreference::SKIN, 'value' => 'blue'], ] ] ]); @@ -462,10 +461,11 @@ class ProfileControllerTest extends ControllerBaseTest $this->assertEquals($hourlyRate, $user->getPreferenceValue(UserPreference::HOURLY_RATE)); $this->assertEquals($expectedInternalRate, $user->getPreferenceValue(UserPreference::INTERNAL_RATE)); - $this->assertEquals('', $user->getPreferenceValue('America/Creston')); - $this->assertEquals('ar', $user->getPreferenceValue('language')); + $this->assertEquals('America/Creston', $user->getPreferenceValue(UserPreference::TIMEZONE)); + $this->assertEquals('America/Creston', $user->getTimezone()); + $this->assertEquals('ar', $user->getPreferenceValue(UserPreference::LOCALE)); + $this->assertEquals('ar', $user->getLanguage()); + $this->assertEquals('ar', $user->getLocale()); $this->assertEquals('blue', $user->getPreferenceValue(UserPreference::SKIN)); - $this->assertEquals(true, $user->getPreferenceValue('theme.collapsed_sidebar')); - $this->assertEquals('agendaDay', $user->getPreferenceValue('calendar.initial_view')); } }