Assist_Design/docs/refactoring/orchestrator-refactoring-plan.md

# Orchestrator Refactoring Plan

## Overview

This document outlines the refactoring plan for BFF orchestrators to align with enterprise-grade code structure patterns used by companies like Google, Amazon, and Microsoft.

**Date**: 2026-02-03
**Branch**: alt-design (post-merge from main)

---

## Orchestrator Assessment Summary

| Orchestrator                                | Lines | Status               | Priority |
| ------------------------------------------- | ----- | -------------------- | -------- |
| `order-fulfillment-orchestrator.service.ts` | ~990  | ❌ Needs Refactoring | **HIGH** |
| `subscriptions-orchestrator.service.ts`     | ~475  | ⚠️ Minor Issues      | LOW      |
| `auth-orchestrator.service.ts`              | ~324  | ✅ Mostly Good       | LOW      |
| `order-orchestrator.service.ts`             | ~270  | ✅ Good              | NONE     |
| `sim-orchestrator.service.ts`               | ~200  | ✅ Excellent         | NONE     |
| `billing-orchestrator.service.ts`           | ~47   | ✅ Perfect           | NONE     |

---

## HIGH Priority: Order Fulfillment Orchestrator

### Current Problems

1. **Inline Anonymous Functions in Distributed Transaction** (lines 172-557)
   - 10 transaction steps defined as inline arrow functions
   - Each step contains 20-80 lines of business logic
   - Violates Single Responsibility Principle
   - Difficult to unit test individual steps

2. **File Size**: ~990 lines (should be < 300 for an orchestrator)

3. **Mixed Concerns**: Helper methods embedded in orchestrator
   - `extractConfigurations()` (60+ lines) - data extraction
   - `extractContactIdentity()` (50+ lines) - data extraction
   - `formatBirthdayToYYYYMMDD()` (30 lines) - date formatting

4. **Duplicate Logic**: Step tracking pattern repeated in each step

### Target Architecture

```
apps/bff/src/modules/orders/
├── services/
│   ├── order-fulfillment-orchestrator.service.ts  (~200 lines)
│   ├── order-fulfillment-steps/
│   │   ├── index.ts
│   │   ├── validation.step.ts
│   │   ├── sf-status-update.step.ts
│   │   ├── sim-fulfillment.step.ts
│   │   ├── sf-activated-update.step.ts
│   │   ├── whmcs-mapping.step.ts
│   │   ├── whmcs-create.step.ts
│   │   ├── whmcs-accept.step.ts
│   │   ├── sf-registration-complete.step.ts
│   │   └── opportunity-update.step.ts
│   └── mappers/
│       └── order-configuration.mapper.ts
```

### Refactoring Steps

#### Step 1: Create Step Interface and Base Class

```typescript
// apps/bff/src/modules/orders/services/order-fulfillment-steps/fulfillment-step.interface.ts

import type { OrderFulfillmentContext } from "../order-fulfillment-orchestrator.service.js";
import type { TransactionStep } from "@bff/infra/database/services/distributed-transaction.service.js";

export interface FulfillmentStepConfig {
  context: OrderFulfillmentContext;
  logger: Logger;
}

export interface FulfillmentStep {
  readonly id: string;
  readonly description: string;
  readonly critical: boolean;

  /**
   * Build the transaction step for the distributed transaction
   */
  build(config: FulfillmentStepConfig): TransactionStep;

  /**
   * Check if this step should be included based on context
   */
  shouldInclude(context: OrderFulfillmentContext): boolean;
}
```

#### Step 2: Extract Each Step to Its Own Class

**Example: SIM Fulfillment Step**

