-
Notifications
You must be signed in to change notification settings - Fork 91
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
ETQ super-admin je peux informer les administrateurs, instructeurs et experts des évolutions du site #9638
ETQ super-admin je peux informer les administrateurs, instructeurs et experts des évolutions du site #9638
Conversation
80ba764
to
9cf8c75
Compare
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.
Très chouette cette feature ! C'est simple et intuitif 👌 J'ai quelques petits retours :
- Etq lecteur d'annonce on voit déjà les annonces dont la publication est programmée dans le futur.
- Etq superadmin dans la liste des annonces, si la publication programmée dans le futur, mettre un badge "programmé" plutôt que "publié" ?
- Si plusieurs notes créées le même jour, la dernière s'affiche en bas de liste -> la remonter pour éviter qu'elle ne passe inaperçue ?
- Pour le fait de ne pas pouvoir détruire une note si c'est la seule de l'annonce, j'ai vu ton commentaire à propos de Turbo. Peut-être que dans l'index des annonces on pourrait ajouter un bouton supprimer ? (parce que là on ne pas supprimer d'annonce je crois)
- Dans la liste des annonces, on pourrait ajouter un texte du type : "Vous n'avez pas encore publié d'annonce" à la place de l'intitulé des 4 colonnes la 1e fois qu'on se connecte
9cf8c75
to
a9eda07
Compare
19708bf
to
0eb2a7f
Compare
Je me dis qu'ETQ superadmin on connaitra bien le fonctionnement et qu'on ne publiera pas dans le futur sauf exception (car ça ne fait pas partie de ce qui a été prévu de faire). Je préfère rester minimaliste au moins le temps de voir ce qu'on veut vraiment faire de l'outil, j'ai juste fait en sorte de masquer les annonces du futur au cas où.
Au contraire on conserve le même ordre chronologique dans l'interface et dans l'affichage. Et de la même manière que précédemment, on n'est pas censé avoir plusieurs blocs d'annonces le même jour, ça n'a pas de sens tant qu'on ne type pas les annonces (incidents/informations)
Oui j'ai pensé à ça, mais même remarque : pour tout ce qui touche au super-admin je préfère rester au plus simple pour le moment. On verra si c'est nécessaire, au pire, on peut réutiliser une annonce non publiée et changer la date.
Là aussi même remarque, et c'est comme le manager |
0eb2a7f
to
0e2937e
Compare
25c1334
to
a1fb8da
Compare
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.
Super, merci beaucoup pour cette simplification.
@@ -11,4 +11,7 @@ class ReleaseNote < ApplicationRecord | |||
|
|||
validates :categories, presence: true, inclusion: { in: CATEGORIES } | |||
validates :body, presence: true | |||
|
|||
scope :published, -> { where(published: true, released_on: ..Date.current) } |
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.
super classe la combinaison des deux 😍
@paginated_groups = ReleaseNote.published | ||
.for_categories(@categories) | ||
.select(:released_on) | ||
.group(:released_on) | ||
.order(released_on: :desc) | ||
.page(params[:page]).per(5) |
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.
@paginated_groups = ReleaseNote.published | |
.for_categories(@categories) | |
.select(:released_on) | |
.group(:released_on) | |
.order(released_on: :desc) | |
.page(params[:page]).per(5) | |
@paginated_release_dates = ReleaseNote.published | |
.for_categories(@categories) | |
.order(released_on: :desc) | |
.page(params[:page]).per(5) | |
.pluck(:released_on) |
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.
ce n'est pas équivalent: on a besoin d'une object paginé pour gérer la pagination avec les next_page etc… dans app/views/release_notes/_page.html.haml
Ou alors tu avais autre chose en tête ?
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 bien vu. En fait mon point était de faire sauter le group(:released_on)
, j'ai l'impression qu'il n'est pas nécessaire.
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, je groupe car je veux que toutes les annonces d'une même date soient affichées dans la même "page". (s'il y a 10 annonces le même jour, je veux qu'elles apparaissent en même temps)
a1fb8da
to
1784582
Compare
7a4456e
Première version :
Dans la PR suivante #9655 (car accompagnée d'un gros refacto):
Evolutions envisageable plus tard :
VUE INTERNAUTE
INTERFACE SUPER ADMIN