[FEATURE][BACK] Exposer les meta données pour la reconciliation des imports à format (PIX-13234) #9414

xav-car · 2024-07-01T14:51:39Z

🦄 Problème

Pour permettre aux apprenants d'une organization ayant l'import générique de se reconcilier, nous avons besoin de savoir quels champs permettent la réconciliation de cet apprenant

🤖 Proposition

Exposer les champs nécessaires à la réconciliation d'un apprenant par rapport à la config de l'import à format

🌈 Remarques

Le FRONT sera fait dans un second temps afin d'éviter une trop grosse PR

💯 Pour tester

CI au vert ( aller faire un test func sur la PR FRONT )

pix-bot-github · 2024-07-01T14:52:29Z

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

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

machestla · 2024-07-03T09:53:32Z

api/src/prescription/campaign/domain/read-models/CampaignToJoin.js

@@ -78,6 +80,10 @@ class CampaignToJoin {
  get isFlash() {
    return this.assessmentMethod === Assessment.methods.FLASH;
  }
+
+  #computeisRestrictedAccess(isManagingStudents, hasImportLearnerFeature) {


Suggestion :

soit renommer en computeIsRestrictedAccess

soit mettre l'ensemble en inline dans le constructor

soit avoir un getter (option choisie ensemble)

en fait le getter est pas possible car on recoit des param dans le constructeur qu'on ne stocke pas, j'ai préféré inliner le code du computeIsRestrictedAccess dans le constructor

machestla · 2024-07-03T09:55:21Z

api/tests/prescription/campaign/unit/domain/read-models/CampaignToJoin_test.js

@@ -63,4 +63,50 @@ describe('Unit | Domain | Models | CampaignToJoin', function () {
      expect(campaignToJoin.isAccessible).to.be.true;
    });
  });
