Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[Carte] Corrige la prise en compte des heures dans les filtres de période d'application #1152

Merged
merged 1 commit into from
Jan 27, 2025

Conversation

florimondmanca
Copy link
Collaborator

Cette PR corrige un bug observé à Mayenne : des restrictions ne portant que sur 1 journée avec des heures précises (le 12/01/2025 de 8h à 17h) n'étaient jamais affichées quels que soient les filtres de date de début / fin

Raisons

  1. on utilisait ::date partout donc les heures étaient ignorées
  2. on faisait (startDate inclus, endDate inclus) mais cela exclut en fait le dernier jour de la période sélectionnée, car endDate a une heure qui vaut 00h00 et pas 23h59. On fait donc (startDate inclus, endDate + 1 jour exclus)

Il restera à déterminer pourquoi la carte affiche toutes les données de temps à autre, en semblant ignorer certains filtres (j'ai pu le voir en local mais difficile à reproduire)

@codecov-commenter
Copy link

codecov-commenter commented Jan 20, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.68%. Comparing base (4d86009) to head (4fe5c55).

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #1152   +/-   ##
=========================================
  Coverage     98.68%   98.68%           
  Complexity     1864     1864           
=========================================
  Files           372      372           
  Lines          8054     8058    +4     
=========================================
+ Hits           7948     7952    +4     
  Misses          106      106           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@florimondmanca florimondmanca marked this pull request as ready for review January 20, 2025 11:09
@florimondmanca florimondmanca requested review from mmarchois and Lealefoulon and removed request for mmarchois January 20, 2025 11:09
@florimondmanca florimondmanca force-pushed the fix/map-date-bug branch 3 times, most recently from 9983944 to 3b64d9e Compare January 20, 2025 11:48
@@ -48,7 +48,7 @@ public function testGetWithComplexVehicles(): void

$this->assertSame('Circulation interdite', $measure1Header->filter('h3')->text());
$this->assertSame('pour les véhicules de plus de 3,5 tonnes, 12 mètres de long ou 2,4 mètres de haut, matières dangereuses, Crit\'Air 4 et Crit\'Air 5, sauf piétons, véhicules d\'urgence et convois exceptionnels', $measure1Content->filter('li')->eq(0)->text());
$this->assertSame('du 10/03/2023 à 00h00 au 20/03/2023 à 23h59', $measure1Content->filter('li')->eq(1)->text());
$this->assertSame('du 10/03/2023 à 00h00 au 20/03/2023 à 23h59du 28/03/2023 à 08h00 au 28/03/2023 à 22h00', $measure1Content->filter('li')->eq(1)->text());
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

23h59du

Il y a un bug dans l'affichage, mais pas très trivial, je vais le corriger dans une PR séparée pour ne pas polluer celle-ci,

->innerjoin('m.periods', 'p')
->leftJoin('m.vehicleSet', 'v')
->leftJoin('p.dailyRange', 'd')
->leftJoin('p.timeSlots', 't')
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

J'ai corrigé les jointures explicites qui manquaient car du coup Doctrine allait faire des requêtes supplémentaires lors de le rendu du template Twig de la popup de localisation

Et surtout, pour une certaine raison ça faisait un flaky test : les périodes à afficher n'étaient pas toujours chargées dans le même ordre

@florimondmanca florimondmanca force-pushed the fix/map-date-bug branch 4 times, most recently from 4c5eaff to 468c669 Compare January 23, 2025 09:57
@florimondmanca
Copy link
Collaborator Author

florimondmanca commented Jan 23, 2025

🤔

Error: This request has been automatically failed because it uses a deprecated version of actions/upload-artifact: v3. Learn more: https://github.blog/changelog/2024-04-16-deprecation-notice-v3-of-the-artifact-actions/

C'est lié à l'upload du logfile qu'on fait dans les CI d'import

Il faut passer à la v4

https://github.blog/changelog/2024-04-16-deprecation-notice-v3-of-the-artifact-actions/

@@ -24,7 +24,7 @@ public function testGet(): void
$this->assertSame('Circulation interdite', $li->eq(0)->text());
$this->assertSame('Avenue de Fonneuve du n° 695 au n° 253 à Montauban (82000)', $li->eq(1)->text());
$this->assertSame('Pour les véhicules de plus de 3,5 tonnes, 12 mètres de long ou 2,4 mètres de haut, matières dangereuses, Crit\'Air 4 et Crit\'Air 5, sauf piétons, véhicules d\'urgence et convois exceptionnels', $li->eq(2)->text());
$this->assertSame('Du 10/03/2023 à 00h00 au 20/03/2023 à 23h59', $li->eq(3)->text());
$this->assertSame('Du 10/03/2023 à 00h00 au 20/03/2023 à 23h59du 28/03/2023 à 08h00 au 28/03/2023 à 22h00', $li->eq(3)->text());
Copy link
Collaborator

Choose a reason for hiding this comment

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

c'est du détail : peut être qu'il faudrait rajouter un espace et une "," ou un "et" entre les deux dates ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

j'ai vu qu'après mais ça semble être le même bug que tu as mentionné dans GetMeasureControllerTest

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oui tout à fait je pense que c'est le même bug 👍 je vais créer un ticket pour le garder dans le backlog, mais je ne vais pas le traiter dans cette PR-ci

@florimondmanca florimondmanca merged commit 27a4c98 into main Jan 27, 2025
5 checks passed
@florimondmanca florimondmanca deleted the fix/map-date-bug branch January 27, 2025 15:04
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants