Repository navigation
Invitations: Paginer la page des collaborateurs d’une structure [GEN-2309] - #6521
Conversation
abac1f0 to
b7ad910
Compare
e1d6874 to
ff6e918
Compare
b7ad910 to
18e2fbb
Compare
|
🥁 La recette jetable est prête ! 👉 Je veux tester cette PR ! |
18e2fbb to
354b8ba
Compare
ff6e918 to
5ea2520
Compare
75930a8 to
b9a7081
Compare
| self.organization = get_current_company_or_404(request) | ||
| if not self.organization.is_active: | ||
| raise PermissionDenied |
There was a problem hiding this comment.
Le boulot est déjà globalement fait par le middleware :
| self.organization = get_current_company_or_404(request) | |
| if not self.organization.is_active: | |
| raise PermissionDenied | |
| self.organization = request.current_organization |
Ce qui devrais même permettre de le monter dans BaseMemberList.
Et il restera plus qu'à ajouter un UserPassesTestMixin() pour s'assurer du type :).
There was a problem hiding this comment.
ajouté dans un commit
|
|
||
| class BaseMemberList(ListView): | ||
| def get_queryset(self): | ||
| memberships = getattr(self.organization, self.membership_related_name) |
There was a problem hiding this comment.
C'est un tout autre sujet mais ça pourrais valoir le coup de normaliser le related_name à l'occasion, ça irais dans la continuité du choix d'"organisation" comme nom commun à toutes les structures.
There was a problem hiding this comment.
C'est vrai qu'on pourrait avoir
company.memberships
organization.memberships
institution.memberships
There was a problem hiding this comment.
Je viens de découvrir qu'on a une property membership qui le fait :)
(mais je vais plutôt aller changer le related name à la place)
There was a problem hiding this comment.
Avec cette généralisation il me semble que context_object_name n'est plus utile maintenant.
There was a problem hiding this comment.
En effet, il fallait juste corriger le organization.is_authorized (que j'ai remplacé par requets.from_authorized_prescriber dans un autre commit)
| </tbody> | ||
| </table> | ||
| </div> | ||
| {% include "includes/pagination.html" with page=page_obj %} |
There was a problem hiding this comment.
Avec cet ajout je me serais attendu à voir un snapshot bougé aussi 🤔.
There was a problem hiding this comment.
La pagination n'est affichée que s'il y a plus d'une page, on n'a pas de snapshot pour ce cas (j'ai préféré juste regarder s'il y avait le lien vers la page 1)
| max_invitation = MAX_ACTIVE_INVITATION - organization.invitations.pending().count() | ||
| if max_invitation < 1: | ||
| messages.error(request, f"Vous ne pouvez avoir plus de {MAX_ACTIVE_INVITATION} invitations.") | ||
| return HttpResponseRedirect(back_url) |
There was a problem hiding this comment.
Plutôt que max_invitation je propose invitation_left pour ne pas donner l'impression d'une valeur absolue et découpler de la constante.
D'ailleurs j'ai l'impression que cette logique pourrais être mise en commun pour tout les types.
There was a problem hiding this comment.
Pour le second point, c'est fait dans la PR suivante ou je refonds les 3 vues
J'aurais pu refondre avant d'ajouter les limites mais je me suis dit que c'était pas trop grave de le faire après, et ça limitait la taille de cette PR.
tonial
left a comment
There was a problem hiding this comment.
J'ai pris en compte tous les commentaires.
J'ai décidé de sortir la partie limitation des invitations dans la PR qui suit (vu que j'y re-ecrit les vues d'invitations).
COmme ça on n'a plus que la pagination ici !
|
|
||
| class BaseMemberList(ListView): | ||
| def get_queryset(self): | ||
| memberships = getattr(self.organization, self.membership_related_name) |
There was a problem hiding this comment.
C'est vrai qu'on pourrait avoir
company.memberships
organization.memberships
institution.memberships
| </tbody> | ||
| </table> | ||
| </div> | ||
| {% include "includes/pagination.html" with page=page_obj %} |
There was a problem hiding this comment.
La pagination n'est affichée que s'il y a plus d'une page, on n'a pas de snapshot pour ce cas (j'ai préféré juste regarder s'il y avait le lien vers la page 1)
There was a problem hiding this comment.
En effet, il fallait juste corriger le organization.is_authorized (que j'ai remplacé par requets.from_authorized_prescriber dans un autre commit)
| self.organization = get_current_company_or_404(request) | ||
| if not self.organization.is_active: | ||
| raise PermissionDenied |
There was a problem hiding this comment.
ajouté dans un commit
| max_invitation = MAX_ACTIVE_INVITATION - organization.invitations.pending().count() | ||
| if max_invitation < 1: | ||
| messages.error(request, f"Vous ne pouvez avoir plus de {MAX_ACTIVE_INVITATION} invitations.") | ||
| return HttpResponseRedirect(back_url) |
There was a problem hiding this comment.
Pour le second point, c'est fait dans la PR suivante ou je refonds les 3 vues
J'aurais pu refondre avant d'ajouter les limites mais je me suis dit que c'était pas trop grave de le faire après, et ça limitait la taille de cette PR.
b9a7081 to
57aca5c
Compare
57aca5c to
bbad355
Compare
xavfernandez
left a comment
There was a problem hiding this comment.
Jolie mise en commun 👍
🤔 Pourquoi ?
Pour éviter d'avoir trop de membres et générer des requêtes trop lourdes
précédé de #6523Cette PR est suivie pas une autre pour reprendre les vues
invite_*pour être DRY et limiter les invitations en cours -> #6535🍰 Comment ?
🚨 À vérifier
🏝️ Comment tester ?
💻 Captures d'écran