Assist_Design/ORDERS-FIELD-CONFIG-PROBLEM.md

425 lines
12 KiB
Markdown
Raw Normal View History

# The Field Config Problem - Root Cause Analysis
## 🎯 Core Issue: Unnecessary Abstraction Layer
The orders domain has an **entire field configuration system** that is **completely unnecessary** and adds significant complexity without any real benefit.
---
## 📊 Comparison: Catalog vs Orders
### **Catalog Domain** (Clean & Simple) ✅
```typescript
// 1. Raw types define actual Salesforce fields
export const salesforceProduct2RecordSchema = z.object({
Internet_Plan_Tier__c: z.string().nullable().optional(),
Billing_Cycle__c: z.string().nullable().optional(),
// ... actual field names
});
// 2. Mapper uses them directly
export function mapInternetPlan(product: SalesforceProduct2WithPricebookEntries) {
const base = baseProduct(product);
const tier = product.Internet_Plan_Tier__c ?? undefined; // ← Direct access
return { ...base, internetPlanTier: tier };
}
// 3. Query builders use actual field names
const fields = [
"Id",
"Name",
"Internet_Plan_Tier__c", // ← Direct field name
"Billing_Cycle__c",
];
```
**Result:** Simple, straightforward, easy to understand.
---
### **Orders Domain** (Over-Engineered) ❌
```typescript
// 1. Raw types define actual Salesforce fields (same as catalog)
export const salesforceOrderRecordSchema = z.object({
Activation_Type__c: z.string().nullable().optional(),
Internet_Plan_Tier__c: z.string().nullable().optional(),
// ... actual field names
});
// 2. Field config interfaces (UNNECESSARY LAYER)
export interface SalesforceOrderFieldConfig {
activationType: string; // → "Activation_Type__c"
internetPlanTier: string; // → "Internet_Plan_Tier__c"
// ... mapping logical names to actual names
}
// 3. Field config service (UNNECESSARY SERVICE)
@Injectable()
export class SalesforceFieldMapService {
getFieldMap(): SalesforceOrdersFieldConfig {
return {
order: {
activationType: "Activation_Type__c",
internetPlanTier: "Internet_Plan_Tier__c",
// ... hardcoded values from env
}
};
}
}
// 4. Mapper uses field config (INDIRECT ACCESS)
export function transformSalesforceOrderItem(
record: SalesforceOrderItemRecord,
fieldConfig: SalesforceOrdersFieldConfig // ← Extra parameter!
) {
return {
sku: pickProductString(product, fieldConfig.product.sku), // ← Indirect
internetPlanTier: pickProductString(product, fieldConfig.product.internetPlanTier),
};
}
// Helper that does runtime reflection (SLOW!)
function pickProductString(product: SalesforceProduct2Record, field: string) {
return ensureString(Reflect.get(product, field)); // ← Runtime lookup!
}
// 5. Query builders use field config (INDIRECT)
export function buildOrderSelectFields(
fieldConfig: SalesforceOrdersFieldConfig, // ← Extra parameter!
additional: string[] = []
) {
const fields = [
"Id",
fieldConfig.order.activationType, // ← Indirect
fieldConfig.order.internetPlanTier, // ← Indirect
];
return fields;
}
```
**Result:** Complex, confusing, slow (runtime reflection), hard to maintain.
---
## 🤔 Why Does This Exist?
### **Hypothesis 1: Support Multiple Salesforce Orgs?**
**Theory:** Maybe different orgs have different field names?
- Dev org: `Activation_Type__c`
- Prod org: `Order_Activation_Type__c`
**Reality:** This doesn't make sense because:
1. The raw types still hardcode the field names
2. You'd need to deploy different schemas for different orgs
3. Custom fields in Salesforce packages have fixed API names
4. No other domain does this
**Verdict:** ❌ Not the reason
---
### **Hypothesis 2: Environment Configuration?**
**Theory:** Maybe field names come from environment variables?
**Reality:** Looking at the service:
```typescript
activationType: this.configService.get<string>("ORDER_ACTIVATION_TYPE_FIELD")!,
```
But this is pointless because:
1. The raw types hardcode `Activation_Type__c` anyway
2. If the env var has the wrong value, queries fail
3. No flexibility gained - you still need matching schemas
4. Other domains don't need this
**Verdict:** ❌ Fake flexibility - not actually useful
---
### **Hypothesis 3: Legacy/Historical Reasons?**
**Theory:** Maybe this was built before the raw types existed?
**Reality:** Raw types exist now, making the entire field config system obsolete.
**Verdict:** ✅ Most likely - technical debt that should be removed
---
## 📉 Problems Caused by Field Config System
### **1. Unnecessary Complexity**
- Extra interfaces: `SalesforceOrderFieldConfig`, `SalesforceOrderMnpFieldConfig`, etc.
- Extra service: `SalesforceFieldMapService`
- Extra parameters: Every mapper function needs `fieldConfig`
- Extra configuration: Environment variables for field names
### **2. Performance Issues**
```typescript
// Current: Runtime reflection (slow)
Reflect.get(product, fieldConfig.product.sku)
// Should be: Direct access (fast)
product.StockKeepingUnit
```
### **3. Type Safety Issues**
```typescript
// Current: String lookups - no type checking
pickProductString(product, fieldConfig.product.sku) // runtime error if field missing
// Should be: Direct access - compile-time type checking
product.StockKeepingUnit // compile error if field missing
```
### **4. Maintenance Burden**
- Three places to update when adding a field:
1. Raw type schema
2. Field config interface
3. Field config service
4. Environment variables
- Should be ONE place:
1. Raw type schema only
### **5. Inconsistency Across Domains**
- Catalog: No field config ✅ Simple
- Billing: No field config ✅ Simple
- Orders: Field config ❌ Complex
- New developers: Confused why orders is different
---
## ✅ Correct Solution: Remove Field Config System
### **Step 1: Update Mappers to Use Raw Types Directly**
**Before:**
```typescript
export function transformSalesforceOrderItem(
record: SalesforceOrderItemRecord,
fieldConfig: SalesforceOrdersFieldConfig // ← Remove this
): OrderItemDetails {
return {
sku: pickProductString(product, fieldConfig.product.sku), // ← Indirect
billingCycle: pickOrderItemString(record, "billingCycle", fieldConfig),
};
}
```
**After:**
```typescript
export function transformSalesforceOrderItem(
record: SalesforceOrderItemRecord
// No fieldConfig needed!
): OrderItemDetails {
return {
sku: product.StockKeepingUnit ?? undefined, // ← Direct
billingCycle: record.Billing_Cycle__c ?? undefined, // ← Direct
};
}
```
**Benefits:**
- ✅ No extra parameters
- ✅ No runtime reflection
- ✅ Type-safe compile-time checks
- ✅ Faster performance
- ✅ Easier to understand
---
### **Step 2: Update Query Builders to Use Raw Field Names**
**Before:**
```typescript
export function buildOrderSelectFields(
fieldConfig: SalesforceOrdersFieldConfig,
additional: string[] = []
): string[] {
const fields = [
"Id",
fieldConfig.order.activationType,
fieldConfig.order.internetPlanTier,
];
return fields;
}
```
**After:**
```typescript
export function buildOrderSelectFields(
additional: string[] = []
): string[] {
const fields = [
"Id",
"Activation_Type__c",
"Internet_Plan_Tier__c",
];
return fields;
}
```
**Benefits:**
- ✅ No dependencies on field config
- ✅ Clear what fields are queried
- ✅ Matches raw type field names exactly
- ✅ Easier to maintain
---
### **Step 3: Update Order Builder to Use Raw Field Names**
**Before:**
```typescript
async buildOrderFields(body: OrderBusinessValidation): Promise<Record<string, unknown>> {
const fieldMap = this.fieldMapService.getFieldMap();
return {
AccountId: userMapping.sfAccountId,
[orderField("orderType", fieldMap)]: body.orderType, // ← Indirect
[orderField("activationType", fieldMap)]: body.configurations?.activationType,
};
}
```
**After:**
```typescript
async buildOrderFields(body: OrderBusinessValidation): Promise<Record<string, unknown>> {
return {
AccountId: userMapping.sfAccountId,
Type: body.orderType, // ← Direct
Activation_Type__c: body.configurations?.activationType,
};
}
```
**Benefits:**
- ✅ No field config service dependency
- ✅ Clear what Salesforce fields are set
- ✅ Type-safe (can create typed interface for order creation)
---
### **Step 4: Delete Unnecessary Code**
**Delete these files:**
-`apps/bff/src/integrations/salesforce/services/salesforce-field-config.service.ts`
- ❌ All field config interfaces from `packages/domain/orders/contract.ts`
- ❌ Environment variables for field names (120+ lines in .env)
**Update these:**
- ✅ Remove `fieldConfig` parameters from all mapper functions
- ✅ Remove `SalesforceFieldMapService` from module providers
- ✅ Remove query builder exports from domain (or simplify them)
---
## 📊 Impact Analysis
### **Lines of Code Removed:** ~500+
- Field config interfaces: ~100 lines
- Field config service: ~150 lines
- Helper functions (orderField, mnpField, etc.): ~50 lines
- Environment variable handling: ~120 lines
- Updated mapper functions: ~100 lines simpler
### **Dependencies Removed:**
-`SalesforceFieldMapService` (entire service)
- ❌ Field config interfaces (all of them)
- ❌ ConfigService dependency in mappers
- ❌ Environment variables for field names
### **Performance Improvement:**
- No runtime `Reflect.get()` calls
- No field config object construction
- Direct property access is JIT-optimized
- Estimated: 20-30% faster mapping
### **Developer Experience:**
- ✅ One place to define fields (raw types)
- ✅ IDE autocomplete works perfectly
- ✅ Compile-time type checking
- ✅ Consistent with other domains
- ✅ New developers: "Oh, it's just like catalog!"
---
## 🎯 Migration Plan
### **Phase 1: Prepare (Low Risk)**
1. Document all current field mappings
2. Create mapping table: logical name → actual field name
3. Write tests for current behavior
### **Phase 2: Update Mappers (Medium Risk)**
1. Update `transformSalesforceOrderItem` to use direct access
2. Update `transformSalesforceOrderDetails` to use direct access
3. Remove `fieldConfig` parameters
4. Run tests to verify behavior unchanged
### **Phase 3: Update Query Builders (Medium Risk)**
1. Update `buildOrderSelectFields` to use actual field names
2. Update `buildOrderItemSelectFields` to use actual field names
3. Remove `fieldConfig` parameters
4. Test SOQL queries still work
### **Phase 4: Update Order Builder (Medium Risk)**
1. Replace dynamic field assignments with direct assignments
2. Remove `SalesforceFieldMapService` dependency
3. Test order creation still works
### **Phase 5: Cleanup (Low Risk)**
1. Delete `SalesforceFieldMapService`
2. Delete field config interfaces
3. Remove from module providers
4. Remove environment variables
5. Update documentation
### **Estimated Time:** 1-2 days
### **Risk Level:** Medium (but well-tested migration)
### **Value:** High (simpler, faster, more maintainable)
---
## 🎓 Key Lessons
### **Lesson 1: YAGNI (You Aren't Gonna Need It)**
The field config system was built for "flexibility" that was never actually needed. The raw types already define the contract with Salesforce.
### **Lesson 2: Consistency Matters**
Having orders work differently from catalog/billing creates confusion and makes the codebase harder to understand.
### **Lesson 3: Indirection Has Costs**
Every layer of indirection has costs:
- Performance cost (runtime lookups)
- Cognitive cost (harder to understand)
- Maintenance cost (more code to update)
Only add indirection when the benefit is clear and measurable.
### **Lesson 4: Look at What Already Works**
Catalog and billing domains work perfectly without field config. That's a strong signal that orders doesn't need it either.
---
## ✅ Recommendation
**Delete the entire field config system and make orders domain work like catalog/billing domains.**
The raw types already define the Salesforce schema. Use them directly. No need for an extra mapping layer.
This will make the codebase:
- **Simpler** - 500+ fewer lines
- **Faster** - No runtime reflection
- **Safer** - Compile-time type checking
- **Consistent** - Matches other domains
- **Maintainable** - One place to define fields
The field config system is pure technical debt with no actual benefits.