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

Traçabilité des modifications sur un arrêté #1147

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

Lealefoulon
Copy link
Collaborator

@Lealefoulon Lealefoulon commented Jan 14, 2025

Des informations sur "qui a fait quelle action et quand sur un arrêté" seront stockées dans la base de données

Dans un premier temps, par exemple

  • qui a créé ou modifié l'arrêté et quand
  • qui l'a publié et quand
  • qui l'a supprimé et quand
  • Tests

Résultat attendu

Ces données permettront de réaliser des investigations si jamais quelque chose se passe mal, ou si une collectivité nous pose une question sur un comportement étrange qu'elle a observé, etc

C'est donc aussi une mesure de sécurisation de l'application

Capture d’écran du 2025-01-14 16-29-55

@florimondmanca
Copy link
Collaborator

@Lealefoulon Je vais créer un ticket dans ce repo pour discuter de l'approche, hélas on a zappé le process habituel avec la design review etc...

@Lealefoulon Lealefoulon marked this pull request as ready for review January 22, 2025 09:15
@florimondmanca
Copy link
Collaborator

florimondmanca commented Jan 22, 2025

@Lealefoulon J'ai vu les erreurs dans la CI

 1) App\Tests\Integration\Infrastructure\Symfony\Command\EudonetParis\EudonetParisImportCommandTest::testExecute
TypeError: App\Application\Regulation\Command\CreateRegulationOrderHistoryCommand::__construct(): Argument #2 ($user) must be of type App\Domain\User\User, null given, called in /home/runner/work/dialog/dialog/src/Application/Regulation/Command/SaveRegulationGeneralInfoCommandHandler.php on line 68

 2) App\Tests\Integration\Infrastructure\Symfony\Command\LitteralisImportCommandTest::testExecute
TypeError: App\Application\Regulation\Command\CreateRegulationOrderHistoryCommand::__construct(): Argument #2 ($user) must be of type App\Domain\User\User, null given, called in /home/runner/work/dialog/dialog/src/Application/Regulation/Command/DeleteRegulationCommandHandler.php on line 36

Je pense que c'est lié au fait qu'il manque un login() dans ces deux tests d'intégration, donc le handler ne peut pas récupérer de user ?

@Lealefoulon
Copy link
Collaborator Author

@florimondmanca je n'ai pas accès à la fonction login() car elle est dispo dans la class AbstractWebTestCase et dans mon cas j'utilise TestCase est-ce qu'il faudrait que je rajoute cette fonction dans TestCase pour y avoir accès ? ou je suis à côté dans ma reflexion?

Copy link
Collaborator

@mmarchois mmarchois left a comment

Choose a reason for hiding this comment

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

Quelques petits retours, sinon c'est top :-) !

@mmarchois
Copy link
Collaborator

@Lealefoulon J'ai vu les erreurs dans la CI

 1) App\Tests\Integration\Infrastructure\Symfony\Command\EudonetParis\EudonetParisImportCommandTest::testExecute
TypeError: App\Application\Regulation\Command\CreateRegulationOrderHistoryCommand::__construct(): Argument #2 ($user) must be of type App\Domain\User\User, null given, called in /home/runner/work/dialog/dialog/src/Application/Regulation/Command/SaveRegulationGeneralInfoCommandHandler.php on line 68

 2) App\Tests\Integration\Infrastructure\Symfony\Command\LitteralisImportCommandTest::testExecute
TypeError: App\Application\Regulation\Command\CreateRegulationOrderHistoryCommand::__construct(): Argument #2 ($user) must be of type App\Domain\User\User, null given, called in /home/runner/work/dialog/dialog/src/Application/Regulation/Command/DeleteRegulationCommandHandler.php on line 36

Je pense que c'est lié au fait qu'il manque un login() dans ces deux tests d'intégration, donc le handler ne peut pas récupérer de user ?

Cette erreur disparaitra avec la modification suggérée plus haut sur le fait de ne pas se trimballer le user partout.

Copy link
Collaborator

@florimondmanca florimondmanca left a comment

Choose a reason for hiding this comment

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

C'est top ! ⭐

Pas d'autres commentaires par rapport à ce qu'a mis Mathieu sur la simplification qui serait permise en mettant authenticatedUser dans CreateRegulationHistoryCommandHandler directement

@Lealefoulon
Copy link
Collaborator Author

Lealefoulon commented Jan 22, 2025

@Lealefoulon J'ai vu les erreurs dans la CI

 1) App\Tests\Integration\Infrastructure\Symfony\Command\EudonetParis\EudonetParisImportCommandTest::testExecute
TypeError: App\Application\Regulation\Command\CreateRegulationOrderHistoryCommand::__construct(): Argument #2 ($user) must be of type App\Domain\User\User, null given, called in /home/runner/work/dialog/dialog/src/Application/Regulation/Command/SaveRegulationGeneralInfoCommandHandler.php on line 68

 2) App\Tests\Integration\Infrastructure\Symfony\Command\LitteralisImportCommandTest::testExecute
TypeError: App\Application\Regulation\Command\CreateRegulationOrderHistoryCommand::__construct(): Argument #2 ($user) must be of type App\Domain\User\User, null given, called in /home/runner/work/dialog/dialog/src/Application/Regulation/Command/DeleteRegulationCommandHandler.php on line 36

Je pense que c'est lié au fait qu'il manque un login() dans ces deux tests d'intégration, donc le handler ne peut pas récupérer de user ?

Cette erreur disparaitra avec la modification suggérée plus haut sur le fait de ne pas se trimballer le user partout.

L'erreur est toujours présente.

  1. App\Tests\Integration\Infrastructure\Symfony\Command\EudonetParis\EudonetParisImportCommandTest::testExecute
    Error: Call to a member function getUuid() on null

2)App\Tests\Integration\Infrastructure\Symfony\Command\LitteralisImportCommandTest::testExecute
Error: Call to a member function getUuid() on null.

@mmarchois
Copy link
Collaborator

  1. App\Tests\Integration\Infrastructure\Symfony\Command\EudonetParis\EudonetParisImportCommandTest::testExecute

Je pense qu'il faut que tu mock l'appelle à la méthode dans le test EudonetParisImportCommandTest

@codecov-commenter
Copy link

codecov-commenter commented Jan 23, 2025

Codecov Report

Attention: Patch coverage is 76.19048% with 10 lines in your changes missing coverage. Please review.

Project coverage is 98.55%. Comparing base (97f7ab1) to head (1743c4c).

Files with missing lines Patch % Lines
src/Domain/Regulation/RegulationOrderHistory.php 16.66% 10 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1147      +/-   ##
============================================
- Coverage     98.67%   98.55%   -0.12%     
- Complexity     1852     1864      +12     
============================================
  Files           368      372       +4     
  Lines          8014     8054      +40     
============================================
+ Hits           7908     7938      +30     
- Misses          106      116      +10     

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

@Lealefoulon Lealefoulon force-pushed the feat/regulation-order-history branch from e100486 to 1743c4c Compare January 23, 2025 11:03
Copy link
Collaborator

@mmarchois mmarchois left a comment

Choose a reason for hiding this comment

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

C'est top, bravo 👏
J'approuve en avance

Copy link
Collaborator

@florimondmanca florimondmanca left a comment

Choose a reason for hiding this comment

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

D'après le message de Codecov, il manque un RegulationOrderHistoryTest.php pour tester les getters :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants