From c1363fd0fb6d089d6030255684dbff9c412e99db Mon Sep 17 00:00:00 2001 From: LionelB Date: Thu, 14 Nov 2024 09:26:49 +0100 Subject: [PATCH 1/2] refactor(api): remove route `/api/users/{userId}/campaign-participations` --- api/lib/application/users/index.js | 26 --- api/lib/application/users/user-controller.js | 10 - ...st-ongoing-user-campaign-participations.js | 5 - .../campaign-participation-repository.js | 37 +-- ...roller-get-campaign-participations_test.js | 210 ------------------ .../campaign-participation-repository_test.js | 77 ------- .../application/security-pre-handlers_test.js | 2 +- .../application/users/user-controller_test.js | 34 --- ...ngoing-user-campaign-participation_test.js | 52 ----- 9 files changed, 2 insertions(+), 451 deletions(-) delete mode 100644 api/lib/domain/usecases/find-latest-ongoing-user-campaign-participations.js delete mode 100644 api/tests/acceptance/application/users/users-controller-get-campaign-participations_test.js delete mode 100644 api/tests/unit/domain/usecases/find-latest-ongoing-user-campaign-participation_test.js diff --git a/api/lib/application/users/index.js b/api/lib/application/users/index.js index e9067af2b73..ff6d48dd53f 100644 --- a/api/lib/application/users/index.js +++ b/api/lib/application/users/index.js @@ -166,32 +166,6 @@ const register = async function (server) { server.route([ ...adminRoutes, - { - method: 'GET', - path: '/api/users/{id}/campaign-participations', - config: { - pre: [ - { - method: securityPreHandlers.checkRequestedUserIsAuthenticatedUser, - assign: 'requestedUserIsAuthenticatedUser', - }, - ], - validate: { - params: Joi.object({ - id: identifiersType.userId, - }), - }, - handler: userController.getCampaignParticipations, - notes: [ - '- **Cette route est restreinte aux utilisateurs authentifiés**\n' + - '- Récupération des participations à des campagnes à partir de l’id\n' + - '- L’id demandé doit correspondre à celui de l’utilisateur authentifié' + - '- Les participations aux campagnes sont triées par ordre inverse de création' + - ' (les plus récentes en premier)', - ], - tags: ['api'], - }, - }, { method: 'GET', path: '/api/users/{id}/trainings', diff --git a/api/lib/application/users/user-controller.js b/api/lib/application/users/user-controller.js index 3dca0e7c669..c361c9ca992 100644 --- a/api/lib/application/users/user-controller.js +++ b/api/lib/application/users/user-controller.js @@ -3,7 +3,6 @@ import * as trainingSerializer from '../../../src/devcomp/infrastructure/seriali import { evaluationUsecases } from '../../../src/evaluation/domain/usecases/index.js'; import * as scorecardSerializer from '../../../src/evaluation/infrastructure/serializers/jsonapi/scorecard-serializer.js'; import * as userDetailsForAdminSerializer from '../../../src/identity-access-management/infrastructure/serializers/jsonapi/user-details-for-admin.serializer.js'; -import * as campaignParticipationSerializer from '../../../src/prescription/campaign-participation/infrastructure/serializers/jsonapi/campaign-participation-serializer.js'; import * as certificationCenterMembershipSerializer from '../../../src/shared/infrastructure/serializers/jsonapi/certification-center-membership.serializer.js'; import * as userSerializer from '../../../src/shared/infrastructure/serializers/jsonapi/user-serializer.js'; import * as requestResponseUtils from '../../../src/shared/infrastructure/utils/request-response-utils.js'; @@ -48,14 +47,6 @@ const findPaginatedUserRecommendedTrainings = async function ( return dependencies.trainingSerializer.serialize(userRecommendedTrainings, meta); }; -const getCampaignParticipations = function (request, h, dependencies = { campaignParticipationSerializer }) { - const authenticatedUserId = request.auth.credentials.userId; - - return usecases - .findLatestOngoingUserCampaignParticipations({ userId: authenticatedUserId }) - .then(dependencies.campaignParticipationSerializer.serialize); -}; - const resetScorecard = function (request, h, dependencies = { scorecardSerializer, requestResponseUtils }) { const authenticatedUserId = request.auth.credentials.userId; const competenceId = request.params.competenceId; @@ -132,7 +123,6 @@ const userController = { findCertificationCenterMembershipsByUser, findPaginatedUserRecommendedTrainings, findUserOrganizationsForAdmin, - getCampaignParticipations, reassignAuthenticationMethods, rememberUserHasSeenAssessmentInstructions, rememberUserHasSeenChallengeTooltip, diff --git a/api/lib/domain/usecases/find-latest-ongoing-user-campaign-participations.js b/api/lib/domain/usecases/find-latest-ongoing-user-campaign-participations.js deleted file mode 100644 index 94b832b1f3f..00000000000 --- a/api/lib/domain/usecases/find-latest-ongoing-user-campaign-participations.js +++ /dev/null @@ -1,5 +0,0 @@ -const findLatestOngoingUserCampaignParticipations = async function ({ userId, campaignParticipationRepository }) { - return campaignParticipationRepository.findLatestOngoingByUserId(userId); -}; - -export { findLatestOngoingUserCampaignParticipations }; diff --git a/api/lib/infrastructure/repositories/campaign-participation-repository.js b/api/lib/infrastructure/repositories/campaign-participation-repository.js index ee42344c150..f25e2cd77fe 100644 --- a/api/lib/infrastructure/repositories/campaign-participation-repository.js +++ b/api/lib/infrastructure/repositories/campaign-participation-repository.js @@ -45,35 +45,6 @@ const get = async function (id) { }); }; -const findLatestOngoingByUserId = async function (userId) { - const campaignParticipations = await knex('campaign-participations') - .join('campaigns', 'campaigns.id', 'campaign-participations.campaignId') - .whereNull('campaigns.archivedAt') - .where({ userId }) - .orderBy('campaign-participations.createdAt', 'DESC') - .select('campaign-participations.*'); - const campaigns = await knex('campaigns').whereIn( - 'id', - campaignParticipations.map((campaignParticipation) => campaignParticipation.campaignId), - ); - const assessments = await knex('assessments') - .whereIn( - 'campaignParticipationId', - campaignParticipations.map((campaignParticipation) => campaignParticipation.id), - ) - .orderBy('createdAt'); - return campaignParticipations.map((campaignParticipation) => { - const campaign = campaigns.find((campaign) => campaign.id === campaignParticipation.campaignId); - return new CampaignParticipation({ - ...campaignParticipation, - campaign: new Campaign(campaign), - assessments: assessments - .filter((assessment) => assessment.campaignParticipationId === campaignParticipation.id) - .map((assessment) => new Assessment(assessment)), - }); - }); -}; - const isRetrying = async function ({ campaignParticipationId }) { const { id: campaignId, userId } = await knex('campaigns') .select('campaigns.id', 'userId') @@ -91,10 +62,4 @@ const isRetrying = async function ({ campaignParticipationId }) { ); }; -export { - findLatestOngoingByUserId, - get, - getCodeOfLastParticipationToProfilesCollectionCampaignForUser, - hasAssessmentParticipations, - isRetrying, -}; +export { get, getCodeOfLastParticipationToProfilesCollectionCampaignForUser, hasAssessmentParticipations, isRetrying }; diff --git a/api/tests/acceptance/application/users/users-controller-get-campaign-participations_test.js b/api/tests/acceptance/application/users/users-controller-get-campaign-participations_test.js deleted file mode 100644 index 663c3ef40e6..00000000000 --- a/api/tests/acceptance/application/users/users-controller-get-campaign-participations_test.js +++ /dev/null @@ -1,210 +0,0 @@ -import { - createServer, - databaseBuilder, - expect, - generateValidRequestAuthorizationHeader, -} from '../../../test-helper.js'; - -describe('Acceptance | Controller | GET /user/id/campaign-participations', function () { - let userId; - let campaign1; - let campaign2; - let campaign3; - let campaignParticipation1; - let campaignParticipation2; - let campaignParticipation3; - let assessment1; - let assessment2; - let options; - let server; - - beforeEach(async function () { - server = await createServer(); - }); - - describe('GET /users/:id/campaign-participations', function () { - beforeEach(function () { - const user = databaseBuilder.factory.buildUser(); - userId = user.id; - - campaign1 = databaseBuilder.factory.buildCampaign(); - const oldDate = new Date('2018-02-03T01:02:03Z'); - campaignParticipation1 = databaseBuilder.factory.buildCampaignParticipation({ - userId, - campaignId: campaign1.id, - createdAt: oldDate, - status: 'STARTED', - }); - - campaign2 = databaseBuilder.factory.buildCampaign(); - const recentDate = new Date('2018-05-06T01:02:03Z'); - campaignParticipation2 = databaseBuilder.factory.buildCampaignParticipation({ - userId, - campaignId: campaign2.id, - createdAt: recentDate, - status: 'SHARED', - }); - - campaign3 = databaseBuilder.factory.buildCampaign({ type: 'PROFILES_COLLECTION' }); - const middleDate = new Date('2018-04-06T01:02:03Z'); - campaignParticipation3 = databaseBuilder.factory.buildCampaignParticipation({ - userId, - campaignId: campaign3.id, - createdAt: middleDate, - status: 'SHARED', - }); - - assessment1 = databaseBuilder.factory.buildAssessment({ campaignParticipationId: campaignParticipation1.id }); - assessment2 = databaseBuilder.factory.buildAssessment({ campaignParticipationId: campaignParticipation2.id }); - - options = { - method: 'GET', - url: `/api/users/${userId}/campaign-participations`, - headers: { authorization: generateValidRequestAuthorizationHeader(userId) }, - }; - - return databaseBuilder.commit(); - }); - - describe('Resource access management', function () { - it('should respond with a 401 - unauthorized access - if user is not authenticated', async function () { - // given - options.headers.authorization = 'invalid.access.token'; - - // when - const response = await server.inject(options); - - // then - expect(response.statusCode).to.equal(401); - }); - - it('should respond with a 403 - forbidden access - if requested user is not the same as authenticated user', async function () { - // given - const otherUserId = 9999; - options.headers.authorization = generateValidRequestAuthorizationHeader(otherUserId); - - // when - const response = await server.inject(options); - - // then - expect(response.statusCode).to.equal(403); - }); - }); - - describe('Success case', function () { - it('should return found campaign-participations with 200 HTTP status code', async function () { - // when - const response = await server.inject(options); - - // then - expect(response.statusCode).to.equal(200); - expect(response.result).to.deep.equal({ - data: [ - { - type: 'campaign-participations', - id: campaignParticipation2.id.toString(), - attributes: { - 'is-shared': true, - 'participant-external-id': campaignParticipation2.participantExternalId, - 'shared-at': campaignParticipation2.sharedAt, - 'deleted-at': campaignParticipation2.deletedAt, - 'created-at': campaignParticipation2.createdAt, - }, - relationships: { - campaign: { - data: { type: 'campaigns', id: `${campaign2.id}` }, - }, - assessment: { - links: { - related: `/api/assessments/${assessment2.id}`, - }, - }, - trainings: { - links: { - related: `/api/campaign-participations/${campaignParticipation2.id}/trainings`, - }, - }, - }, - }, - { - type: 'campaign-participations', - id: campaignParticipation3.id.toString(), - attributes: { - 'is-shared': true, - 'participant-external-id': campaignParticipation3.participantExternalId, - 'shared-at': campaignParticipation3.sharedAt, - 'deleted-at': campaignParticipation3.deletedAt, - 'created-at': campaignParticipation3.createdAt, - }, - relationships: { - campaign: { - data: { type: 'campaigns', id: `${campaign3.id}` }, - }, - trainings: { - links: { - related: `/api/campaign-participations/${campaignParticipation3.id}/trainings`, - }, - }, - }, - }, - { - type: 'campaign-participations', - id: campaignParticipation1.id.toString(), - attributes: { - 'is-shared': false, - 'participant-external-id': campaignParticipation1.participantExternalId, - 'shared-at': campaignParticipation1.sharedAt, - 'deleted-at': campaignParticipation1.deletedAt, - 'created-at': campaignParticipation1.createdAt, - }, - relationships: { - campaign: { - data: { type: 'campaigns', id: `${campaign1.id}` }, - }, - assessment: { - links: { - related: `/api/assessments/${assessment1.id}`, - }, - }, - trainings: { - links: { - related: `/api/campaign-participations/${campaignParticipation1.id}/trainings`, - }, - }, - }, - }, - ], - included: [ - { - type: 'campaigns', - id: campaign2.id.toString(), - attributes: { - code: campaign2.code, - title: campaign2.title, - type: 'ASSESSMENT', - }, - }, - { - type: 'campaigns', - id: campaign3.id.toString(), - attributes: { - code: campaign3.code, - title: campaign3.title, - type: 'PROFILES_COLLECTION', - }, - }, - { - type: 'campaigns', - id: campaign1.id.toString(), - attributes: { - code: campaign1.code, - title: campaign1.title, - type: 'ASSESSMENT', - }, - }, - ], - }); - }); - }); - }); -}); diff --git a/api/tests/integration/infrastructure/repositories/campaign-participation-repository_test.js b/api/tests/integration/infrastructure/repositories/campaign-participation-repository_test.js index 2fb4269f30d..90f8b63aeb6 100644 --- a/api/tests/integration/infrastructure/repositories/campaign-participation-repository_test.js +++ b/api/tests/integration/infrastructure/repositories/campaign-participation-repository_test.js @@ -2,7 +2,6 @@ import { DomainTransaction } from '../../../../lib/infrastructure/DomainTransact import * as campaignParticipationRepository from '../../../../lib/infrastructure/repositories/campaign-participation-repository.js'; import { CampaignParticipationStatuses, CampaignTypes } from '../../../../src/prescription/shared/domain/constants.js'; import { constants } from '../../../../src/shared/domain/constants.js'; -import { Campaign } from '../../../../src/shared/domain/models/Campaign.js'; import { databaseBuilder, expect, sinon } from '../../../test-helper.js'; const { STARTED, SHARED, TO_SHARE } = CampaignParticipationStatuses; @@ -318,82 +317,6 @@ describe('Integration | Repository | Campaign Participation', function () { }); }); - describe('#findLatestOngoingByUserId', function () { - let userId; - - beforeEach(async function () { - userId = databaseBuilder.factory.buildUser().id; - await databaseBuilder.commit(); - }); - - it('should only retrieve participations from user', async function () { - const campaignId = databaseBuilder.factory.buildCampaign({ - createdAt: new Date('2000-01-01T10:00:00Z'), - archivedAt: null, - }).id; - const otherUserId = databaseBuilder.factory.buildUser().id; - - databaseBuilder.factory.buildCampaignParticipation({ - userId, - campaignId, - }); - databaseBuilder.factory.buildCampaignParticipation({ - userId: otherUserId, - campaignId, - }); - await databaseBuilder.commit(); - - const latestCampaignParticipations = await campaignParticipationRepository.findLatestOngoingByUserId(userId); - - expect(latestCampaignParticipations.length).to.equal(1); - }); - - it('should retrieve the most recent campaign participations where the campaign is not archived', async function () { - const campaignId = databaseBuilder.factory.buildCampaign({ - createdAt: new Date('2000-01-01T10:00:00Z'), - archivedAt: null, - }).id; - const moreRecentCampaignId = databaseBuilder.factory.buildCampaign({ - createdAt: new Date('2000-02-01T10:00:00Z'), - archivedAt: null, - }).id; - const mostRecentButArchivedCampaignId = databaseBuilder.factory.buildCampaign({ - createdAt: new Date('2001-03-01T10:00:00Z'), - archivedAt: new Date('2000-09-01T10:00:00Z'), - }).id; - - databaseBuilder.factory.buildCampaignParticipation({ - userId, - createdAt: new Date('2000-04-01T10:00:00Z'), - campaignId: moreRecentCampaignId, - }); - const expectedCampaignParticipationId = databaseBuilder.factory.buildCampaignParticipation({ - userId, - createdAt: new Date('2000-07-01T10:00:00Z'), - campaignId, - }).id; - databaseBuilder.factory.buildCampaignParticipation({ - userId, - createdAt: new Date('2001-08-01T10:00:00Z'), - campaignId: mostRecentButArchivedCampaignId, - }); - - databaseBuilder.factory.buildAssessment({ userId, campaignParticipationId: expectedCampaignParticipationId }); - databaseBuilder.factory.buildAssessment({ userId, campaignParticipationId: expectedCampaignParticipationId }); - - await databaseBuilder.commit(); - - const latestCampaignParticipations = await campaignParticipationRepository.findLatestOngoingByUserId(userId); - const [latestCampaignParticipation1, latestCampaignParticipation2] = latestCampaignParticipations; - - expect(latestCampaignParticipation1.createdAt).to.deep.equal(new Date('2000-07-01T10:00:00Z')); - expect(latestCampaignParticipation2.createdAt).to.deep.equal(new Date('2000-04-01T10:00:00Z')); - expect(latestCampaignParticipation1.assessments).to.be.instanceOf(Array); - expect(latestCampaignParticipation1.campaign).to.be.instanceOf(Campaign); - expect(latestCampaignParticipations).to.have.lengthOf(2); - }); - }); - describe('#isRetrying', function () { let campaignId; let campaignParticipationId; diff --git a/api/tests/shared/acceptance/application/security-pre-handlers_test.js b/api/tests/shared/acceptance/application/security-pre-handlers_test.js index e939f44c438..2ce0bc0bf55 100644 --- a/api/tests/shared/acceptance/application/security-pre-handlers_test.js +++ b/api/tests/shared/acceptance/application/security-pre-handlers_test.js @@ -48,7 +48,7 @@ describe('Acceptance | Application | SecurityPreHandlers', function () { // given const options = { method: 'GET', - url: '/api/users/1/campaign-participations', + url: '/api/users/{id}/trainings', headers: { authorization: generateValidRequestAuthorizationHeader(2) }, }; diff --git a/api/tests/unit/application/users/user-controller_test.js b/api/tests/unit/application/users/user-controller_test.js index c88f0ad91f1..fb28f9f9590 100644 --- a/api/tests/unit/application/users/user-controller_test.js +++ b/api/tests/unit/application/users/user-controller_test.js @@ -134,40 +134,6 @@ describe('Unit | Controller | user-controller', function () { }); }); - describe('#getCampaignParticipations', function () { - const userId = '1'; - - const request = { - auth: { - credentials: { - userId: userId, - }, - }, - params: { - id: userId, - }, - }; - - beforeEach(function () { - sinon.stub(usecases, 'findLatestOngoingUserCampaignParticipations'); - }); - - it('should return serialized campaignParticipations', async function () { - // given - const campaignParticipationSerializer = { serialize: sinon.stub() }; - usecases.findLatestOngoingUserCampaignParticipations.withArgs({ userId }).resolves([]); - campaignParticipationSerializer.serialize.withArgs([]).returns({}); - - // when - const response = await userController.getCampaignParticipations(request, hFake, { - campaignParticipationSerializer, - }); - - // then - expect(response).to.deep.equal({}); - }); - }); - describe('#resetScorecard', function () { beforeEach(function () { sinon.stub(evaluationUsecases, 'resetScorecard').resolves({ diff --git a/api/tests/unit/domain/usecases/find-latest-ongoing-user-campaign-participation_test.js b/api/tests/unit/domain/usecases/find-latest-ongoing-user-campaign-participation_test.js deleted file mode 100644 index 6930da381f6..00000000000 --- a/api/tests/unit/domain/usecases/find-latest-ongoing-user-campaign-participation_test.js +++ /dev/null @@ -1,52 +0,0 @@ -import { findLatestOngoingUserCampaignParticipations as findCampaignParticipationsRelatedToUser } from '../../../../lib/domain/usecases/find-latest-ongoing-user-campaign-participations.js'; -import { domainBuilder, expect, sinon } from '../../../test-helper.js'; - -describe('Unit | UseCase | find-latest-user-campaign-participations', function () { - let userId; - const campaignParticipationRepository = { findLatestOngoingByUserId: () => undefined }; - - beforeEach(function () { - sinon.stub(campaignParticipationRepository, 'findLatestOngoingByUserId'); - }); - - it('should call findLatestOngoingByUserId to find all campaign-participations', async function () { - // given - userId = 1; - campaignParticipationRepository.findLatestOngoingByUserId.resolves(); - - // when - await findCampaignParticipationsRelatedToUser({ - userId, - campaignParticipationRepository, - }); - - // then - expect(campaignParticipationRepository.findLatestOngoingByUserId).to.have.been.calledWithExactly(userId); - }); - - it('should return user with his campaign participations', async function () { - // given - const campaignParticipation1 = campaignParticipationRepository.findLatestOngoingByUserId.resolves( - domainBuilder.buildCampaignParticipation({ userId }), - ); - const campaignParticipation2 = campaignParticipationRepository.findLatestOngoingByUserId.resolves( - domainBuilder.buildCampaignParticipation({ userId }), - ); - campaignParticipationRepository.findLatestOngoingByUserId.resolves([ - campaignParticipation1, - campaignParticipation2, - ]); - - // when - const foundCampaignParticipations = await findCampaignParticipationsRelatedToUser({ - userId, - campaignParticipationRepository, - }); - - // then - expect(foundCampaignParticipations).to.be.an.instanceOf(Array); - expect(foundCampaignParticipations).to.have.length(2); - expect(foundCampaignParticipations[0]).to.equal(campaignParticipation1); - expect(foundCampaignParticipations[1]).to.equal(campaignParticipation2); - }); -}); From 5e4d7002aba74487d40ce54b2d6d1bc26e04bc40 Mon Sep 17 00:00:00 2001 From: Xavier Carron <33637571+xav-car@users.noreply.github.com> Date: Mon, 18 Nov 2024 11:47:53 +0100 Subject: [PATCH 2/2] fix(api): use dedicated route to validate security handler --- .../application/security-pre-handlers_test.js | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/api/tests/shared/acceptance/application/security-pre-handlers_test.js b/api/tests/shared/acceptance/application/security-pre-handlers_test.js index 2ce0bc0bf55..831a5497b3c 100644 --- a/api/tests/shared/acceptance/application/security-pre-handlers_test.js +++ b/api/tests/shared/acceptance/application/security-pre-handlers_test.js @@ -44,11 +44,26 @@ describe('Acceptance | Application | SecurityPreHandlers', function () { }); describe('#checkRequestedUserIsAuthenticatedUser', function () { + beforeEach(async function () { + server.route({ + method: 'GET', + path: '/test_route/{userId}', + handler: (r, h) => h.response({}).code(200), + config: { + pre: [ + { + method: securityPreHandlers.checkRequestedUserIsAuthenticatedUser, + }, + ], + }, + }); + }); + it('should return a well formed JSON API error when user in query params is not the same as authenticated', async function () { // given const options = { method: 'GET', - url: '/api/users/{id}/trainings', + url: '/test_route/3', headers: { authorization: generateValidRequestAuthorizationHeader(2) }, };