+
+  describe('#isRestrictedAccess', function () {


Suggestion : renommer le titre selon le nom de la propriété

machestla · 2024-07-03T09:57:23Z

api/src/prescription/campaign/infrastructure/repositories/campaign-to-join-repository.js

@@ -1,5 +1,6 @@
 import { knex } from '../../../../../db/knex-database-connection.js';
 import { NotFoundError } from '../../../../../lib/domain/errors.js';
+import * as organizationFeatureAPI from '../../../../organizational-entities/application/api/organization-features-api.js';


To do: injecter la dépendance au lieu de faire un import

machestla · 2024-07-03T10:01:38Z

...ription/campaign/integration/infrastructure/repositories/campaign-to-join-repository_test.js

@@ -50,6 +52,26 @@ describe('Integration | Repository | CampaignToJoin', function () {
      expect(actualCampaign.identityProvider).to.equal(OidcIdentityProviders.POLE_EMPLOI.code);
    });

+    it('should return restricted access if organization has learner import feature', async function () {
+      // given
+      const organization = databaseBuilder.factory.buildOrganization({ isManagingStudents: false });


To do : une fois l'injection de dépendance faite, nous n'aurons pas besoin de "re-"tester cette partie là donc nous n'aurons pas besoin de recréer une organization ou une featureId

machestla · 2024-07-03T10:05:08Z

...rescription/campaign-participation/unit/application/learner-participation-controller_test.js

@@ -7,7 +7,7 @@ import { usecases } from '../../../../../src/prescription/campaign-participation
 import { ApplicationTransaction } from '../../../../../src/prescription/shared/infrastructure/ApplicationTransaction.js';
 import { domainBuilder, expect, hFake, sinon } from '../../../../test-helper.js';

-describe('Unit | Application | Controller | Campaign-Participation', function () {
+describe('Unit | Application | Controller | learner-Participation', function () {


To do : mettre une majuscule à Learner

machestla · 2024-07-03T10:05:30Z

api/tests/unit/application/campaign-participations/campaign-participation-controller_test.js

@@ -1,7 +1,7 @@
 import { campaignParticipationController } from '../../../../lib/application/campaign-participations/campaign-participation-controller.js';
 import { usecases as devcompUsecases } from '../../../../src/devcomp/domain/usecases/index.js';
 import { expect, sinon } from '../../../test-helper.js';
-describe('Unit | Application | Controller | Campaign-Participation', function () {
+describe('Unit | Application | Controller | Campaign-Participation (lib)', function () {


Suggestion : retirer (lib)

machestla · 2024-07-03T12:20:14Z

api/src/prescription/campaign-participation/application/learner-participation-controller.js

@@ -9,10 +9,12 @@ const save = async function (request, h, dependencies = { campaignParticipationS
  const userId = request.auth.credentials.userId;


Suggestion : drop ce commit car potentiellement on augmente la dette vis à vis de cette PR

machestla · 2024-07-03T12:21:37Z

api/src/organizational-entities/infrastructure/repositories/organization-feature-repository.js

@@ -3,6 +3,7 @@
 */
 import { knex } from '../../../../db/knex-database-connection.js';
 import * as knexUtils from '../../../../src/shared/infrastructure/utils/knex-utils.js';
+import { ApplicationTransaction } from '../../../prescription/shared/infrastructure/ApplicationTransaction.js';


to do : drop ce commit aussi car utilise uniquement ce qui se trouve dans le commit précédent

machestla · 2024-07-03T12:24:04Z

api/src/prescription/campaign-participation/domain/models/CampaignToStartParticipation.js

    this.multipleSendings = multipleSendings;
    this.assessmentMethod = assessmentMethod;
    this.skillCount = skillCount;
    this.organizationId = organizationId;
    this.deletedAt = deletedAt;
+    this.#computeisRestrictedAccess(isManagingStudents, hasLearnersImportFeature);


to do :

appliquer la même modification que pour CampaignToJoin

ajouter dans la dette technique : regarder la différence entre CampaignToJoin et CampaignToStartParticipation

soit se baser sur le model Campaign pour les deux

soit les merger ensemble s'ils font la même chose

machestla · 2024-07-03T12:28:40Z

api/tests/prescription/campaign-participation/unit/domain/models/CampaignParticipant_test.js

@@ -226,13 +226,34 @@ describe('Unit | Domain | Models | CampaignParticipant', function () {
      let restrictedCampaign;
      beforeEach(function () {
        userIdentity = { id: 1 };
+      });
+
+      it('throws a ForbiddenAccess exception when the user is not in the organization trainees on managingStudents cases', async function () {


To do : remplacer "trainees" par "learners"

Suggestion : déplacer ce fichier dans le commit où la gestion d'erreur est faite

machestla · 2024-07-03T12:33:39Z

...iption/campaign-participation/infrastructure/repositories/campaign-participant-repository.js

@@ -8,12 +8,12 @@ import {
 import { OrganizationLearnerForStartingParticipation } from '../../../../../lib/domain/read-models/OrganizationLearnerForStartingParticipation.js';
 import { UserIdentity } from '../../../../../lib/domain/read-models/UserIdentity.js';
 import * as campaignRepository from '../../../../../lib/infrastructure/repositories/campaign-repository.js';
+import * as organizationImportFeatureApi from '../../../../organizational-entities/application/api/organization-features-api.js';


To do : remplacer par l'injection de dépendance

machestla · 2024-07-03T12:38:20Z

api/src/prescription/campaign/domain/read-models/CampaignToJoin.js

    this.#computeisRestrictedAccess(organizationIsManagingStudents, hasLearnersImportFeature);
+    this.reconciliationFields = null;


Suggestion : remonter cette ligne une ligne plus haut

machestla · 2024-07-03T12:44:36Z

api/tests/prescription/campaign/unit/domain/read-models/CampaignToJoin_test.js

@@ -109,4 +109,46 @@ describe('Unit | Domain | Models | CampaignToJoin', function () {
      expect(campaignToJoin.isRestricted).to.be.false;
    });
  });
+
+  describe('#setReconciliationFields', function () {
+    it('should return null when no reconcilation defined', function () {


Suggestion :
it('should return null when no reconciliation defined', function () {

machestla · 2024-07-03T12:45:47Z

api/tests/prescription/campaign/unit/domain/read-models/CampaignToJoin_test.js

+  });
+
+  describe('#isReconciliationRequired', function () {
+    it('should return false when feature not enabled with reconcilationFields', function () {


Suggestion :
t('should return false when feature not enabled with reconciliationFields', function () {

…ature internal API

… campaignToStartParticipation model

xav-car assigned lionelB and xav-car Jul 1, 2024

xav-car added 👀 Tech Review Needed team-prescription labels Jul 1, 2024

xav-car marked this pull request as ready for review July 1, 2024 14:52

xav-car requested review from a team as code owners July 1, 2024 14:52

machestla reviewed Jul 3, 2024

View reviewed changes

lionelB force-pushed the pix-13234/expose-reconciliation-meta branch 5 times, most recently from b028f4c to 5da82f8 Compare July 3, 2024 21:54

machestla approved these changes Jul 8, 2024

View reviewed changes

machestla added Tech Review OK and removed 👀 Tech Review Needed labels Jul 8, 2024

machestla added the 🚀 Ready to Merge label Jul 8, 2024

lionelB force-pushed the pix-13234/expose-reconciliation-meta branch from 5da82f8 to 61fd17c Compare July 8, 2024 08:31

lionelB and others added 11 commits July 8, 2024 08:31

test?(api): rename unit test for campaign participation controller

e157cac

feat(api): add campaign on generic pro orga

8a9010a

fix?(api): use correct variable name

b6ea397

feat(api): update campaign to join model to enhance restricted access

fbfc948

feat(api): update campaign to join repository to call organization fe…

f82b4f2

…ature internal API

refactor(api): add a computeisRestrictedAccess to set isRestricted in…

c1fff20

… campaignToStartParticipation model

feat(api): use internal api to set restrictedAccess

83b9e72

feat(api): set reconciliation field on usecase when needed

c6d459c

feat(api): update st erializers

3479d1a

feat(api): add missing type validation

d63cdf6

test(api): fix typo

14d15ba

pix-service-auto-merge force-pushed the pix-13234/expose-reconciliation-meta branch from 61fd17c to 14d15ba Compare July 8, 2024 08:31

pix-service-auto-merge merged commit f448c8d into dev Jul 8, 2024
6 of 7 checks passed

pix-service-auto-merge deleted the pix-13234/expose-reconciliation-meta branch July 8, 2024 08:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FEATURE][BACK] Exposer les meta données pour la reconciliation des imports à format (PIX-13234) #9414

[FEATURE][BACK] Exposer les meta données pour la reconciliation des imports à format (PIX-13234) #9414

xav-car commented Jul 1, 2024 •

edited by machestla

Loading

pix-bot-github commented Jul 1, 2024

machestla Jul 3, 2024 •

edited

Loading

lionelB Jul 3, 2024

machestla Jul 3, 2024

machestla Jul 3, 2024

machestla Jul 3, 2024

machestla Jul 3, 2024

machestla Jul 3, 2024

machestla Jul 3, 2024

machestla Jul 3, 2024

machestla Jul 3, 2024 •

edited

Loading

machestla Jul 3, 2024

machestla Jul 3, 2024

machestla Jul 3, 2024

machestla Jul 3, 2024

machestla Jul 3, 2024

machestla Jul 3, 2024

		@@ -9,10 +9,12 @@ const save = async function (request, h, dependencies = { campaignParticipationS
		const userId = request.auth.credentials.userId;

		this.#computeisRestrictedAccess(organizationIsManagingStudents, hasLearnersImportFeature);
		this.reconciliationFields = null;

[FEATURE][BACK] Exposer les meta données pour la reconciliation des imports à format (PIX-13234) #9414

[FEATURE][BACK] Exposer les meta données pour la reconciliation des imports à format (PIX-13234) #9414

Conversation

xav-car commented Jul 1, 2024 • edited by machestla Loading

🦄 Problème

🤖 Proposition

🌈 Remarques

💯 Pour tester

pix-bot-github commented Jul 1, 2024

machestla Jul 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

machestla Jul 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xav-car commented Jul 1, 2024 •

edited by machestla

Loading

machestla Jul 3, 2024 •

edited

Loading

machestla Jul 3, 2024 •

edited

Loading