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

[Territoire 69] Modifications communes à autoriser + règles affectation #893

Merged
merged 15 commits into from
Feb 10, 2023

Conversation

sfinx13
Copy link
Collaborator

@sfinx13 sfinx13 commented Feb 7, 2023

Ticket

#841

Description

Le territoire du Rhône a un fonctionnement particulier avec deux agglomérations. Nous avons la métropole de Lyon avec ses communes qui sont les premiers à utiliser la plateforme et nous avons la COR qui rejoint la plateforme.
Depuis la liste des signalements, de la cartographie et des notifications de la plateforme, les signalements venant de la MDL ne doivent pas être par la COR et les signalement de la COR ne doivent pas être par la MDL

Changements apportés

  • Ajout d'un fichier de configuration qui contient la liste des codes insee des deux agglomérations
  • Modification des requêtes dans SignalementRepository et NotificationRepository afin d'appliquer les filtres nécessaires
  • Mise à jour des fixtures avec des partenaires du 69
  • Mise à jour des migrations
  • Suppression de code mort
  • Mise à jour smoke test en mode connecté
  • Nom et code insee en lecture seule pour les partenaires COR et MDL (Responsable Territoire)
  • Mise à jour de test afin de vérifier que les signalement du 69 ont bien les partenaires possibles
  • Activation du compte et mot de passe par défaut sur la commande app-add-user

Commande à utiliser sur les review app et staging

Tests

https://histologe-staging-pr893.osc-fr1.scalingo.io/

Mot de passe : histologe

Territoire Partenaire Email Rôle
N/A Admin HISTOLOGE [email protected] ROLE_ADMIN
Rhône - 69 EMHA - Métropole de Lyon [email protected] ROLE_ADMIN_TERRITORY
Rhône - 69 COR [email protected] ROLE_ADMIN_TERRITORY
Rhône - 69 Commune 69-03 MDL [email protected] ROLE_USER_PARTNER
Rhône - 69 Commune 69-04 COR [email protected] ROLE_USER_PARTNER
Rhône - 69 Partenaire 69-05 [email protected] ROLE_USER_PARTNER
Rhône - 69 Partenaire 69-06 [email protected] ROLE_USER_PARTNER
  • En tant que super admin, je peux voir tous les signalement du Rhône
  • En tant que responsable territoire MDL, je peux voir tous les signalement MDL et ne pas voir les signalements COR
    - Liste des signalements
    - Cartographie
    - Notification
  • En tant que responsable territoire COR, je peux voir tous les signalement COR et ne pas voir les signalements MDL
    - Liste des signalements
    - Cartographie
    - Notification
  • En tant que responsable territoire MDL, je ne peux pas affecter la COR à un signalement
  • En tant que responsable territoire COR, je ne peux pas affecter la MDL à un signalement
  • En tant que responsable territoire MDL, je ne peux pas affecter un partenaire commune COR à un signalement
  • En tant que responsable territoire COR, je ne peux pas affecter un partenaire commune MDL à un signalement
  • En tant que responsable terrtoire COR et MDL, je peux affecter un partenaire lambda du 69
  • En tant qu'usager COR je peux déposer un signalement (code postal : [Territoire 69] Modifications communes à autoriser + règles affectation #841 )
  • En tant qu'usager MDL, je peux déposer un signalement (code postal: [Territoire 69] Liste des communes à autoriser #673)

Documentaton

Pour rendre un territoire spécial, il faudra compléter le fichier de configuration
config/territory/insee_agglomeration.yaml

Historique

#673

