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

[TECH] Migrer la route GET /api/admin/certification-centers/{id} dans src (PIX-15465) #10652

Conversation

er-lim
Copy link
Contributor

@er-lim er-lim commented Nov 26, 2024

🍂 Problème

Le code de la route GET /api/admin/certification-centers/{id} est encore dans lib 🚗

🌰 Proposition

Le migrer dans src.

🎃 Remarques

Le controller appelle un usecase getCenterForAdmin du contexte certification/enrollment. Ce usecase devrait se trouver dans organizational-entities.

🪵 Pour tester

@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 :

@er-lim er-lim added 👀 Tech Review Needed 👀 Func Review Needed Need PO validation for this functionally and removed Development in progress labels Nov 26, 2024
@yaf
Copy link
Member

yaf commented Nov 27, 2024

Merci beaucoup pour cette PR !

Question : Je ne suis pas encore très habitué au scope accès ou certif, est-ce que tu pourrais me dire comment savoir ce qui doit être dans organizational-entities et ce qui ne doit pas y être ?

En voulant me plonger dans ce qu'il reste à migrer côté certif, j'ai essayé de trouver les bons contextes pour les routes restantes dans ce fichier https://docs.google.com/spreadsheets/d/1uYQu1ezjbO1oJwibmcmDN-i78AuooQ1o6gzPrYUJJl4/edit?gid=829217428#gid=829217428 j'ai eu l'impression que tout ce qui touche au centre de certif c'était à mettre dans certification/session-management. J'ai sans doute tort sur certain point du coup ?

Copy link
Contributor

@EmmanuelleBonnemay EmmanuelleBonnemay left a comment

Choose a reason for hiding this comment

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

Lu et testé avec succès

@bpetetot
Copy link
Contributor

bpetetot commented Nov 27, 2024

Question : Je ne suis pas encore très habitué au scope accès ou certif, est-ce que tu pourrais me dire comment savoir ce qui doit être dans organizational-entities et ce qui ne doit pas y être ?

Normalement, on devrait trouver dans organizational-entities tout ce qui concerne la gestion et le déploiement d'organisations et centres de certification auprès de partenaires. C'est à dire la création de nouveaux centre ou organisation, leur configuration... (voire même la gestion de leur facturation). On ne retrouvera aucune feature liée à la Prescription ou Certification mais seulement les "entités organisationnelles" liées à leur déploiement. Peut être que le nom de contexte "deployment" aurait été plus approprié.

Je ne connais pas dans le détail le contexte certification/session-management, mais je dirais qui contient les features liées à la gestion des sessions de certif, et pas les features de gestion des centres de certification.

@yaf J'espère que c'est un peut plus clair.

@er-lim er-lim added this to the Bounded context migration milestone Nov 27, 2024
@er-lim er-lim added Tech Review OK 🚀 Ready to Merge Func Review OK PO validated functionally the PR and removed 👀 Tech Review Needed 👀 Func Review Needed Need PO validation for this functionally labels Nov 27, 2024
@pix-service-auto-merge pix-service-auto-merge force-pushed the pix-15465-migrate-get-admin-certification-center-details branch from 83448ba to b88d894 Compare November 27, 2024 09:31
@pix-service-auto-merge pix-service-auto-merge force-pushed the pix-15465-migrate-get-admin-certification-center-details branch from b88d894 to e12f3bf Compare November 27, 2024 09:38
@pix-service-auto-merge pix-service-auto-merge merged commit b19d129 into dev Nov 27, 2024
8 of 9 checks passed
@pix-service-auto-merge pix-service-auto-merge deleted the pix-15465-migrate-get-admin-certification-center-details branch November 27, 2024 09:44
@yaf
Copy link
Member

yaf commented Nov 28, 2024

@yaf J'espère que c'est un peut plus clair.

Oui, très bien, merci beaucoup !
Je ne me basais que sur le nom... C'est effectivement faible comme grille de lecture. Je regarderai plus en détail ce que fait le usecase lié... Merci !

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.

6 participants