add e2e test

apps/edr-freight-api/src/common/freight-permission.util.spec.ts

@@ -0,0 +1,51 @@
+import {
+  assertCanApproveContractStep,
+  canEditContractStep,
+} from './freight-permission.util';
+import { FREIGHT_PERMS } from '../seed/freight-permissions.registry';
+
+// The document-edit gate (canEditContractStep) must be STRICT: only the approver
+// whose turn it is may edit. This is the fix for a previous approver keeping the
+// "Edit contract articles" button after acting, because the approve gate lets
+// through anyone holding any contract-approve permission.
+describe('canEditContractStep (strict per-step edit gate)', () => {
+  const director = {
+    employee: { position: { positionType: { key: '-marketing-director-' } } },
+  };
+  // A line staff who already approved their own step but still holds a
+  // contract-approve permission — the exact actor that leaked edit rights.
+  const officerWithApprovePerm = {
+    employee: {
+      position: {
+        positionType: { key: '-marketing-officer-' },
+        permissions: [{ key: FREIGHT_PERMS.contracts.approveLineStaff }],
+      },
+    },
+  };
+  const superAdmin = { roles: [{ key: 'super_admin' }] };
+
+  it('lets the step’s own approver edit', () => {
+    expect(canEditContractStep(director, '-marketing-director-')).toBe(true);
+  });
+
+  it('lets an approval admin edit any step', () => {
+    expect(canEditContractStep(superAdmin, '-marketing-director-')).toBe(true);
+  });
+
+  it('does NOT let a different approver edit just because they hold an approve permission', () => {
+    expect(canEditContractStep(officerWithApprovePerm, '-marketing-director-')).toBe(
+      false,
+    );
+  });
+
+  it('stays intentionally stricter than the approve gate (which keeps the blanket fallback)', () => {
+    // The approve gate passes the officer via the any-permission blanket…
+    expect(() =>
+      assertCanApproveContractStep(officerWithApprovePerm, '-marketing-director-'),
+    ).not.toThrow();
+    // …but the edit gate does not — that divergence IS the fix.
+    expect(canEditContractStep(officerWithApprovePerm, '-marketing-director-')).toBe(
+      false,
+    );
+  });
+});

apps/edr-freight-api/src/common/freight-permission.util.ts

@@ -201,6 +201,34 @@ export function assertCanApproveContractStep(
   );
 }
 