@sfinx13 sfinx13 changed the title Feature/841 cor lyon [Territoire 69] Modifications communes à autoriser + règles affectation Feb 8, 2023
@sfinx13 sfinx13 force-pushed the feature/841-cor-lyon branch from acb2185 to 05d2301 Compare February 8, 2023 17:05
@@ -87,8 +87,8 @@ private function createFilters(Request $request, ?Territory $territory): Statist
{
$communes = json_decode($request->get('communes'));
$statut = $request->get('statut');
$strEtiquettes = $request->get('etiquettes');
$etiquettes = array_map(fn ($value): int => $value * 1, json_decode($strEtiquettes));
$strEtiquettes = json_decode($request->get('etiquettes') ?? '[]');
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 ajouté des tests dans SmokeTest en mode connecté pour les super admin, admin territoire et utilisateurs partenaires et ça plantait à ce niveau la.

@@ -41,6 +48,7 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
->add('nom', TextType::class, [
'attr' => [
'class' => 'fr-input',
'readonly' => isset($insee[$partner?->getTerritory()?->getZip()][$partner->getNom()]),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Les partenaires COR et MDL ne pourront pas modifier leur nom

Copy link
Collaborator

Choose a reason for hiding this comment

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

pourquoi c'est nécessaire ?

Copy link
Collaborator Author

@sfinx13 sfinx13 Feb 10, 2023

Choose a reason for hiding this comment

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

Car le filtre est appliqué sur le 69 uniquement si le partenaire s’appelle COR (solution simple et facile à identifier pour ce cas exceptionnel)
Pour MDL ce n'était pas nécessaire mais j'ai repris son nom de prod au lieu d'un nom générique Partenaire 69-01 pour faciliter les test
Les deux partenaires qui gèrent les signalements sont ces deux la

@@ -61,6 +69,7 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
->add('insee', TextType::class, [
'attr' => [
'class' => 'fr-input',
'readonly' => isset($insee[$partner?->getTerritory()?->getZip()][$partner->getNom()]),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Les partenaires COR et MDL ne pourront pas modifier leur modifier leur code insee

/**
* @deprecated
*/
public function findAllOrByInseeIfCommune(string|null $insee, Territory|null $territory)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Comme mentionné deprecated (plus utilisé)

Copy link
Collaborator

Choose a reason for hiding this comment

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

<3

@@ -21,24 +21,6 @@ public function __construct(ManagerRegistry $registry)
parent::__construct($registry, Territory::class);
}

public function add(Territory $entity, bool $flush = false): void
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

code mort

@@ -50,18 +32,6 @@ public function findAllList()
->getResult();
}

public function findByZip($zip)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

code mort

@sfinx13 sfinx13 marked this pull request as ready for review February 9, 2023 14:28
@emilschn
Copy link
Collaborator

emilschn commented Feb 9, 2023

Remarque avant relecture complète : il y a aussi un check js dans app.js qui fait une vérification sur les codes postaux

@sfinx13
Copy link
Collaborator Author

sfinx13 commented Feb 9, 2023

Remarque avant relecture complète : il y a aussi un check js dans app.js qui fait une vérification sur les codes postaux

@emilschn oui j'avais pris en compte la vérification JS sur les deux fichiers dont la version minifiée


public function down(Schema $schema): void
{
// this down() migration is auto-generated, please modify it to your needs
Copy link
Collaborator

Choose a reason for hiding this comment

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

commentaire à supprimer

@@ -41,6 +48,7 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
->add('nom', TextType::class, [
'attr' => [
'class' => 'fr-input',
'readonly' => isset($insee[$partner?->getTerritory()?->getZip()][$partner->getNom()]),
Copy link
Collaborator

Choose a reason for hiding this comment

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

pourquoi c'est nécessaire ?

/**
* @deprecated
*/
public function findAllOrByInseeIfCommune(string|null $insee, Territory|null $territory)
Copy link
Collaborator

Choose a reason for hiding this comment

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

<3

@emilschn
Copy link
Collaborator

emilschn commented Feb 9, 2023

Remarque avant relecture complète : il y a aussi un check js dans app.js qui fait une vérification sur les codes postaux

@emilschn oui j'avais pris en compte la vérification JS sur les deux fichiers dont la version minifiée

t'as bien modifié const.js et const.min.js mais pas app.js :)

config/signalement/insee_agglomeration.yaml Outdated Show resolved Hide resolved
src/Repository/SignalementRepository.php Show resolved Hide resolved
src/Repository/SignalementRepository.php Show resolved Hide resolved
$paramsRhone: '%special_territory%'
App\Repository\NotificationRepository:
arguments:
$paramsRhone: '%special_territory%'
Copy link
Collaborator

Choose a reason for hiding this comment

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

je n'ai pas forcément d'idées pour faire mieux, mais il y 'a quelque-chose qui me gêne à appeler ce param paramsRhone, j'ai l'impression qu'on ne pourra pas étendre ce fonctionnement à d'autres territoire si besoin...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ce fonctionnement ne risque pas de s'étendre car histologe n'a pas vocation d'avoir une telle segmentation dans un territoire et si ça arrivat c'est la modélisation qu'il faudrait revoir on est vraiment sur du hack

A la limite si ça arrive sur un deuxieme territoire, il faudrait rajouter un niveau

    special_territory:
        69:
           partner_name_cor: 'COR'
        13:
           partner_name: 'toto'

Au lieu d'avoir $paramsRhone c'est d'avoir $params

    App\Repository\SignalementRepository:
        arguments:
            $params: '%special_territory%'
    App\Repository\NotificationRepository:
        arguments:
            $params: '%special_territory%'

Je peut faire ça, t'en pense quoi ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

oui c'est ok, même si ça risque peut d'arriver c'est plus générique

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Modif effectuée

@sfinx13 sfinx13 force-pushed the feature/841-cor-lyon branch from be0913e to e6ff49a Compare February 10, 2023 14:24
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 6 Code Smells

No Coverage information No Coverage information
9.1% 9.1% Duplication

@hmeneuvrier hmeneuvrier merged commit c63d702 into develop Feb 10, 2023
@sfinx13 sfinx13 deleted the feature/841-cor-lyon branch February 11, 2023 22:40
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.

3 participants