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

[BO - Utilisateur] Pré-requis tech - Gestion multi territoires #3263

Merged
merged 6 commits into from
Dec 9, 2024

Conversation

numew
Copy link
Collaborator

@numew numew commented Nov 6, 2024

Ticket

#3227

Description

  • Supprimer le lien Territory de la table User (garder la colonne en base, supprimer son utilisation dans le code)
  • Remplacer le lien Partner de la table User par un lien multiple Partners (garder la colonne en base, supprimer son utilisation dans le code)

L'objectif de préparation technique est ok avec les deux point précédent.
Chemin faisant j'ai aussi pris un peu d'avance sur les parties à venir, soit pour débloquer des situations, soit pour prendre de l'avance.
L’utilisateur de test sur deux territoires "[email protected]" qui sera utile pour la phase suivante à en l'état : la plupart du temps l'affichage des résultats correspondant a son premier territoire/partenaire, parfois à ses deux territoires/partenaires (sur la liste des signalements en l’occurrence)

De manière générale quand l'utilisateur à plusieurs partenaires/territoires j'ai prévu de les afficher, il y'a deux cas ou ce n'est pas faisable et ou j'ai prévu de garder le premier territoire pour :

  • La timezone utilisé
  • Le code département de l'avatar par défaut

Pré-requis

make execute-migration name=Version20241203141732 direction=up
make load-fixtures
make npm-build

Tests

Je ne sais pas trop quoi vous demander comme tests, j'ai souvent vérifié le fonctionnement des modifications apporté dans le code mais pas toujours vu le nombre de fichiers modifiés. Les TU m'ont rapportés quelques points que j'avais ratés (mais je m'attendais a pire).

d’après moi il faudra garder en tête que la MEP de cette PR sera un peu plus a risque que d'habitude

@numew numew force-pushed the feature/3227-multi-partners-data-structure branch 2 times, most recently from 075a5f5 to 4058735 Compare November 13, 2024 17:03
@numew numew force-pushed the feature/3227-multi-partners-data-structure branch from 8cbbea3 to 2600c2b Compare November 15, 2024 17:05
@numew numew changed the title [WIP] [BO - Utilisateur] Pré-requis tech - Gestion multi territoires [BO - Utilisateur] Pré-requis tech - Gestion multi territoires Nov 18, 2024
@numew numew requested review from sfinx13, emilschn and hmeneuvrier and removed request for sfinx13 and emilschn November 18, 2024 13:25
@numew numew marked this pull request as ready for review November 18, 2024 13:26
$user = $userFactory->createInstanceFromArray($partner, $data);
$user = $userFactory->createInstanceFromArray($data);
$userPartner = (new UserPartner())->setUser($user)->setPartner($partner);
$user->addUserPartner($userPartner);
Copy link
Collaborator

Choose a reason for hiding this comment

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

on pourrait remplacer la méthode par addPartner qui se chargerait de faire la ligne du dessus au passage ? ou ça a des implications ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

ah non, ça pourrait être confondu avec le setPartner actuel qu'on garde pour l'instant
Peut-être garder addUserPartner et passer $partner en paramètre ?

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 serait pas propre ni pratique de faire un new UserPartner() dans le User entity. Et je l'ai sorti du userFactory->createInstanceFromArray pour pouvoir récupérer l'object. Je vois pas trop comment faire mieux que ca.

Copy link
Collaborator

Choose a reason for hiding this comment

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

pourquoi pas propre, ni pratique ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Dans le sens ou j'ai du mal à donnée a une entité (User) la responsabilité d'en crée une autre (UserPartner)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Et pourquoi ne pas le faire dans la userFactory du coup ?
Bon j'insiste pas plus que ça... mais ça fait beaucoup de répétition du même type de ligne ! :)

Copy link
Collaborator

@emilschn emilschn left a comment

Choose a reason for hiding this comment

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

Quelques commentaires, pas encore testé

@numew numew force-pushed the feature/3227-multi-partners-data-structure branch from a291819 to bc40958 Compare November 21, 2024 10:08
@emilschn
Copy link
Collaborator

Retours de test :

  • quand on est sur la page d'édition d'un partenaire (et qu'on en a plusieurs), les deux sont sélectionnées dans le menu horizontal
    • et aucun dans le vieux menu vertical :)
  • avec mon RT du 13, j'ai pu passer l'utilisateur multi-territoire en RT aussi ; il me semblait qu'on avait dit que ça devait pas être possible d'être RT
    • du coup, dans les utilisateurs, j'ai accès à l'ensemble des utilisateurs de la plateforme ! (ce bug est le résultat du premier, mais je pense qu'il faudrait le corriger quand même et limiter la requête aux territoires liés)

src/Entity/User.php Show resolved Hide resolved
src/DataFixtures/Files/User.yml Show resolved Hide resolved
src/Entity/User.php Show resolved Hide resolved
src/Entity/User.php Show resolved Hide resolved
src/Entity/User.php Show resolved Hide resolved
src/Entity/User.php Show resolved Hide resolved
src/Manager/HistoryEntryManager.php Outdated Show resolved Hide resolved
src/Service/CacheCommonKeyGenerator.php Show resolved Hide resolved
@numew numew force-pushed the feature/3227-multi-partners-data-structure branch from 0576988 to 95ccb64 Compare November 26, 2024 09:52
@numew numew force-pushed the feature/3227-multi-partners-data-structure branch 2 times, most recently from fa72c04 to 5e1e87f Compare November 29, 2024 08:48
Copy link
Collaborator

@emilschn emilschn left a comment

Choose a reason for hiding this comment

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

Bon, les choses que je repère sont liés à tout ce qui sera fait dans l'UX (correction des requêtes existantes, etc.)
Un peu difficile sur l'entre-deux de faire des tests complets :)
En théorie, ça m'a l'air ok !

Copy link
Collaborator

@sfinx13 sfinx13 left a comment

Choose a reason for hiding this comment

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

Ça sera mon dernier retour.

Rapports de mes tests, j'ai pas vu de problème de régression à la navigation.

  • Stress test OK (50 appels concurrents de création de signalement) make run-concurrency-request nb=50
  • Export PDF/XLS OK
  • Parcours de la création à la fermeture d'un dossier OK (création, validation, cloture agent et cloture RT)
  • Import grille affectation OK
  • Interconnexion OK
  • Import signalement OK

Bravo :-)

use Doctrine\ORM\Mapping as ORM;

#[ORM\Entity(repositoryClass: UserPartnerRepository::class)]
class UserPartner
Copy link
Collaborator

Choose a reason for hiding this comment

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

Je propose de tracker les ajouts en ajoutant la date de création, il faudra donc régénérer la migration

Copy link
Collaborator

@hmeneuvrier hmeneuvrier left a comment

Choose a reason for hiding this comment

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

J'ai refait quelques tests, ça m'a l'air OK aussi !
Bien joué !!

@numew numew force-pushed the feature/3227-multi-partners-data-structure branch 5 times, most recently from 65d2a60 to 26a45a3 Compare December 5, 2024 15:00
@numew numew force-pushed the feature/3227-multi-partners-data-structure branch from 8fb5784 to 2f8310e Compare December 9, 2024 12:36
Copy link

sonarcloud bot commented Dec 9, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
4.3% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@numew numew merged commit d0da062 into develop Dec 9, 2024
3 of 4 checks passed
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.

4 participants