-
Notifications
You must be signed in to change notification settings - Fork 7
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
Fichier des commentaires #3307
Fichier des commentaires #3307
Conversation
e36c8d4
to
e26ad5c
Compare
0505cd3
to
0811ac1
Compare
@LucienMLD y'a pas de review demandée, mais je me demande quand même si tu voulais pas que je revoies 🤔 |
sisi, ce qui reste c'est des lignes vides sans commentaire parfois mais je ne sais pas comment faire pour l'instant sans y passer beaucoup (trop ?) de temps par rapport aux problèmes |
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.
Je pense que y'a des améliorations possibles à apporter :
- le fait qu'on exporte que la liste des commentaires liés aux besoins qq soit les commentaires qu'on affiche en admin, ça me chiffonne. Pour la lisibilité de l'action et pour l'avenir (faudra se creuser les méninges quand ils voudront l'export d'autres types de commentaires :))
Est-ce qu'il y a moyen de le rendre explicite ? Renommer le bouton Export ? Bloquer l'export si plusieurs types de commentaires ? Avoir des filters (les onglets en haut là) explicites, avec les commentaires besoin en défaut ?
Toujours est-il qu'en l'état, pour une première version, pour moi c'est tout à fait ok
@@ -3,7 +3,7 @@ class Conseiller::CsvExportsController < ApplicationController | |||
|
|||
def index | |||
authorize @user, policy_class: CsvExportPolicy | |||
@exports = current_user.csv_exports.includes(:blob).references(:blob).order('active_storage_blobs.filename DESC') | |||
@exports = current_user.csv_exports.includes(:blob).references(:blob).order('active_storage_blobs.created_at DESC') |
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.
mieux comme ça effectivement
match_closed_at: -> { I18n.l(closed_at, format: :admin) if closed_at.present? }, | ||
expert_antenne: :expert_antenne, | ||
expert_institution: :expert_institution, | ||
comments: -> { "- #{expert.feedbacks.where(user: expert.users, feedbackable: need).order(:created_at).pluck(:description).join("\n- ")}" }, |
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.
me demande si ça vaudrait pas le coup de mettre aussi la date du commentaire
(par exemple, dans l'extrait que j'ai téléchargé, y'avait un vieux vieux besoin, je me demandais ce qu'il faisait là et si c'était un bug. J'ai été vérifié sur la fiche et effectivement, un dernier commentaire a été fait le 16 janvier. Connaissant les collègues, ça m'étonnerait pas qu'eux aussi se questionnent)
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.
J'ai ajouté la date des commentaires c'est vrai que c'est mieux je pense
Oui il y a des choses à améliorer, le souci c'est que ça utilise un exporter qui est très générique, le bouton export c'est le même pour tous les models. Si on bloque l'expoert ça va faire échouer le job c'est tout. Il faudrait trouver une solution. Peut-être mettre des scopes par type avec le scope "Besoins" sélectionné par défaut ? |
81fcced
to
6fa4e8f
Compare
@@ -45,5 +45,13 @@ def sort_relation(relation) | |||
def filename | |||
"commentaires-#{Time.zone.now.iso8601}" | |||
end | |||
|
|||
def self.display_comments(expert, need) |
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.
Pouruqoi tu en fais une méthode de classe plutôt qu'une méthode d'instance ?
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.
si je mets juste display_comments
ça crois que c'est une méthode pour le Match
en cours
6fa4e8f
to
a76450b
Compare
closes #3282