```typescript
// apps/bff/src/modules/orders/services/order-fulfillment-steps/sim-fulfillment.step.ts

import { Injectable, Inject } from "@nestjs/common";
import { Logger } from "nestjs-pino";
import type { FulfillmentStep, FulfillmentStepConfig } from "./fulfillment-step.interface.js";
import type { OrderFulfillmentContext } from "../order-fulfillment-orchestrator.service.js";
import { SimFulfillmentService } from "../sim-fulfillment.service.js";
import { OrderConfigurationMapper } from "../mappers/order-configuration.mapper.js";
import { extractErrorMessage } from "@bff/core/utils/error.util.js";

@Injectable()
export class SimFulfillmentStep implements FulfillmentStep {
  readonly id = "sim_fulfillment";
  readonly description = "SIM activation via Freebit (PA05-18 + PA02-01 + PA05-05)";
  readonly critical = true; // Set dynamically based on SIM type

  constructor(
    private readonly simFulfillmentService: SimFulfillmentService,
    private readonly configMapper: OrderConfigurationMapper,
    @Inject(Logger) private readonly logger: Logger
  ) {}

  shouldInclude(context: OrderFulfillmentContext): boolean {
    return context.orderDetails?.orderType === "SIM";
  }

  build(config: FulfillmentStepConfig) {
    const { context } = config;

    return {
      id: this.id,
      description: this.description,
      execute: async () => this.execute(context),
      rollback: async () => this.rollback(context),
      critical: context.validation?.sfOrder?.SIM_Type__c === "Physical SIM",
    };
  }

  private async execute(context: OrderFulfillmentContext) {
    if (context.orderDetails?.orderType !== "SIM") {
      return { activated: false, simType: "eSIM" as const };
    }

    const sfOrder = context.validation?.sfOrder;
    const configurations = this.configMapper.extractConfigurations(
      context.payload?.configurations,
      sfOrder
    );

    const assignedPhysicalSimId = this.extractAssignedSimId(sfOrder);
    const voiceMailEnabled = sfOrder?.SIM_Voice_Mail__c === true;
    const callWaitingEnabled = sfOrder?.SIM_Call_Waiting__c === true;
    const contactIdentity = this.configMapper.extractContactIdentity(sfOrder);

    this.logger.log("Starting SIM fulfillment (before WHMCS)", {
      orderId: context.sfOrderId,
      simType: sfOrder?.SIM_Type__c,
      assignedPhysicalSimId,
      voiceMailEnabled,
      callWaitingEnabled,
      hasContactIdentity: !!contactIdentity,
    });

    const result = await this.simFulfillmentService.fulfillSimOrder({
      orderDetails: context.orderDetails,
      configurations,
      assignedPhysicalSimId,
      voiceMailEnabled,
      callWaitingEnabled,
      contactIdentity,
    });

    context.simFulfillmentResult = result;
    return result;
  }

  private async rollback(context: OrderFulfillmentContext) {
    this.logger.warn("SIM fulfillment step needs rollback - manual intervention may be required", {
      sfOrderId: context.sfOrderId,
      simFulfillmentResult: context.simFulfillmentResult,
    });
  }

  private extractAssignedSimId(sfOrder?: SalesforceOrderRecord | null): string | undefined {
    return typeof sfOrder?.Assign_Physical_SIM__c === "string"
      ? sfOrder.Assign_Physical_SIM__c
      : undefined;
  }
}
```

#### Step 3: Extract Configuration Mapper

