Refactor code structure for improved readability and maintainability

apps/edr-freight-api/src/modules/rule-engine/services/rate-change-requests.service.spec.ts

@@ -33,6 +33,9 @@ describe('RateChangeRequestsService', () => {
     rate?: Rate;
     pending?: RateChangeRequest | null;
     applyThrows?: Error;
+    /** Columns buildUpdate would derive beyond the literal patch (e.g. rateType). */
+    derived?: Partial<Rate>;
+    previewThrows?: Error;
   } = {}) => {
     const rate = opts.rate ?? liveRate();
     const saved: RateChangeRequest[] = [];
@@ -53,6 +56,12 @@ describe('RateChangeRequestsService', () => {
     const rates = {
       findById: jest.fn(async () => rate),
       assertUpdateValid: jest.fn(async () => undefined),
+      // Stands in for buildUpdate: it resolves a patch into the full column
+      // set, including columns the form never posts (rateType and friends).
+      previewUpdate: jest.fn(async (_id: string, dto: Record<string, unknown>) => {
+        if (opts.previewThrows) throw opts.previewThrows;
+        return { ...dto, ...(opts.derived ?? {}) } as Partial<Rate>;
+      }),
       applyApprovedUpdate: jest.fn(async () => {
         if (opts.applyThrows) throw opts.applyThrows;
         return rate;
@@ -155,14 +164,48 @@ describe('RateChangeRequestsService', () => {
     });
 
     it('validates up front so the requester hears about a bad patch, not the approver', async () => {
-      const { service, rates } = build();
-      rates.assertUpdateValid.mockRejectedValueOnce(
-        new BadRequestException('Rate unit "PER_TON" is not valid for this rate.'),
-      );
+      // Resolving the patch IS the validation — buildUpdate throws on a bad
+      // unit, so previewUpdate surfaces it at submit time.
+      const { service } = build({
+        previewThrows: new BadRequestException('Rate unit "PER_TON" is not valid for this rate.'),
+      });
       await expect(
         service.submit({ rateId: 'rate-1', update: { rateUnit: 'PER_TON' } }),
       ).rejects.toThrow(/not valid for this rate/);
     });
+
+    it('shows the approver a bulk switch, which only exists as a derived column', async () => {
+      // The form posts intercityKind: BULK — never stored. The real edit lands
+      // on rateType (+ the cargo/container swap), so that is what the approver
+      // must see. Diffing the raw patch showed an empty change list.
+      const { service } = build({
+        rate: liveRate({
+          rateType: 'INTERCITY_CONTAINER',
+          appliesTo: 'INTERCITY',
+          containerTypeId: 'ct-1',
+        }),
+        derived: {
+          rateType: 'INTERCITY_BULK',
+          containerTypeId: null,
+          cargoTypeId: 'cargo-9',
+        } as Partial<Rate>,
+      });
+
+      const request = await service.submit({
+        rateId: 'rate-1',
+        update: { intercityKind: 'BULK', cargoTypeId: 'cargo-9' } as never,
+      });
+
+      expect(request.payload).toMatchObject({
+        rateType: 'INTERCITY_BULK',
+        containerTypeId: null,
+        cargoTypeId: 'cargo-9',
+      });
+      expect(request.previousValues).toMatchObject({
+        rateType: 'INTERCITY_CONTAINER',
+        containerTypeId: 'ct-1',
+      });
+    });
   });
 
   describe('approve', () => {

apps/edr-freight-api/src/modules/rule-engine/services/rate-change-requests.service.ts

@@ -24,7 +24,14 @@ import { FREIGHT_PERMS } from '../../../seed/freight-permissions.registry';
 /** Backoffice page where both the queue and the rates live. */
 const RATES_LINK = '/dashboard/rules/rates';
 
-/** Fields a change request may carry — anything else in the patch is ignored. */
+/**
+ * Persisted columns an approver is shown a before→after for.
+ *
+ * These are RESOLVED entity columns, not raw form fields: the diff runs
+ * against `RatesService.previewUpdate`, so a change the form expresses through
+ * a non-stored selector still shows up here as the column it actually moves
+ * (a flip to bulk lands on `rateType` + the container/cargo swap).
+ */
 const DIFFABLE_FIELDS = [
   'rateValue',
   'currency',
@@ -34,6 +41,13 @@ const DIFFABLE_FIELDS = [
   'tradeDirection',
   'containerTypeId',
   'cargoTypeId',
+  // The container-vs-bulk shape of the rate. Missing here, switching a LIVE
+  // rate to bulk showed the approver an empty change list — the only column
+  // that records the kind is rateType, and the form never posts it directly.
+  'rateType',
+  // Line-scoped pricing. Missing here, moving a rate onto (or off) a shipping
+  // line diffed to nothing.
+  'shippingLineCompanyId',
   // The leg a route-scoped rate prices. Missing here, a re-routed LIVE rate
   // diffed to nothing and the submit was refused as "nothing changed".
   'originYardId',
@@ -82,7 +96,11 @@ export class RateChangeRequestsService {
       );
     }
 
-    const payload = this.changedFieldsOnly(rate, dto.update);
+    // Diff the RESOLVED columns, not the raw patch: the form's cargoKind /
+    // intercityKind selectors are never stored, so a bulk switch only shows up
+    // once the patch is resolved into the columns it moves.
+    const resolved = await this.rates.previewUpdate(dto.rateId, dto.update as UpdateRateDto);
+    const payload = this.changedFieldsOnly(rate, resolved);
     if (Object.keys(payload).length === 0) {
       throw new BadRequestException('Nothing changed — the proposed values match the live rate.');
     }
@@ -98,7 +116,9 @@ export class RateChangeRequestsService {
       );
     }
 
-    await this.rates.assertUpdateValid(dto.rateId, payload as UpdateRateDto);
+    // previewUpdate above already ran the full validation (it IS buildUpdate),
+    // so re-validating here would only repeat it — and the trimmed payload is
+    // resolved columns, not a form patch, so it is not the right input for it.
 
     const request = await this.repo.save(
       this.repo.create({
@@ -186,13 +206,14 @@ export class RateChangeRequestsService {
   }
 
   /**
-   * Keep only fields the requester actually changed. A form posts every field
-   * back, so without this the diff would list untouched values as changes.
+   * Keep only columns the edit actually moves. `buildUpdate` returns a full
+   * resolved column set (it re-derives scope on every patch), so without this
+   * the diff would list every untouched column as a change.
    */
-  private changedFieldsOnly(rate: Rate, update: UpdateRateDto): Record<string, unknown> {
+  private changedFieldsOnly(rate: Rate, resolved: Partial<Rate>): Record<string, unknown> {
     const patch: Record<string, unknown> = {};
     for (const field of DIFFABLE_FIELDS) {
-      const proposed = (update as Record<string, unknown>)[field];
+      const proposed = (resolved as Record<string, unknown>)[field];
       if (proposed === undefined) continue;
       if (this.sameValue(proposed, (rate as unknown as Record<string, unknown>)[field])) continue;
       patch[field] = proposed;

apps/edr-freight-api/src/modules/rule-engine/services/rates.service.ts

@@ -753,6 +753,19 @@ export class RatesService {
     await this.buildUpdate(await this.findById(id), dto);
   }
 
+  /**
+   * The exact column changes applying this patch would make, without writing.
+   *
+   * A change request diffs against THIS rather than the raw patch: the form
+   * posts selectors that are never stored (`cargoKind`, `intercityKind`), and
+   * the real edit they encode lands on derived columns — flipping a rate to
+   * bulk moves `rateType` and swaps `containerTypeId`/`cargoTypeId`. Diffing
+   * the raw patch missed all of it, so the approver saw an empty change list.
+   */
+  async previewUpdate(id: string, dto: UpdateRateDto): Promise<Partial<Rate>> {
+    return this.buildUpdate(await this.findById(id), dto);
+  }
+
   private async applyUpdate(existing: Rate, dto: UpdateRateDto): Promise<Rate> {
     const updates = await this.buildUpdate(existing, dto);
     const updated = await this.repository.update(existing.id, updates);