From 2fc7514f0f54f9795ed253fc04f962a9c661432d Mon Sep 17 00:00:00 2001 From: Fred <98240+farnoux@users.noreply.github.com> Date: Wed, 11 Dec 2024 09:14:43 +0100 Subject: [PATCH] fiche-action: bulk edit for `libreTags` --- .../bulk-edit/bulk-edit.router.e2e-spec.ts | 87 +++++++++++++++ .../src/fiches/bulk-edit/bulk-edit.service.ts | 39 ++++++- .../models/update-fiche-action.request.ts | 11 +- .../services/fiches-action-update.service.ts | 102 +----------------- .../fiches/fiche-action-update.e2e-spec.ts | 59 +++------- 5 files changed, 146 insertions(+), 152 deletions(-) diff --git a/backend/src/fiches/bulk-edit/bulk-edit.router.e2e-spec.ts b/backend/src/fiches/bulk-edit/bulk-edit.router.e2e-spec.ts index 49b541bb602..8608f587518 100644 --- a/backend/src/fiches/bulk-edit/bulk-edit.router.e2e-spec.ts +++ b/backend/src/fiches/bulk-edit/bulk-edit.router.e2e-spec.ts @@ -9,7 +9,9 @@ import { getTestRouter, } from '../../../test/common/app-utils'; import { AuthenticatedUser } from '../../auth/models/auth.models'; +import { libreTagTable } from '../../taxonomie/models/libre-tag.table'; import { AppRouter, TrpcRouter } from '../../trpc/trpc.router'; +import { ficheActionLibreTagTable } from '../models/fiche-action-libre-tag.table'; import { ficheActionPiloteTable } from '../models/fiche-action-pilote.table'; import { FicheActionStatutsEnumType, @@ -64,6 +66,17 @@ describe('BulkEditRouter', () => { .groupBy(ficheActionPiloteTable.ficheId); } + function getFichesWithLibreTags(ficheIds: number[]) { + return db.db + .select({ + ficheId: ficheActionLibreTagTable.ficheId, + libreTagIds: sql`array_remove(array_agg(${ficheActionLibreTagTable.libreTagId}), NULL)`, + }) + .from(ficheActionLibreTagTable) + .where(inArray(ficheActionLibreTagTable.ficheId, ficheIds)) + .groupBy(ficheActionLibreTagTable.ficheId); + } + beforeAll(async () => { const app = await getTestApp(); router = await getTestRouter(app); @@ -167,6 +180,80 @@ describe('BulkEditRouter', () => { }); }); + test('authenticated, bulk edit `libreTags`', async () => { + function createLibreTagIds() { + return db.db + .insert(libreTagTable) + .values([ + { + collectiviteId: COLLECTIVITE_ID, + nom: 'tag-1', + }, + { + collectiviteId: COLLECTIVITE_ID, + nom: 'tag-2', + }, + ]) + .returning() + .then((tags) => tags.map((tag) => tag.id)); + } + + const caller = router.createCaller({ user: yoloDodo }); + const tagIds = await createLibreTagIds(); + + const input = { + ficheIds, + libreTags: { + add: [{ id: tagIds[0] }], + }, + } satisfies Input; + + const result = await caller.plans.fiches.bulkEdit(input); + expect(result).toBeUndefined(); + + // Verify that all fiches have been updated with libreTags + const fiches = await getFichesWithLibreTags(ficheIds); + + for (const fiche of fiches) { + expect(fiche.libreTagIds).toContain(input.libreTags.add[0].id); + } + + // Add again the same libreTags to check there is no conflict error + await caller.plans.fiches.bulkEdit(input); + expect(result).toBeUndefined(); + + // Remove one pilote and add another one + const input2 = { + ficheIds, + libreTags: { + add: [{ id: tagIds[1] }], + remove: [{ id: input.libreTags.add[0].id }], + }, + } satisfies Input; + + await caller.plans.fiches.bulkEdit(input2); + expect(result).toBeUndefined(); + + // Verify that all fiches have been updated with libreTags + const updatedFiches = await getFichesWithLibreTags(ficheIds); + + for (const fiche of updatedFiches) { + expect(fiche.libreTagIds).toContain(input2.libreTags.add[0].id); + expect(fiche.libreTagIds).not.toContain(input.libreTags.add[0].id); + } + + // Delete inserted or existing pilotes after test + onTestFinished(async () => { + await db.db + .delete(ficheActionLibreTagTable) + .where(inArray(ficheActionLibreTagTable.ficheId, ficheIds)); + + await db.db + .delete(libreTagTable) + .where(inArray(libreTagTable.id, tagIds)); + }); + }); + test('authenticated, bulk edit `priorite`', async () => { const caller = router.createCaller({ user: yoloDodo }); diff --git a/backend/src/fiches/bulk-edit/bulk-edit.service.ts b/backend/src/fiches/bulk-edit/bulk-edit.service.ts index f4b0e8050a4..ff5a8b39b1b 100644 --- a/backend/src/fiches/bulk-edit/bulk-edit.service.ts +++ b/backend/src/fiches/bulk-edit/bulk-edit.service.ts @@ -5,6 +5,7 @@ import { and, inArray, or } from 'drizzle-orm'; import z from 'zod'; import { AuthService } from '../../auth/services/auth.service'; import DatabaseService from '../../common/services/database.service'; +import { ficheActionLibreTagTable } from '../models/fiche-action-libre-tag.table'; import { ficheActionPiloteTable } from '../models/fiche-action-pilote.table'; import { ficheActionSchema, @@ -28,9 +29,13 @@ export class BulkEditService { dateFin: ficheActionSchema.shape.dateFin.optional(), ameliorationContinue: ficheActionSchema.shape.ameliorationContinue.optional(), + pilotes: listSchema( updateFicheActionRequestSchema.shape.pilotes.unwrap().unwrap() ), + libreTags: listSchema( + updateFicheActionRequestSchema.shape.libresTag.unwrap().unwrap() + ), }); async bulkEdit( @@ -52,7 +57,7 @@ export class BulkEditService { NiveauAcces.EDITION ); - const { pilotes, ...plainValues } = params; + const { pilotes, libreTags, ...plainValues } = params; await this.db.transaction(async (tx) => { // Update plain values @@ -63,7 +68,7 @@ export class BulkEditService { .where(inArray(ficheActionTable.id, ficheIds)); } - // Update external relations + // Update external relation `pilotes` if (pilotes !== undefined) { if (pilotes.add?.length) { const values = ficheIds.flatMap((ficheId) => { @@ -101,6 +106,36 @@ export class BulkEditService { ); } } + + // Update external relation `libreTags` + if (libreTags !== undefined) { + if (libreTags.add?.length) { + const values = ficheIds.flatMap((ficheId) => { + return (libreTags.add ?? []).map((tag) => ({ + ficheId, + libreTagId: tag.id, + })); + }); + + await tx + .insert(ficheActionLibreTagTable) + .values(values) + .onConflictDoNothing(); + } + + if (libreTags.remove?.length) { + const ids = libreTags.remove.map((tag) => tag.id) as number[]; + + await tx + .delete(ficheActionLibreTagTable) + .where( + and( + inArray(ficheActionLibreTagTable.ficheId, ficheIds), + inArray(ficheActionLibreTagTable.libreTagId, ids) + ) + ); + } + } }); } } diff --git a/backend/src/fiches/models/update-fiche-action.request.ts b/backend/src/fiches/models/update-fiche-action.request.ts index 1578a9bbecb..17b1d30776d 100644 --- a/backend/src/fiches/models/update-fiche-action.request.ts +++ b/backend/src/fiches/models/update-fiche-action.request.ts @@ -80,17 +80,10 @@ export const updateFicheActionRequestSchema = updateFicheActionSchema.extend({ actions: actionRelationSchema.pick({ id: true }).array().nullish(), indicateurs: indicateurDefinitionSchema.pick({ id: true }).array().nullish(), services: serviceTagSchema.pick({ id: true }).array().nullish(), - financeurs: z.array(financeurWithMontantSchema).nullish(), + financeurs: financeurWithMontantSchema.array().nullish(), fichesLiees: ficheActionSchema.pick({ id: true }).array().nullish(), resultatsAttendus: effetAttenduSchema.pick({ id: true }).array().nullish(), - libresTag: z - .array( - z.union([ - libreTagSchema.pick({ id: true }), - libreTagSchema.pick({ nom: true }), - ]) - ) - .nullish(), + libresTag: libreTagSchema.pick({ id: true }).array().nullish(), }); export type UpdateFicheActionRequestType = z.infer< diff --git a/backend/src/fiches/services/fiches-action-update.service.ts b/backend/src/fiches/services/fiches-action-update.service.ts index b26775fec13..70947135ebb 100644 --- a/backend/src/fiches/services/fiches-action-update.service.ts +++ b/backend/src/fiches/services/fiches-action-update.service.ts @@ -20,6 +20,7 @@ import { ficheActionAxeTable } from '../models/fiche-action-axe.table'; import { ficheActionEffetAttenduTable } from '../models/fiche-action-effet-attendu.table'; import { ficheActionFinanceurTagTable } from '../models/fiche-action-financeur-tag.table'; import { ficheActionIndicateurTable } from '../models/fiche-action-indicateur.table'; +import { ficheActionLibreTagTable } from '../models/fiche-action-libre-tag.table'; import { ficheActionLienTable } from '../models/fiche-action-lien.table'; import { ficheActionNoteTable, @@ -38,8 +39,6 @@ import { } from '../models/fiche-action.table'; import { UpdateFicheActionRequestType } from '../models/update-fiche-action.request'; import FicheService from './fiche.service'; -import { ficheActionLibreTagTable } from '../models/fiche-action-libre-tag.table'; -import { libreTagTable } from '../../taxonomie/models/libre-tag.table'; type TxType = PgTransaction< PostgresJsQueryResultHKT, @@ -323,40 +322,17 @@ export default class FichesActionUpdateService { } if (libresTag !== undefined) { - const { collectiviteId } = await this.getCollectiviteIdForFiche( - tx, - ficheActionId - ); - - // tagMap = new Map([['Mobility', 1], ['Transport', 2], ['Bicycle', 3]]); - const tagMap = await this.getOrCreateLibreTag( - tx, - libresTag, - collectiviteId - ); - - // Example of libresTagWithResolvedIds: - // libresTagWithResolvedIds = [ - // { id: 1 }, - // { id: 2 }, - // { id: 3 } - // ] - const libresTagWithResolvedIds = this.resolveLibreTagIds( - libresTag, - tagMap - ); - // Delete existing relations await tx .delete(ficheActionLibreTagTable) .where(eq(ficheActionLibreTagTable.ficheId, ficheActionId)); // Insert new relations - if (libresTagWithResolvedIds.length > 0) { + if (libresTag !== null && libresTag.length > 0) { updatedLibresTag = await tx .insert(ficheActionLibreTagTable) .values( - libresTagWithResolvedIds.map((relation) => ({ + libresTag.map((relation) => ({ ficheId: ficheActionId, libreTagId: relation.id, createdBy: tokenInfo.id, @@ -480,78 +456,6 @@ export default class FichesActionUpdateService { })); } - private async getCollectiviteIdForFiche(tx: TxType, ficheActionId: number) { - return await tx - .select({ collectiviteId: ficheActionTable.collectiviteId }) - .from(ficheActionTable) - .where(eq(ficheActionTable.id, ficheActionId)) - .then((rows: { collectiviteId: number }[]) => rows[0]); - } - - /** - * Create or get libre tags - * @param tx - current database transaction - * @param libresTag - array of libre tags to create or get - * @param collectiviteId - id of the collectivite - * @returns a map of name -> id for the newly created or existing libre tags - */ - private async getOrCreateLibreTag( - tx: TxType, - libresTag: { id?: number; nom?: string }[] | null, - collectiviteId: number - ): Promise> { - const tagMap = new Map(); - - if (!libresTag) { - return tagMap; - } - - for (const tag of libresTag) { - if (this.isNewTag(tag)) { - const existingTag = await this.findExistingLibreTagByName(tx, tag.nom!); - if (existingTag) { - tagMap.set(tag.nom!, existingTag.id); - } else { - const [newTag] = await tx - .insert(libreTagTable) - .values({ - nom: tag.nom!, - collectiviteId: collectiviteId, - }) - .returning(); - tagMap.set(tag.nom!, newTag.id); - } - } - } - return tagMap; - } - - private isNewTag(tag: { id?: number; nom?: string }) { - return !tag.id && tag.nom; - } - - private resolveLibreTagIds( - libresTag: { id?: number; nom?: string }[] | null, - tagMap: Map - ): { id: number }[] { - if (!libresTag) { - return []; - } - return libresTag - .map((tag) => ({ - id: tag.id ?? tagMap.get(tag.nom ?? ''), - })) - .filter((tag): tag is { id: number } => tag.id !== undefined); - } - - private async findExistingLibreTagByName(tx: TxType, nom: string) { - return tx - .select() - .from(libreTagTable) - .where(eq(libreTagTable.nom, nom)) - .then((rows: { id: number; nom: string }[]) => rows[0]); - } - /** Insère ou met à jour des notes de suivi */ async upsertNotes( ficheId: number, diff --git a/backend/test/fiches/fiche-action-update.e2e-spec.ts b/backend/test/fiches/fiche-action-update.e2e-spec.ts index 17fb6c10728..c82be473944 100644 --- a/backend/test/fiches/fiche-action-update.e2e-spec.ts +++ b/backend/test/fiches/fiche-action-update.e2e-spec.ts @@ -1,22 +1,15 @@ import { INestApplication } from '@nestjs/common'; -import { eq, or } from 'drizzle-orm'; +import { eq } from 'drizzle-orm'; import { default as request } from 'supertest'; import { describe, expect, it } from 'vitest'; import DatabaseService from '../../src/common/services/database.service'; import { UpdateFicheActionRequestClass } from '../../src/fiches/controllers/fiches-action.controller'; -import { - FicheActionCiblesEnumType, - piliersEciEnumType, - FicheActionStatutsEnumType, - ficheActionTable, -} from '../../src/fiches/models/fiche-action.table'; -import { ficheActionFixture } from './fixtures/fiche-action.fixture'; -import { ficheActionAxeTable } from '../../src/fiches/models/fiche-action-axe.table'; -import { ficheActionThematiqueTable } from '../../src/fiches/models/fiche-action-thematique.table'; import { ficheActionActionTable } from '../../src/fiches/models/fiche-action-action.table'; +import { ficheActionAxeTable } from '../../src/fiches/models/fiche-action-axe.table'; import { ficheActionEffetAttenduTable } from '../../src/fiches/models/fiche-action-effet-attendu.table'; import { ficheActionFinanceurTagTable } from '../../src/fiches/models/fiche-action-financeur-tag.table'; import { ficheActionIndicateurTable } from '../../src/fiches/models/fiche-action-indicateur.table'; +import { ficheActionLibreTagTable } from '../../src/fiches/models/fiche-action-libre-tag.table'; import { ficheActionLienTable } from '../../src/fiches/models/fiche-action-lien.table'; import { ficheActionPartenaireTagTable } from '../../src/fiches/models/fiche-action-partenaire-tag.table'; import { ficheActionPiloteTable } from '../../src/fiches/models/fiche-action-pilote.table'; @@ -24,8 +17,15 @@ import { ficheActionReferentTable } from '../../src/fiches/models/fiche-action-r import { ficheActionServiceTagTable } from '../../src/fiches/models/fiche-action-service.table'; import { ficheActionSousThematiqueTable } from '../../src/fiches/models/fiche-action-sous-thematique.table'; import { ficheActionStructureTagTable } from '../../src/fiches/models/fiche-action-structure-tag.table'; +import { ficheActionThematiqueTable } from '../../src/fiches/models/fiche-action-thematique.table'; +import { + FicheActionCiblesEnumType, + FicheActionStatutsEnumType, + ficheActionTable, + piliersEciEnumType, +} from '../../src/fiches/models/fiche-action.table'; +import { libreTagTable } from '../../src/taxonomie/models/libre-tag.table'; import { getAuthToken } from '../auth/auth-utils'; -import { ficheActionLibreTagTable } from '../../src/fiches/models/fiche-action-libre-tag.table'; import { getTestApp } from '../common/app-utils'; import { UpdateFicheActionRequestType } from './../../src/fiches/models/update-fiche-action.request'; import { @@ -34,6 +34,7 @@ import { fichesLieesFixture, financeursFixture, indicateursFixture, + libresFixture, partenairesFixture, pilotesFixture, referentsFixture, @@ -42,9 +43,8 @@ import { sousThematiquesFixture, structuresFixture, thematiquesFixture, - libresFixture, } from './fixtures/fiche-action-relations.fixture'; -import { libreTagTable } from '../../src/taxonomie/models/libre-tag.table'; +import { ficheActionFixture } from './fixtures/fiche-action.fixture'; const collectiviteId = 1; const ficheActionId = 9999; @@ -245,7 +245,6 @@ describe('FichesActionUpdateService', () => { }); describe('Update relations', () => { - it('should update the axes relations in the database', async () => { const data: UpdateFicheActionRequestClass = { axes: [{ id: 1 }, { id: 2 }], @@ -502,9 +501,9 @@ describe('FichesActionUpdateService', () => { it('should update libre tag relations in the database when an existing tag is added', async () => { // Setup test data - await databaseService.db.insert(libreTagTable).values([ - { id: 2, nom: 'Tag 2', collectiviteId: collectiviteId }, - ]); + await databaseService.db + .insert(libreTagTable) + .values([{ id: 2, nom: 'Tag 2', collectiviteId: collectiviteId }]); const data: UpdateFicheActionRequestClass = { libresTag: [{ id: 1 }, { id: 2 }], @@ -529,30 +528,6 @@ describe('FichesActionUpdateService', () => { await cleanupLibreTags(); }); }); - - it('should create and link new libre tags to fiche action', async () => { - const nom = 'My new tag'; - const data: UpdateFicheActionRequestClass = { - libresTag: [{ nom }], - }; - - const response = await putRequest(data); - const body = response.body; - - const newLibreTag = await databaseService.db - .select() - .from(libreTagTable) - .where(eq(libreTagTable.nom, nom)) - .then((rows) => rows[0]); - - expect(newLibreTag.collectiviteId).toBe(collectiviteId); - expect(body.libresTag).toContainEqual( - expect.objectContaining({ - ficheId: ficheActionId, - libreTagId: newLibreTag.id, - }) - ); - }); }); describe('Access Rights', () => { @@ -705,7 +680,7 @@ describe('FichesActionUpdateService', () => { id: 1, nom: 'Tag 1', collectiviteId: collectiviteId, - } + }, ]); await databaseService.db.insert(ficheActionLibreTagTable).values({