Merge pull request #1339 from Tria-plc/freight_feature/usermanagement

feat: implement consolidation approval process for shared-wagon bookings

apps/edr-freight-api/src/modules/bookings/consolidation-approval.service.spec.ts

@@ -11,9 +11,8 @@ import { Booking } from './entities/booking.entity';
  * straight to Operations.
  *
  * The invariants that matter: both halves are held and released TOGETHER (a
- * decision on one side of a shared wagon is meaningless without the other), the
- * requester cannot approve their own pairing, and a decided pairing cannot be
- * decided twice.
+ * decision on one side of a shared wagon is meaningless without the other), and
+ * a decided pairing cannot be decided twice.
  */
 describe('ConsolidationApprovalService', () => {
   const PENDING = {
@@ -154,13 +153,19 @@ describe('ConsolidationApprovalService', () => {
     });
   });
 
-  it('refuses to let the requester approve their own pairing', async () => {
-    const { service, bookingsRepository } = makeService();
+  it('lets the requester approve their own pairing', async () => {
+    // No maker-checker separation: the permission alone decides who may approve,
+    // and the audit trail still records requester and approver separately.
+    const { service, approvals } = makeService();
 
-    await expect(
-      service.approve('ap-1', 'gl-user'),
-    ).rejects.toThrow(/must be approved by someone else/i);
-    expect(bookingsRepository.update).not.toHaveBeenCalled();
+    await service.approve('ap-1', 'gl-user');
+
+    expect(approvals.decide).toHaveBeenCalledWith(
+      'ap-1',
+      ConsolidationApprovalStatus.Approved,
+      'gl-user',
+      undefined,
+    );
   });
 
   it('requires a reason to reject', async () => {

apps/edr-freight-api/src/modules/bookings/consolidation-approval.service.ts

@@ -37,6 +37,10 @@ export const CONSOLIDATION_APPROVAL_PENDING = "CONSOLIDATION_APPROVAL_PENDING";
  * decision on one is meaningless without the other. Every request is kept,
  * decided or not: the table is the audit trail of who approved which pairing,
  * when, and why.
+ *
+ * No maker-checker separation: whoever holds the approve permission may decide a
+ * pairing, including the GL user who created it. The record of who requested and
+ * who decided is still kept either way.
  */
 @Injectable()
 export class ConsolidationApprovalService {
@@ -114,7 +118,6 @@ export class ConsolidationApprovalService {
     note?: string,
   ): Promise<{ booking: Booking; partner: Booking }> {
     const approval = await this.loadPending(approvalId);
-    this.assertDifferentPerson(approval, decidedBy);
 
     await this.dataSource.transaction(async () => {
       const claimed = await this.approvals.decide(
@@ -166,7 +169,6 @@ export class ConsolidationApprovalService {
       );
     }
     const approval = await this.loadPending(approvalId);
-    this.assertDifferentPerson(approval, decidedBy);
 
     await this.dataSource.transaction(async () => {
       const claimed = await this.approvals.decide(
@@ -237,19 +239,4 @@ export class ConsolidationApprovalService {
     }
     return approval;
   }
-
-  /**
-   * Maker–checker: the point of the gate is a second pair of eyes, so the GL
-   * user who created the pairing cannot also approve it.
-   */
-  private assertDifferentPerson(
-    approval: ConsolidationApproval,
-    decidedBy: string,
-  ): void {
-    if (approval.requestedBy && approval.requestedBy === decidedBy) {
-      throw new BadRequestException(
-        "You created this consolidation — it must be approved by someone else.",
-      );
-    }
-  }
 }