Project

General

Profile

Développement #66330

Garde partagée, avoir des bornes temporelles

Added by Valentin Deniaud over 2 years ago. Updated over 2 years ago.

Status:
Fermé
Priority:
Normal
Assignee:
Valentin Deniaud
Category:
-
Target version:
-
Start date:
16 June 2022
Due date:
% Done:

0%

Estimated time:
Patch proposed:
Yes
Planning:
No

Description

Au moment du changement de règle il faut pouvoir :
  • Poser une date de fin sur l'agenda en cours
  • Créer un deuxième agenda avec une date de début qui serait la même que la date précédente

Files

0001-agendas-add-date-start-field-to-shared-custody-agend.patch (28.7 KB) 0001-agendas-add-date-start-field-to-shared-custody-agend.patch Valentin Deniaud, 30 June 2022 03:29 PM
0002-api-use-custody-agendas-date-start-66330.patch (19.7 KB) 0002-api-use-custody-agendas-date-start-66330.patch Valentin Deniaud, 30 June 2022 03:29 PM

Associated revisions

Revision bf9463e6 (diff)
Added by Valentin Deniaud over 2 years ago

agendas: add date start field to shared custody agenda (#66330)

Revision ebe1884e (diff)
Added by Valentin Deniaud over 2 years ago

api: use custody agendas date start (#66330)

History

#1

Updated by Valentin Deniaud over 2 years ago

Bon ça ne s'est pas passé tout à fait comme je l'imaginais, notamment en prenant en compte le cas :
  • Agenda de garde pour les enfants A et B
  • Nouveau mode de garde pour A
  • Il faut prendre en compte l'ancien agenda pour B et en créer un nouveau pour A

Il y aurait pu y avoir de grands plans pour rétropédaler et revenir à un agenda de garde par enfant, mais en fait ça donne un patch assez simple de ne se baser que sur la date de début et ne pas avoir de date de fin.

Ça nécessite d'avoir confiance dans la partie wcs pour bien présenter les agendas de garde en cours de transition (car une fois dans chrono rien ne dit que l'agenda ne s'applique plus à partir de telle date (et ce serait compliqué à dire parce que ça ne pourrait concerner que certains enfants, cf l'exemple plus haut)). Comme cette partie wcs n'existe pas encore beaucoup, difficile d'imaginer les problèmes concrets.

Techniquement, pas fan de mes tests, le code est bien écrit pour utiliser partout filter_for_guardian et ça ne sert pas à grand chose de tester les quatre API qui en font usage (ça reviendrait à écrire 20 tests à chaque modif de Agenda.get_open_events). Je serais pour ne garder que test_datetimes_multiple_agendas_shared_custody_date_start et dégager les autres.

Et je devance la relecture, tickets à ouvrir :
  • utiliser les date_min/date_max de l'API pour ne pas récupérer tous les agendas de garde
  • avoir une api pour changer la date de début (en l'état, une erreur de saisie est irrécupérable)
#2

Updated by Lauréline Guérin over 2 years ago

Techniquement, pas fan de mes tests, le code est bien écrit pour utiliser partout filter_for_guardian et ça ne sert pas à grand chose de tester les quatre API qui en font usage (ça reviendrait à écrire 20 tests à chaque modif de Agenda.get_open_events). Je serais pour ne garder que test_datetimes_multiple_agendas_shared_custody_date_start et dégager les autres.

Perso je préfère pouvoir m'appuyer sur la lecture des tests pour vérifier que le code se comporte comme on l'attend :)
Est-ce qu'il serait envisageable de unit-tester filter_for_guardian, et ensuite dans chaque endpoint qui l'utilise, écrire un test qui mocke la méthode en lui faisant renvoyer un truc improbable, et vérifier que le retour du endpoint est bien raccord avec ce truc improbable ?

#3

Updated by Valentin Deniaud over 2 years ago

Lauréline Guerin a écrit :

Est-ce qu'il serait envisageable de unit-tester filter_for_guardian, et ensuite dans chaque endpoint qui l'utilise, écrire un test qui mocke la méthode en lui faisant renvoyer un truc improbable, et vérifier que le retour du endpoint est bien raccord avec ce truc improbable ?

Pourquoi pas mais dans le cas présent je ne suis pas sûr de bien saisir l'idée :
  • Quatre API utilisent filter_for_guardian, il existe déjà 4 tests
  • On ajoute une fonctionnalité à filter_for_guardian
  • On veut avoir confiance que les 4 API bénéficient de la fonctionnalité et on se retrouve à écrire 4 nouveaux tests

Comment utiliser un mock pourrait donner cette confiance ?

#4

Updated by Lauréline Guérin over 2 years ago

Tu disais:

Je serais pour ne garder que test_datetimes_multiple_agendas_shared_custody_date_start et dégager les autres.

Je te propose une alternative :) Dégager tous les tests qui en fait vérifient ce qui se passe dans filter_for_guardian pour les 4 endpoints, les remplacer par un test unitaire (ou plus) de cette méthode + des mocks dans les endpoints pour vérifier que c'est bien appelé.

Mais ce n'est qu'une suggestion :)

#5

Updated by Valentin Deniaud over 2 years ago

Je ne suis pas convaincu, on verra pour la prochaine fois où je râle mais si ça te va restons sur les tests classiques écrits ici ?

#6

Updated by Lauréline Guérin over 2 years ago

  • Status changed from Solution proposée to Solution validée

Oui c'est ok, je pensais que tu râlais que tu n'aimais pas les tests que tu avais écrits et que tu voulais les supprimer.

#7

Updated by Valentin Deniaud over 2 years ago

  • Status changed from Solution validée to Résolu (à déployer)
commit 479efa769115b16ce4566107bda87cf35a23c6fd
Author: Valentin Deniaud <vdeniaud@entrouvert.com>
Date:   Wed Jun 29 17:15:03 2022 +0200

    api: use custody agendas date start (#66330)

commit 742c1b15a5e4d212d18aaf1bcbebc3e08db0216e
Author: Valentin Deniaud <vdeniaud@entrouvert.com>
Date:   Wed Jun 29 17:13:46 2022 +0200

    agendas: add date start field to shared custody agenda (#66330)
#8

Updated by Transition automatique over 2 years ago

  • Status changed from Résolu (à déployer) to Solution déployée
#9

Updated by Transition automatique about 2 years ago

Automatic expiration

Also available in: Atom PDF