-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Admin : nouveau paramètre qui permet d'empêcher les membres de (in)valider leur propre créneau #801
Conversation
a7bdb82
to
598bca4
Compare
d02a6fd
to
53e45fb
Compare
$validate = $form->get('validate')->getData() == 1; | ||
$current = $shift->getWasCarriedOut() == 1; | ||
if ($validate == $current) { | ||
$shifter_is_current_user = $current_app_user->getBeneficiary() == $shift->getShifter(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Attention un user n'a pas toujours de beneficiary (ex les salariés). Je ne sais pas si ce cas est problématique.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
T'as raison, je vais tester
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok testé : $current_app_user->getBeneficiary() = null
si le user n'a pas de beneficiary, donc ca passe
if ($validate == $current) { | ||
$shifter_is_current_user = $current_app_user->getBeneficiary() == $shift->getShifter(); | ||
// check if user is allowed to (in)validate shift | ||
if ($shifter_is_current_user && $this->forbid_own_shift_validate_admin && !$this->get('security.authorization_checker')->isGranted('ROLE_ADMIN')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Au debut de la methode tu as un voters :
$this->denyAccessUnlessGranted(ShiftVoter::VALIDATE, $shift);
Ca serait plus logique de mettre cette condition dans le voter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah oui c'est vrai que je n'avais pas considéré le voter pour shift_free, mais ici ca peut être possible à faire..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
en fait j'ai un peu regardé, en passant par ShiftVoter on n'a pas la possibilité de mettre des message d'erreurs custom, si ?
là c'est une règle métier à un niveau supérieur que savoir si t'as l'authorisation d'accéder à cet endpoint, donc je vais laisser comme ça (mais si on trouve un moyen d'avoir des messages d'erreurs custom, on pourra les basculer dans ShiftVoter en effet)
…lider leur propre créneau (elefan-grenoble#801) * If forbid_own_shift_validate_admin, then non-admin cannot (in)validate their own shift via admin * Add security on shift_validate route * Additional fixes
Suite de #796
Quoi ?
forbid_own_shift_validate_admin
Modifications annexes :
shift_validate
enshift_validate_admin
(vu que ce n'est seulement possible via l'admin)Pourquoi ?
Pour restreindre le nombre de membres qui peuvent s'auto-valider leur créneaux