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

Affiche, appro : option pour dépublier diagnostics non-TDés #3913

Merged
merged 44 commits into from
May 21, 2024

Conversation

hfroot
Copy link
Collaborator

@hfroot hfroot commented May 13, 2024

closes #3888

les parties de cette PR:

  • création d'un nouveau composant DsfrToggle
  • refacto tests endpoints publication pour les isoler en: single published canteen; claim canteen; public canteen preview
  • ajout du champ redacted_appro_years
  • ajout de la logique de badges sur les diagnostics
  • ajout d'une separation de diagnostics sur le modèle cantine entre appro et "service"
  • ajouter les badges pour une cantine sur plusieurs serializers

rappel des règles fonctionnelles existantes:

  • si l'établissement n'a pas les données de l'année n-1 on prend les données les plus recentes qu'on a (n-2, n-3...)
  • le diag de la CC prend précedent du diag du satellite (vois hasCentralDiagnosticForMeasure)

règles fonctionnelles créées par cette PR:

  • n'importe quel établissement peut décider de cacher les données d'appro pas télédéclarées pour n'importe quelle année
  • la décision est fait au niveau de chaque établissement, alors les décisions prises par les satellites et CCs n’impactent pas l'un et l'autre
  • si il cache les données d'appro pour l'année n - 1, les données seront caché : sur l'affiche en ligne, sur la carte dans nos cantines, dans les filtres nos cantines, dans le widget qu'il pourrait intégré sur leur site. Le badge appro sera éteint dans ce cas.
  • si il cache les données d'appro pour l'année n - 1, les données restent visibles sur l'affiche générable en PDF (à voir si c'est le comportement voulu), et la vérsion édition affiche en ligne

les views a tester sur le front:

  • affiche en ligne, modifiable
  • affiche en ligne, public
  • carte cantine publiée
  • widget cantine publiée

les configurations a tester:

  • cantine site
  • satellite avec CC diag ALL
  • satellite avec CC diag APPRO
  • satellite avec leur propre diag et un CC diag
  • cacher n-1
  • cacher n-2
  • cacher tous
  • cacher, puis TD?

Later:
Complete tests of new badges backend logic. Consider removing applicableDiagnosticRules from the frontend

Screenshot 2024-05-13 at 16-10-34 ma cantine
Screenshot 2024-05-13 at 16-10-20 ma cantine

Screenshot 2024-05-13 at 16-10-47 ma cantine

todo:

  • make DSFR toggle component
  • remove added field, admin stuff, tests
  • add unpublished_years array to canteen model
  • define behaviour for satellites with CC appro data
  • define behaviour for satellites with CC all data
  • define behaviour for satellites with their own diag
  • front and backend functionality to support changing redacted appro diagnostic years
  • update frontend affiche page to use new appro_diagnostics
  • test with satellite with one own diag and one CC diag
  • handle published canteen preview (appro badge)
  • remove central kitchen diagnostics from published canteen serializer?
  • check on appro badge filter behaviour

hfroot added 30 commits May 13, 2024 13:38
…iagnostics with appro and service diagnostics
@hfroot hfroot requested a review from alemangui May 17, 2024 15:22
@hfroot hfroot changed the title [DRAFT] Affiche, appro : option pour dépublier diagnostics non-TDés Affiche, appro : option pour dépublier diagnostics non-TDés May 17, 2024
@hfroot hfroot marked this pull request as ready for review May 17, 2024 15:22
api/serializers/canteen.py Outdated Show resolved Hide resolved
api/tests/test_canteens.py Outdated Show resolved Hide resolved
return self.service_diagnostics

@property
def latest_published_year(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

(Pas obligatoire)... Je me demande si on pouvait le rendre plus transparent en utilisant max 🤔

@property
def latest_published_year(self):
    if not self.published_appro_diagnostics and not self.published_service_diagnostics:
        return None

    max_appro_year = self.published_appro_diagnostics.only("year").order_by("-year").first().year if self.published_appro_diagnostics else 0
    max_service_year = self.published_service_diagnostics.only("year").order_by("-year").first().year if self.published_service_diagnostics else 0

    return max(max_appro_year, max_service_year)

if not max_year or year > max_year:
max_year = year
return max_year

Copy link
Collaborator

Choose a reason for hiding this comment

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

À voir après, mais si l'année est différente pour published_appro_diagnostics et latest_published_service_diagnostic, une de ces deux properties retournera None même s'il y a un diagnostic derrière. C'est peut-être voulu ?

Par ex, imaginons que

  • self.published_appro_diagnostics.only("year").order_by("-year").first().year est égal à 2024, et
  • self.published_service_diagnostics.only("year").order_by("-year").first().year est égal à 2021

Donc, latest_published_year sera égal à 2024.

  • latest_published_appro_diagnosticretournera bien un diagnostic, mais
  • latest_published_service_diagnostic retournera None, et non pas le dernier diagnostic service de 2021

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Alors je l'ai fait comme ça surtout pour les badges, pour qu'on montre toujours les badges pour la même année. Ça me paraît le plus simple à comprendre, tu penses quoi ? Peut-être c'est un problème du nom de variable plutôt de comportement aussi.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

En vrai, entre ce commentaire et le commentaire perf, peut-être c'est mieux de faire comme tu proposes en premier temps, vu que le cas où les deux sont en décalage est rare.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Moi ça me va de rester comme tu l'as fait, je voulais juste comprendre si c'était voulu ou non

data/models/diagnostic.py Show resolved Hide resolved
api/views/canteen.py Show resolved Hide resolved
hfroot and others added 3 commits May 20, 2024 17:14
Co-authored-by: Alejandro M Guillén <alejandro@amguillen.dev>
…ualityMeasureResults.vue

Co-authored-by: Alejandro M Guillén <alejandro@amguillen.dev>
@hfroot hfroot requested a review from alemangui May 20, 2024 15:33
@hfroot
Copy link
Collaborator Author

hfroot commented May 21, 2024

en deuxieme temps : p-e au lieu de queryset.exclude(redacted_appro_years__contains=[publication_year]) le exclude pourrait être effectué sur qs_diag comme .exclude(canteen__redacted_appro_years__contains... pour éviter avoir les diagnostics inutils dès le debut. Mais aussi ce query est un peu moche car les cartes pourrait montrer le badge dès une année n-2 etc mais cette logique vérifie que n-1... bof c'est un pb pour une autre PR vu que ce comportement existe déjà

@hfroot hfroot merged commit f19014d into staging May 21, 2024
5 checks passed
@hfroot hfroot deleted the diag-depublish branch May 21, 2024 10:48
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.

Affiche, appro : option de depublier les données non-TDées
2 participants