```typescript
// apps/bff/src/modules/orders/services/mappers/order-configuration.mapper.ts

import { Injectable } from "@nestjs/common";
import type { SalesforceOrderRecord } from "@customer-portal/domain/orders/providers";
import type { ContactIdentityData } from "../sim-fulfillment.service.js";

@Injectable()
export class OrderConfigurationMapper {
  /**
   * Extract and normalize configurations from payload and Salesforce order
   */
  extractConfigurations(
    rawConfigurations: unknown,
    sfOrder?: SalesforceOrderRecord | null
  ): Record<string, unknown> {
    const config: Record<string, unknown> = {};

    if (rawConfigurations && typeof rawConfigurations === "object") {
      Object.assign(config, rawConfigurations as Record<string, unknown>);
    }

    if (sfOrder) {
      this.fillFromSalesforceOrder(config, sfOrder);
    }

    return config;
  }

  /**
   * Extract contact identity data from Salesforce order porting fields
   */
  extractContactIdentity(sfOrder?: SalesforceOrderRecord | null): ContactIdentityData | undefined {
    if (!sfOrder) return undefined;

    const firstnameKanji = sfOrder.Porting_FirstName__c;
    const lastnameKanji = sfOrder.Porting_LastName__c;
    const firstnameKana = sfOrder.Porting_FirstName_Katakana__c;
    const lastnameKana = sfOrder.Porting_LastName_Katakana__c;
    const genderRaw = sfOrder.Porting_Gender__c;
    const birthdayRaw = sfOrder.Porting_DateOfBirth__c;

    if (!this.validateNameFields(firstnameKanji, lastnameKanji, firstnameKana, lastnameKana)) {
      return undefined;
    }

    const gender = this.validateGender(genderRaw);
    if (!gender) return undefined;

    const birthday = this.formatBirthdayToYYYYMMDD(birthdayRaw);
    if (!birthday) return undefined;

    return {
      firstnameKanji: firstnameKanji!,
      lastnameKanji: lastnameKanji!,
      firstnameKana: firstnameKana!,
      lastnameKana: lastnameKana!,
      gender,
      birthday,
    };
  }

  /**
   * Format birthday from various formats to YYYYMMDD
   */
  formatBirthdayToYYYYMMDD(dateStr?: string | null): string | undefined {
    if (!dateStr) return undefined;

    if (/^\d{8}$/.test(dateStr)) {
      return dateStr;
    }

    const isoMatch = dateStr.match(/^(\d{4})-(\d{2})-(\d{2})/);
    if (isoMatch) {
      return `${isoMatch[1]}${isoMatch[2]}${isoMatch[3]}`;
    }

    try {
      const date = new Date(dateStr);
      if (!isNaN(date.getTime())) {
        const year = date.getFullYear();
        const month = String(date.getMonth() + 1).padStart(2, "0");
        const day = String(date.getDate()).padStart(2, "0");
        return `${year}${month}${day}`;
      }
    } catch {
      // Parsing failed
    }

    return undefined;
  }

  private fillFromSalesforceOrder(
    config: Record<string, unknown>,
    sfOrder: SalesforceOrderRecord
  ): void {
    const fieldMappings: Array<[string, keyof SalesforceOrderRecord]> = [
      ["simType", "SIM_Type__c"],
      ["eid", "EID__c"],
      ["activationType", "Activation_Type__c"],
      ["scheduledAt", "Activation_Scheduled_At__c"],
      ["mnpPhone", "MNP_Phone_Number__c"],
      ["mnpNumber", "MNP_Reservation_Number__c"],
      ["mnpExpiry", "MNP_Expiry_Date__c"],
      ["mvnoAccountNumber", "MVNO_Account_Number__c"],
      ["portingFirstName", "Porting_FirstName__c"],
      ["portingLastName", "Porting_LastName__c"],
      ["portingFirstNameKatakana", "Porting_FirstName_Katakana__c"],
      ["portingLastNameKatakana", "Porting_LastName_Katakana__c"],
      ["portingGender", "Porting_Gender__c"],
      ["portingDateOfBirth", "Porting_DateOfBirth__c"],
    ];

    for (const [configKey, sfField] of fieldMappings) {
      if (!config[configKey] && sfOrder[sfField]) {
        config[configKey] = sfOrder[sfField];
      }
    }

    // Handle MNP flag specially
    if (!config["isMnp"] && sfOrder.MNP_Application__c) {
      config["isMnp"] = "true";
    }
  }

  private validateNameFields(
    firstnameKanji?: string | null,
    lastnameKanji?: string | null,
    firstnameKana?: string | null,
    lastnameKana?: string | null
  ): boolean {
    return !!(firstnameKanji && lastnameKanji && firstnameKana && lastnameKana);
  }

  private validateGender(genderRaw?: string | null): "M" | "F" | undefined {
    return genderRaw === "M" || genderRaw === "F" ? genderRaw : undefined;
  }
}
```

#### Step 4: Create Step Builder Service

```typescript
// apps/bff/src/modules/orders/services/order-fulfillment-steps/fulfillment-step-builder.service.ts

import { Injectable, Inject } from "@nestjs/common";
import { Logger } from "nestjs-pino";
import type { TransactionStep } from "@bff/infra/database/services/distributed-transaction.service.js";
import type { OrderFulfillmentContext } from "../order-fulfillment-orchestrator.service.js";

// Import all steps
import { ValidationStep } from "./validation.step.js";
import { SfStatusUpdateStep } from "./sf-status-update.step.js";
import { SimFulfillmentStep } from "./sim-fulfillment.step.js";
import { SfActivatedUpdateStep } from "./sf-activated-update.step.js";
import { WhmcsMappingStep } from "./whmcs-mapping.step.js";
import { WhmcsCreateStep } from "./whmcs-create.step.js";
import { WhmcsAcceptStep } from "./whmcs-accept.step.js";
import { SfRegistrationCompleteStep } from "./sf-registration-complete.step.js";
import { OpportunityUpdateStep } from "./opportunity-update.step.js";

@Injectable()
export class FulfillmentStepBuilder {
  private readonly allSteps: FulfillmentStep[];

  constructor(
    validationStep: ValidationStep,
    sfStatusUpdateStep: SfStatusUpdateStep,
    simFulfillmentStep: SimFulfillmentStep,
    sfActivatedUpdateStep: SfActivatedUpdateStep,
    whmcsMappingStep: WhmcsMappingStep,
    whmcsCreateStep: WhmcsCreateStep,
    whmcsAcceptStep: WhmcsAcceptStep,
    sfRegistrationCompleteStep: SfRegistrationCompleteStep,
    opportunityUpdateStep: OpportunityUpdateStep,
    @Inject(Logger) private readonly logger: Logger
  ) {
    // Define step execution order
    this.allSteps = [
      sfStatusUpdateStep,
      simFulfillmentStep,
      sfActivatedUpdateStep,
      whmcsMappingStep,
      whmcsCreateStep,
      whmcsAcceptStep,
      sfRegistrationCompleteStep,
      opportunityUpdateStep,
    ];
  }

  /**
   * Build transaction steps based on context
   */
  buildTransactionSteps(context: OrderFulfillmentContext): TransactionStep[] {
    const config = { context, logger: this.logger };

    return this.allSteps
      .filter(step => step.shouldInclude(context))
      .map(step => step.build(config));
  }
}
```

#### Step 5: Refactor Orchestrator to Use Step Builder

