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

Conversations : améliorer l'affichage admin #866

Merged
merged 5 commits into from
Aug 22, 2023

Conversation

raphodn
Copy link
Contributor

@raphodn raphodn commented Aug 21, 2023

Quoi ?

  • mettre le uuid à gauche du titre
  • afficher le nombre de réponses
  • chercher par sender_email
  • filtre par répondu ou pas
  • queryset, tests

Captures d'écran

Image
Avant image
Après image

Copy link
Contributor

@madjid-asa madjid-asa left a comment

Choose a reason for hiding this comment

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

petite suggestion de typo sinon LGTM

from django.utils import timezone
from django_extensions.db.fields import ShortUUIDField
from shortuuid import uuid


class ConversationQuerySet(models.QuerySet):
pass
def has_answer(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
def has_answer(self):
def with_answers(self):

Je te suggère de modifier le nom ou de modifier la nature de la méthode, pcq avec has_answer je m'attend à un retour True ou False

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes ca fait sens !
on a eu tendance à utiliser has_* dans les queryset, faudrait changer ca dans une autre PR.
le seul hic avec ca, c'est qu'on a aussi des with_ qui servent à faire des annotation (au lieu de filtrer), faudrait leur trouver un meilleur naming ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

bon en fait j'hésite pour le with, ca me fait penser qu'on fait un prefetch et qu'on rajoute les answers en "plus"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

je vais merger, j'ai lancé la discussion sur nos canaux, pour trouver un meilleur naming global 👍

@raphodn raphodn merged commit 5e7fd5b into master Aug 22, 2023
2 checks passed
@raphodn raphodn deleted the raphodn/conversations-admin-answer-count branch August 22, 2023 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants