Skip to content

[TECH] Déplacer le domaine parcoursup dans certification/results (PIX-16604). #11442

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

Conversation

Jeyffrey
Copy link
Contributor

@Jeyffrey Jeyffrey commented Feb 18, 2025

🥞 Problème

Il n'y a pas d'intérêt à avoir un domaine métier particulier pour parcoursup.
Parcoursup est un format de resultat, et nous avons deja tout un contexte dedie au partage de resultats de certification.

🥓 Proposition

Déplacer ce domaine dans le sous-domaine certification/results.

😋 Pour tester

  1. Tests verts
  2. Non rég sur Swagger

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

@Jeyffrey Jeyffrey force-pushed the pix-16604-migrate-parcoursup-domain-to-certification-results-sub-domain branch 2 times, most recently from 998d1f0 to b8f8f88 Compare February 18, 2025 14:51
@Steph0 Steph0 force-pushed the pix-16604-migrate-parcoursup-domain-to-certification-results-sub-domain branch from b8f8f88 to 16e417d Compare February 18, 2025 15:09
Copy link
Contributor

@Steph0 Steph0 left a comment

Choose a reason for hiding this comment

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

Test func (et au pire osef de mes retours de PR :D )

Par INE (datamart 1)
image

Par verification code (datamart 2)
image

@@ -1,9 +1,9 @@
import { usecases } from '../domain/usecases/index.js';

const getCertificationResult = async function (request) {
const getCertificationResultForParcoursup = async function (request) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const getCertificationResultForParcoursup = async function (request) {
const getParcoursupResult = async function (request) {

*
* @returns {CertificationResult} matching candidate certification result
* @throws {MoreThanOneMatchingCertificationError} in some cases (INE for example) there might be duplicates
* @throws {NotFoundError} if no certification exists for this candidate
**/
export const getCertificationResult = async ({
export const getCertificationResultForParcoursup = async ({
Copy link
Contributor

Choose a reason for hiding this comment

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

On sait deja que on est dans des resultats de certif

Suggested change
export const getCertificationResultForParcoursup = async ({
export const getResultForParcoursup = async ({

@Steph0 Steph0 added Func Review OK PO validated functionally the PR Tech Review OK and removed 👀 Tech Review Needed labels Feb 18, 2025
@Steph0
Copy link
Contributor

Steph0 commented Feb 18, 2025

Allez on est 3 a etre OK

@pix-service-auto-merge pix-service-auto-merge merged commit 0078361 into dev Feb 18, 2025
12 of 13 checks passed
@pix-service-auto-merge pix-service-auto-merge deleted the pix-16604-migrate-parcoursup-domain-to-certification-results-sub-domain branch February 18, 2025 15:30
@yaf yaf added this to the Bounded context migration milestone Feb 21, 2025
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