```typescript
// apps/bff/src/modules/orders/services/order-fulfillment-orchestrator.service.ts (REFACTORED)

import { Injectable, Inject } from "@nestjs/common";
import { Logger } from "nestjs-pino";
import { DistributedTransactionService } from "@bff/infra/database/services/distributed-transaction.service.js";
import { FulfillmentStepBuilder } from "./order-fulfillment-steps/fulfillment-step-builder.service.js";
import { OrderFulfillmentValidator } from "./order-fulfillment-validator.service.js";
import { OrderFulfillmentErrorService } from "./order-fulfillment-error.service.js";
import { OrderEventsService } from "./order-events.service.js";
import { OrdersCacheService } from "./orders-cache.service.js";
import { extractErrorMessage } from "@bff/core/utils/error.util.js";
import { FulfillmentException } from "@bff/core/exceptions/domain-exceptions.js";

// ... types remain the same ...

@Injectable()
export class OrderFulfillmentOrchestrator {
  constructor(
    @Inject(Logger) private readonly logger: Logger,
    private readonly stepBuilder: FulfillmentStepBuilder,
    private readonly validator: OrderFulfillmentValidator,
    private readonly errorService: OrderFulfillmentErrorService,
    private readonly transactionService: DistributedTransactionService,
    private readonly orderEvents: OrderEventsService,
    private readonly ordersCache: OrdersCacheService
  ) {}

  async executeFulfillment(
    sfOrderId: string,
    payload: Record<string, unknown>,
    idempotencyKey: string
  ): Promise<OrderFulfillmentContext> {
    const context = this.initializeContext(sfOrderId, idempotencyKey, payload);

    this.logger.log("Starting transactional fulfillment orchestration", {
      sfOrderId,
      idempotencyKey,
    });

    try {
      // Step 1: Validate
      const validation = await this.validator.validateFulfillmentRequest(sfOrderId, idempotencyKey);
      context.validation = validation;

      if (validation.isAlreadyProvisioned) {
        return this.handleAlreadyProvisioned(context);
      }

      // Step 2: Build and execute transaction steps
      const steps = this.stepBuilder.buildTransactionSteps(context);
      const result = await this.transactionService.executeDistributedTransaction(steps, {
        description: `Order fulfillment for ${sfOrderId}`,
        timeout: 300000,
        continueOnNonCriticalFailure: true,
      });

      if (!result.success) {
        throw new FulfillmentException(result.error || "Fulfillment transaction failed", {
          sfOrderId,
          idempotencyKey,
          stepsExecuted: result.stepsExecuted,
          stepsRolledBack: result.stepsRolledBack,
        });
      }

      this.logger.log("Transactional fulfillment completed successfully", {
        sfOrderId,
        stepsExecuted: result.stepsExecuted,
        duration: result.duration,
      });

      await this.invalidateCaches(context);
      return context;
    } catch (error) {
      await this.handleFulfillmentError(context, error as Error);
      throw error;
    }
  }

  private initializeContext(
    sfOrderId: string,
    idempotencyKey: string,
    payload: Record<string, unknown>
  ): OrderFulfillmentContext {
    return {
      sfOrderId,
      idempotencyKey,
      validation: null,
      payload,
      steps: [],
    };
  }

  private handleAlreadyProvisioned(context: OrderFulfillmentContext): OrderFulfillmentContext {
    this.logger.log("Order already provisioned, skipping fulfillment", {
      sfOrderId: context.sfOrderId,
    });
    this.orderEvents.publish(context.sfOrderId, {
      orderId: context.sfOrderId,
      status: "Completed",
      activationStatus: "Activated",
      stage: "completed",
      source: "fulfillment",
      message: "Order already provisioned",
      timestamp: new Date().toISOString(),
      payload: { whmcsOrderId: context.validation?.whmcsOrderId },
    });
    return context;
  }

  private async invalidateCaches(context: OrderFulfillmentContext): Promise<void> {
    const accountId = context.validation?.sfOrder?.AccountId;
    await Promise.all([
      this.ordersCache.invalidateOrder(context.sfOrderId),
      accountId ? this.ordersCache.invalidateAccountOrders(accountId) : Promise.resolve(),
    ]).catch(error => {
      this.logger.warn("Failed to invalidate caches", { error: extractErrorMessage(error) });
    });
  }

  private async handleFulfillmentError(
    context: OrderFulfillmentContext,
    error: Error
  ): Promise<void> {
    await this.invalidateCaches(context);
    await this.errorService.handleAndReport(context, error);
    this.orderEvents.publishFailure(context.sfOrderId, error.message);
  }

  getFulfillmentSummary(context: OrderFulfillmentContext) {
    // ... same as before, no changes needed ...
  }
}
```

#### Step 6: Update Module Registration

```typescript
// apps/bff/src/modules/orders/orders.module.ts

// Add new providers
import { FulfillmentStepBuilder } from "./services/order-fulfillment-steps/fulfillment-step-builder.service.js";
import { OrderConfigurationMapper } from "./services/mappers/order-configuration.mapper.js";
import {
  SfStatusUpdateStep,
  SimFulfillmentStep,
  SfActivatedUpdateStep,
  WhmcsMappingStep,
  WhmcsCreateStep,
  WhmcsAcceptStep,
  SfRegistrationCompleteStep,
  OpportunityUpdateStep,
} from "./services/order-fulfillment-steps/index.js";

@Module({
  providers: [
    // Existing services...

    // New step classes
    FulfillmentStepBuilder,
    OrderConfigurationMapper,
    SfStatusUpdateStep,
    SimFulfillmentStep,
    SfActivatedUpdateStep,
    WhmcsMappingStep,
    WhmcsCreateStep,
    WhmcsAcceptStep,
    SfRegistrationCompleteStep,
    OpportunityUpdateStep,
  ],
})
export class OrdersModule {}
```

---

## LOW Priority: Subscriptions Orchestrator

### Current Issues