+/**
+ * Strict "is it exactly this caller's turn?" test — mirrors the backoffice
+ * `canApproveContractStep`. Same passes as {@link assertCanApproveContractStep}
+ * EXCEPT the blanket "holds any contract-approve permission" fallback is
+ * dropped: a line-staff holding `approveLineStaff` must NOT read as the director
+ * for a director step. Used to gate contract-document editing so approval hands
+ * edit rights to the NEXT approver only — a previous approver who already acted
+ * (but still holds an approve permission) loses the edit button, as required.
+ *
+ * (Kept separate from the approve/reject gate, which keeps the blanket fallback
+ * so delegates whose token omits a position type can still action their step.)
+ */
+export function canEditContractStep(
+  user: TCurrentUser | MeLikeUser | null | undefined,
+  requiredRole: string,
+): boolean {
+  if (isFreightApprovalAdmin(user)) return true;
+
+  const positionTypes = collectPositionTypeKeys(user);
+  if (positionTypes.includes(requiredRole)) return true;
+
+  const aliases = LEGACY_ROLE_POSITION_TYPES[requiredRole] ?? [];
+  if (aliases.some((alias) => positionTypes.includes(alias))) return true;
+
+  const legacyPermission = CONTRACT_APPROVE_ROLE_PERMISSION[requiredRole];
+  return Boolean(legacyPermission && hasFreightPermission(user, legacyPermission));
+}
+
 export function assertCanApproveBookingStep(
   user: TCurrentUser | MeLikeUser | null | undefined,
   requiredRole: string,

apps/edr-freight-api/src/modules/contracts/contract-transition.service.ts

@@ -18,7 +18,10 @@ import { ContractPdfService } from '../../contracts/contract-pdf.service';
 import { ContractViewModel } from '../../contracts/contract-view-model.builder';
 import { MinioService } from '../minio/minio.service';
 import { FileRecord } from '../files/entities/file.entity';
-import { assertCanApproveContractStep } from '../../common/freight-permission.util';
+import {
+  assertCanApproveContractStep,
+  canEditContractStep,
+} from '../../common/freight-permission.util';
 import { ContractDocumentHistoryService } from './contract-document-history.service';
 import { ApprovalRulesService } from '../rule-engine/services/approval-rules.service';
 import { CargoTypesService } from '../rule-engine/services/cargo-types.service';
@@ -431,12 +434,11 @@ export class ContractTransitionService {
     if (!next) return false;
     if (!user) return false;
 
-    try {
-      assertCanApproveContractStep(user, next.requiredRole);
-      return true;
-    } catch {
-      return false;
-    }
+    // Strict match: ONLY the approver whose turn it is (the next pending step's
+    // role) may edit. Using the looser approve gate here let any approver who
+    // held a contract-approve permission keep the edit button after acting —
+    // approval must hand edit rights to the next approver, not share them.
+    return canEditContractStep(user, next.requiredRole);
   }
 
   /** The role that currently holds editing rights, for UI messaging. */

apps/edr-freight-api/src/modules/locomotives/locomotives.controller.ts

@@ -1,7 +1,7 @@
 import { Body, Controller, Get, Param, ParseUUIDPipe, Patch, Post, Query } from '@nestjs/common';
 import { ApiBearerAuth, ApiOperation, ApiTags } from '@nestjs/swagger';
 
-import { FleetManage, FleetView } from '../../common/booking-guards';
+import { FleetManage, StaffReference } from '../../common/booking-guards';
 import { CreateLocomotiveDto } from './dto/create-locomotive.dto';
 import { FilterLocomotivesDto } from './dto/filter-locomotives.dto';
 import { UpdateLocomotiveDto } from './dto/update-locomotive.dto';
@@ -9,18 +9,22 @@ import { LocomotivesService } from './locomotives.service';
 
 @ApiTags('locomotives')
 @ApiBearerAuth()
+// No class-level guard: reads are login-only reference data (any staff can
+// fetch a locomotive for a cross-flow view without the fleet:view that drives
+// the Fleet sidebar). Every mutation carries its own @FleetManage().
 @Controller('locomotives')
-@FleetView()
 export class LocomotivesController {
   constructor(private readonly locomotivesService: LocomotivesService) {}
 
   @Get()
+  @StaffReference()
   @ApiOperation({ summary: 'List locomotives' })
   findAll(@Query() filter: FilterLocomotivesDto) {
     return this.locomotivesService.findAll(filter);
   }
 
   @Get(':id')
+  @StaffReference()
   @ApiOperation({ summary: 'Get a locomotive by ID' })
   findOne(@Param('id', ParseUUIDPipe) id: string) {
     return this.locomotivesService.findById(id);

apps/edr-freight-api/src/modules/wagons/wagons.controller.ts

@@ -12,7 +12,7 @@ import {
 import { ApiOperation, ApiTags } from '@nestjs/swagger';
 import { CurrentUser } from '@edr/api-common';
 import type { TCurrentUser } from '@tria-plc/api-common/modules/auth/types/current-user.type';
-import { FleetManage, FleetView } from '../../common/booking-guards';
+import { FleetManage, FleetView, StaffReference } from '../../common/booking-guards';
 import { CreateWagonDto } from './dto/create-wagon.dto';
 import { ListWagonsQueryDto } from './dto/list-wagons-query.dto';
 import { UpdateWagonDto } from './dto/update-wagon.dto';
@@ -23,8 +23,10 @@ import { BulkSetWagonStatusDto } from './dto/bulk-set-wagon-status.dto';
 import { WagonsService } from './wagons.service';
 
 @ApiTags('wagons')
+// No class-level guard: reads (list, by-id, movements) are login-only reference
+// data — any staff can fetch wagon data for a cross-flow view without the
+// fleet:view that drives the Fleet sidebar. Every mutation has its @FleetManage().
 @Controller('wagons')
-@FleetView()
 export class WagonsController {
   constructor(private readonly wagonsService: WagonsService) {}
 
@@ -36,18 +38,21 @@ export class WagonsController {
   }
 
   @Get()
+  @StaffReference()
   @ApiOperation({ summary: 'List all wagons' })
   findAll(@Query() query: ListWagonsQueryDto) {
     return this.wagonsService.findAll(query);
   }
 
   @Get(':id')
+  @StaffReference()
   @ApiOperation({ summary: 'Get a wagon by ID' })
   findOne(@Param('id', ParseUUIDPipe) id: string) {
     return this.wagonsService.findById(id);
   }
 
   @Get(':id/movements')
+  @StaffReference()
   @ApiOperation({
     summary: "Wagon movement ledger (loaded legs, empty repositions, manual moves), newest first",
   })