Skip to content
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

Ajout de la route pour les responsables de groupe #30

Merged
merged 4 commits into from
Mar 12, 2018

Conversation

prytoegrian
Copy link
Member

On avance, petit à petit. Cette PR, en plus de faire son job, pose les bases pour l'ajout quasi instantanée de groupe_haut_responsable et groupe_employe.

Comme la table derrière cette route est une relation N-N, il n'y aucun intérêt de rechercher par id d'entité, il n'y a donc pas de route GET /groupe/88/responsable/1, seule la liste est dispo. Parlant des entités, par essence de la relation N-N, il ne peut y avoir d'entité représentante. Puisque pour l'instant, l'application a été construite appuyée sur les entités, j'ai pris le parti de brancher le composant groupe/responsable sur l'entité utilisateur. Ce n'est pas idéal, mais il serait stupide de faire de profondes refacto à chaque fois qu'un nouvel événement arrive ; il vaut mieux patienter, faire remonter les cas et le besoin, et ainsi prendre une décision la plus documentée possible. C'est ce que je compte faire.

J'en ai profité pour faire une petite refacto, en supprimant les foreach pour une vision plus « programmation fonctionnelle » 😋

Le test

Comme nous l'avions évoqué, l'API est presqu'exclusivement en lecture seule, aussi il n'y a que l'ordre GET, sur la route énoncée plus haut. Gare à la gestion des droits !

protected function ensureAccessUser($order, UtilisateurEntite $utilisateur)
{
unset($order);
if (!$utilisateur->isAdmin()) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

N'est plus vrai depuis libertempo/web#542

Copy link
Member

@wouldsmina wouldsmina Mar 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

si si! c'est le resp direct qui saute dans 542. Par contre, la suite logique que je prévois (sur Akhaten j'espère) :

  • création d'un compte RH à l'installation
  • le compte admin saute
  • les options permettant de paramétrer les droits des utilisateurs avec le droit admin (conges_users.u_is_admin) sautent
  • les utilisateurs ayant les droits admin auront accès à la configuration globale (conf métier, mail, type de congés...) et à la backup de la bdd

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, cool!

@wouldsmina
Copy link
Member

bpm! 🥂

@libertempo libertempo deleted a comment Mar 12, 2018
@libertempo libertempo deleted a comment Mar 12, 2018
@libertempo libertempo deleted a comment Mar 12, 2018
@libertempo libertempo deleted a comment Mar 12, 2018
@libertempo libertempo deleted a comment Mar 12, 2018
@libertempo libertempo deleted a comment Mar 12, 2018
@libertempo libertempo deleted a comment Mar 12, 2018
@libertempo libertempo deleted a comment Mar 12, 2018
@prytoegrian prytoegrian merged commit 593d3b4 into master Mar 12, 2018
@prytoegrian prytoegrian deleted the pry/groupeResponsable branch March 12, 2018 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants