From 9428a983f5b35e7434695b56e80c5e207fd50038 Mon Sep 17 00:00:00 2001 From: shancheas Date: Tue, 1 Sep 2026 19:25:19 +0700 Subject: [PATCH] Refactor PlansRepository to optimize data loading and enhance error handling - Replaced the previous hydration logic with a new method, `loadChildrenForRows`, to efficiently load related data for plans in a single query. - Implemented a `groupByPlanId` utility to organize related entities by plan ID, improving data retrieval performance. - Updated error handling in the `getPlan` method to throw a `NotFoundException` if a plan is not found. - Modified end-to-end tests to validate the new structure of the response, ensuring that related destinations are correctly included in the API output. --- src/modules/field/plans/plans.repository.ts | 73 ++++++++++++++++----- test/plans.e2e-spec.ts | 23 +++++-- 2 files changed, 74 insertions(+), 22 deletions(-) diff --git a/src/modules/field/plans/plans.repository.ts b/src/modules/field/plans/plans.repository.ts index 9ad30b2..23e0a20 100644 --- a/src/modules/field/plans/plans.repository.ts +++ b/src/modules/field/plans/plans.repository.ts @@ -79,9 +79,7 @@ export class PlansRepository { .limit(filters.limit) .offset(filters.offset); return { - data: await this.hydrate( - rows.map((row) => this.toDomain(row, [], [], [])), - ), + data: await this.loadChildrenForRows(this.db, rows), total: Number(totalRows[0]?.total ?? 0), }; } @@ -340,20 +338,61 @@ export class PlansRepository { executor: QueryExecutor, row: PlanRow, ): Promise { - const destinations = await executor - .select() - .from(planDestinations) - .where(eq(planDestinations.planId, row.id)) - .orderBy(asc(planDestinations.sortOrder)); - const invoices = await executor - .select() - .from(planInvoices) - .where(eq(planInvoices.planId, row.id)); - const packingSlips = await executor - .select() - .from(planPackingSlips) - .where(eq(planPackingSlips.planId, row.id)); - return this.hydrateOne(row, destinations, invoices, packingSlips); + const [plan] = await this.loadChildrenForRows(executor, [row]); + if (!plan) { + throw new NotFoundException('Plan not found'); + } + return plan; + } + + private async loadChildrenForRows( + executor: QueryExecutor, + rows: PlanRow[], + ): Promise { + if (rows.length === 0) { + return []; + } + const ids = rows.map((row) => row.id); + const [destinationRows, invoiceRows, packingSlipRows] = await Promise.all([ + executor + .select() + .from(planDestinations) + .where(inArray(planDestinations.planId, ids)) + .orderBy(asc(planDestinations.sortOrder)), + executor + .select() + .from(planInvoices) + .where(inArray(planInvoices.planId, ids)), + executor + .select() + .from(planPackingSlips) + .where(inArray(planPackingSlips.planId, ids)), + ]); + const destinationsByPlan = this.groupByPlanId(destinationRows); + const invoicesByPlan = this.groupByPlanId(invoiceRows); + const packingByPlan = this.groupByPlanId(packingSlipRows); + return this.hydrate( + rows.map((row) => + this.toDomain( + row, + destinationsByPlan.get(row.id) ?? [], + invoicesByPlan.get(row.id) ?? [], + packingByPlan.get(row.id) ?? [], + ), + ), + ); + } + + private groupByPlanId( + rows: T[], + ): Map { + const ids = [...new Set(rows.map((row) => row.planId))]; + return new Map( + ids.map((planId) => [ + planId, + rows.filter((row) => row.planId === planId), + ]), + ); } private async replaceChildren( diff --git a/test/plans.e2e-spec.ts b/test/plans.e2e-spec.ts index 0bfc528..031410e 100644 --- a/test/plans.e2e-spec.ts +++ b/test/plans.e2e-spec.ts @@ -251,7 +251,17 @@ describe('Plans (e2e)', () => { .query({ employeeId, purpose: 'sales', date: from }) .set('Authorization', `Bearer ${adminAccessToken}`) .expect(200); - const planId = (list.body.data as Array<{ id: string }>)[0].id; + const listed = ( + list.body as { + data: Array<{ + id: string; + destinations: Array<{ customer: { id: string } | null }>; + }>; + } + ).data; + expect(listed[0].destinations).toHaveLength(1); + expect(listed[0].destinations[0].customer?.id).toBe(customerId); + const planId = listed[0].id; const added = await request(app.getHttpServer()) .post(`/plans/${planId}/destinations`) @@ -259,13 +269,16 @@ describe('Plans (e2e)', () => { .send({ customerId: extraCustomerId }) .expect(201); expect( - (added.body as { destinations: Array<{ customerId: string }> }) - .destinations, + (added.body as { destinations: unknown[] }).destinations, ).toHaveLength(2); const extraDest = ( - added.body as { destinations: Array<{ id: string; customerId: string }> } - ).destinations.find((d) => d.customerId === extraCustomerId); + added.body as { + destinations: Array<{ id: string; customer: { id: string } | null }>; + } + ).destinations.find( + (destination) => destination.customer?.id === extraCustomerId, + ); const removed = await request(app.getHttpServer()) .delete(`/plans/${planId}/destinations/${extraDest?.id}`) .set('Authorization', `Bearer ${adminAccessToken}`)