1. Business logic in filter methods (lines 193-250):
   - `getExpiringSoon()` - date filtering logic
   - `getRecentActivity()` - date filtering logic
   - `searchSubscriptions()` - search logic

2. Cache helper methods could be in a separate service

### Recommended Changes

1. Extract date filtering to a utility:

   ```typescript
   // @bff/core/utils/date-filter.util.ts
   export function filterByDateRange<T>(
     items: T[],
     dateExtractor: (item: T) => Date,
     range: { start?: Date; end?: Date }
   ): T[] { ... }
   ```

2. Consider extracting `SubscriptionFilterService` if filtering becomes more complex

**Status**: Not urgent - current implementation is acceptable

---

## LOW Priority: Auth Orchestrator

### Current Issues

1. `getAccountStatus()` method (lines 241-296) contains complex conditional logic
   - Could be extracted to `AccountStatusResolver` service

### Recommended Changes

```typescript
// apps/bff/src/modules/auth/application/account-status-resolver.service.ts

@Injectable()
export class AccountStatusResolver {
  async resolve(email: string): Promise<AccountStatus> {
    // Move logic from getAccountStatus here
  }
}
```

**Status**: Not urgent - method is well-contained and testable as-is

---

## No Changes Needed

### Order Orchestrator ✅

- Clean thin delegation pattern
- Appropriate size (~270 lines)
- Single private method is justified

### SIM Orchestrator ✅

- Excellent thin delegation
- All methods delegate to specialized services
- `getSimInfo` has reasonable composition logic

### Billing Orchestrator ✅

- Perfect thin facade pattern
- Pure delegation only
- Model example for others

---

## Implementation Checklist

### Phase 1: High Priority (Order Fulfillment Orchestrator)

- [ ] Create `order-fulfillment-steps/` directory
- [ ] Create `fulfillment-step.interface.ts`
- [ ] Create `OrderConfigurationMapper` service
- [ ] Extract `SfStatusUpdateStep`
- [ ] Extract `SimFulfillmentStep`
- [ ] Extract `SfActivatedUpdateStep`
- [ ] Extract `WhmcsMappingStep`
- [ ] Extract `WhmcsCreateStep`
- [ ] Extract `WhmcsAcceptStep`
- [ ] Extract `SfRegistrationCompleteStep`
- [ ] Extract `OpportunityUpdateStep`
- [ ] Create `FulfillmentStepBuilder` service
- [ ] Refactor `OrderFulfillmentOrchestrator` to use step builder
- [ ] Update `OrdersModule` with new providers
- [ ] Add unit tests for each step class
- [ ] Add integration tests for step builder

### Phase 2: Low Priority

- [ ] Extract `SubscriptionFilterService` (if needed)
- [ ] Extract `AccountStatusResolver` (if needed)

---

## Testing Strategy

### Step Classes

Each step class should have unit tests that verify:

1. `shouldInclude()` returns correct boolean based on context
2. `execute()` performs expected operations
3. `rollback()` handles cleanup appropriately
4. Error cases are handled correctly

### Step Builder

Integration tests should verify:

1. Correct steps are included for SIM orders
2. Correct steps are included for non-SIM orders
3. Step order is maintained
4. Context is properly passed between steps

### Orchestrator

E2E tests should verify:

1. Complete fulfillment flow works
2. Partial failures trigger rollbacks
3. Already provisioned orders are handled
4. Error reporting works correctly

---

## Benefits of This Refactoring

1. **Testability**: Each step can be unit tested in isolation
2. **Maintainability**: Changes to one step don't affect others
3. **Readability**: Orchestrator becomes a thin coordinator
4. **Extensibility**: New steps can be added without modifying orchestrator
5. **Reusability**: Steps can potentially be reused in other workflows
6. **Debugging**: Easier to identify which step failed
7. **Code Review**: Smaller, focused PRs for each step

---

## References

- Google Engineering Practices: https://google.github.io/eng-practices/
- Microsoft .NET Application Architecture: https://docs.microsoft.com/en-us/dotnet/architecture/
- Clean Architecture by Robert C. Martin
- Domain-Driven Design by Eric Evans