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

[FEATURE] Ajouter une route de duplication des profils cibles (PIX-11828). #9273

Merged

Conversation

La-toile-cosmique
Copy link
Contributor

@La-toile-cosmique La-toile-cosmique commented Jun 14, 2024

🦄 Problème

Dans le cadre de l'epix de duplication d'un profil cible, on souhaite créer la route de copie d'un profil cible.

🤖 Proposition

Créer une route qui dupliquera un profil cible et renommera le nouveau profil cible Copie_de_ancienNom.

Remarque

Code temporairement dans le dossier Lib, on compte migrer avant de livrer l'Epix.

💯 Pour tester

Cette PR étant la partie Backend de la feature, le test fonctionnel viendra avec la PR frontend.

@La-toile-cosmique La-toile-cosmique added the team-evaluation PR relatives à l'expérience d'évaluation label Jun 14, 2024
@pix-bot-github
Copy link

Une fois les applications déployées, elles seront accessibles via les liens suivants :

Les variables d'environnement seront accessibles via les liens suivants :

@Libouk Libouk force-pushed the pix-11828-add-target-profile-duplication-route branch from 1c16aa1 to 3ea4e47 Compare June 14, 2024 09:46
@Libouk Libouk marked this pull request as ready for review June 14, 2024 09:55
@Jeyffrey Jeyffrey changed the title [FEATURE] Ajouter une route de duplication des profils cibles [FEATURE] Ajouter une route de duplication des profils cibles (PIX-11828). Jun 14, 2024
Copy link
Contributor

@Jeyffrey Jeyffrey left a comment

Choose a reason for hiding this comment

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

L'ordre des fichiers dans les commits est un peu compliqué :/
Je pense qu'il vaut mieux tout squash a la limite, c'est pas si gros.

@La-toile-cosmique La-toile-cosmique force-pushed the pix-11828-add-target-profile-duplication-route branch 2 times, most recently from 6844b8f to ccb93be Compare June 17, 2024 09:33
@Libouk
Copy link
Member

Libouk commented Jun 17, 2024

Dans un test unitaire du usecase copyTargetProfile : Le usecase doit appeler le repo avec les bonnes infos, stub le get du targetProfile qui renverra au choix un symbol ou une instance du domainBuilder, puis stub l'appeler de la création d'un nouveau target profile

@Libouk
Copy link
Member

Libouk commented Jun 17, 2024

Rajouter un test unitaire ou integ si on tente d'appeler la route avec un rôle non autorisé

@Libouk
Copy link
Member

Libouk commented Jun 17, 2024

Rajouter un test unitaire sur le controller qui vérifie qu'on appelle le usecase avec les paramètres attendus. A voir si nécessaire par rapport à ce qui est testé dans le test d'acceptance

@Libouk Libouk force-pushed the pix-11828-add-target-profile-duplication-route branch from ccb93be to a480e35 Compare June 18, 2024 12:37
@La-toile-cosmique La-toile-cosmique force-pushed the pix-11828-add-target-profile-duplication-route branch 3 times, most recently from 145443d to 6877f83 Compare June 20, 2024 13:02
@Libouk Libouk removed the request for review from a team June 20, 2024 13:17
@Libouk Libouk closed this Jun 20, 2024
@Libouk Libouk reopened this Jun 20, 2024
@pix-bot-github
Copy link

Une fois les applications déployées, elles seront accessibles via les liens suivants :

Les variables d'environnement seront accessibles via les liens suivants :

@Libouk Libouk force-pushed the pix-11828-add-target-profile-duplication-route branch 2 times, most recently from e7098f3 to dfce982 Compare June 21, 2024 08:41
const copiedTargetProfile = TargetProfileForCreation.copyTargetProfile({ ...targetProfileToCopy, tubes });
const copiedTargetProfile = TargetProfileForCreation.copyTargetProfile({
...targetProfileToCopy,
tubes: targetProfileTubes.map((tube) => ({
Copy link
Contributor

@xav-car xav-car Jun 21, 2024

Choose a reason for hiding this comment

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

suggestion :

Si le repo retourne un model "mappé" comme vous le souhaitez. Ce mapping n'est plus nécessaire côté usecase.
Votre test unitaire dans le dernier commit pourrait s'affranchir de devoir créer un POJO { tubeId, level } pour que le test fonctionne.

Copy link
Member

Choose a reason for hiding this comment

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

Je n'ai pas bien compris ce que tu suggérais comme modification ici

Copy link
Contributor

Choose a reason for hiding this comment

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

C'est plus au repository de te remonter un objet du domain comme tu souhaites l'utiliser. Plutôt que de faire le mapping dans ton usecase a partir d'un Pojo "simple".

En créant un objet du domain TargetProfileTube par exemple. Ton repository remonte une liste de trajet profil tube. Et tu as juste a le passer au copyTragetProgile ensuite.

Copy link
Contributor

@xav-car xav-car left a comment

Choose a reason for hiding this comment

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

tech ok ( les retours ne sont que des suggestions )

Copy link
Contributor

@xav-car xav-car left a comment

Choose a reason for hiding this comment

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

Func validé en curl sur la duplication du profil cible 6001

curl -H "Accept: application/json" -H "Content-Type: application/json" \
  -H "Authorization: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ1c2VyX2lkIjo5MDAwMCwic291cmNlIjoicGl4IiwiaWF0IjoxNzE4OTc4NDc5LCJleHAiOjE3MTg5Nzk2Nzl9.TYEhAwDURfJGyhdyq4f0eH-mVt39kxbfdPF85Wil-ek" \
  -X POST https://app-pr9273.review.pix.fr//api/admin/target-profiles/6001/copy

@xav-car xav-car added Func Review OK PO validated functionally the PR and removed 👀 Func Review Needed labels Jun 21, 2024
@Libouk Libouk force-pushed the pix-11828-add-target-profile-duplication-route branch from dfce982 to d4fce08 Compare June 21, 2024 14:19
@pix-service-auto-merge pix-service-auto-merge force-pushed the pix-11828-add-target-profile-duplication-route branch from d4fce08 to b73c7d9 Compare June 24, 2024 07:54
@pix-service-auto-merge pix-service-auto-merge merged commit 6f9fba3 into dev Jun 24, 2024
6 of 7 checks passed
@pix-service-auto-merge pix-service-auto-merge deleted the pix-11828-add-target-profile-duplication-route branch June 24, 2024 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Func Review OK PO validated functionally the PR 🚀 Ready to Merge team-evaluation PR relatives à l'expérience d'évaluation Tech Review OK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants