refactor: improve error handling and caching in BFF services
- Enhanced UnifiedExceptionFilter to handle ZodValidationException, extracting field errors for better user feedback. - Updated WhmcsRequestQueueService and WhmcsHttpClientService to use logger.warn for non-critical errors, improving log clarity. - Introduced Redis-backed caching in JapanPostFacade for postal code lookups, reducing redundant API calls. - Refactored address handling in AddressWriterService to deduplicate concurrent Japan Post lookups, optimizing API usage. - Improved error parsing in various forms and hooks to provide clearer error messages and field-specific feedback.
This commit is contained in:
barsa
parent
6299fbabdc
commit
9b7cbcf78f
@ -10,6 +10,7 @@ import type { ExceptionFilter, ArgumentsHost } from "@nestjs/common";
|
||||
import type { Request, Response } from "express";
|
||||
import { Logger } from "nestjs-pino";
|
||||
import { ConfigService } from "@nestjs/config";
|
||||
import { ZodValidationException } from "nestjs-zod";
|
||||
import {
|
||||
ErrorCode,
|
||||
ErrorMessages,
|
||||
@ -79,16 +80,22 @@ export class UnifiedExceptionFilter implements ExceptionFilter {
|
||||
const errorContext = this.buildErrorContext(request);
|
||||
|
||||
// Extract status code and error details
|
||||
const { status, errorCode, originalMessage } = this.extractErrorDetails(exception);
|
||||
const { status, errorCode, originalMessage, fieldErrors } = this.extractErrorDetails(exception);
|
||||
|
||||
// Get user-friendly message (with dev details if in development)
|
||||
const userMessage = this.getUserMessage(errorCode, originalMessage);
|
||||
const userMessage = this.getUserMessage(errorCode, originalMessage, fieldErrors);
|
||||
|
||||
// Log the error
|
||||
this.logError(errorCode, originalMessage, status, errorContext, exception);
|
||||
|
||||
// Build and send response
|
||||
const errorResponse = this.buildErrorResponse(errorCode, userMessage, status, errorContext);
|
||||
const errorResponse = this.buildErrorResponse(
|
||||
errorCode,
|
||||
userMessage,
|
||||
status,
|
||||
errorContext,
|
||||
fieldErrors
|
||||
);
|
||||
response.status(status).json(errorResponse);
|
||||
}
|
||||
|
||||
@ -99,12 +106,22 @@ export class UnifiedExceptionFilter implements ExceptionFilter {
|
||||
status: number;
|
||||
errorCode: ErrorCodeType;
|
||||
originalMessage: string;
|
||||
fieldErrors?: Record<string, string> | undefined;
|
||||
} {
|
||||
let status = HttpStatus.INTERNAL_SERVER_ERROR;
|
||||
let originalMessage = "An unexpected error occurred";
|
||||
let explicitCode: ErrorCodeType | undefined;
|
||||
let fieldErrors: Record<string, string> | undefined;
|
||||
|
||||
if (exception instanceof HttpException) {
|
||||
// ZodValidationException must be checked before HttpException
|
||||
// since ZodValidationException extends BadRequestException extends HttpException
|
||||
if (exception instanceof ZodValidationException) {
|
||||
status = exception.getStatus();
|
||||
const extracted = this.extractZodFieldErrors(exception);
|
||||
fieldErrors = extracted.fieldErrors;
|
||||
originalMessage = extracted.message;
|
||||
explicitCode = ErrorCode.VALIDATION_FAILED;
|
||||
} else if (exception instanceof HttpException) {
|
||||
status = exception.getStatus();
|
||||
const extracted = this.extractExceptionDetails(exception);
|
||||
originalMessage = extracted.message;
|
||||
@ -120,7 +137,49 @@ export class UnifiedExceptionFilter implements ExceptionFilter {
|
||||
// Map to error code
|
||||
const errorCode = explicitCode ?? mapHttpStatusToErrorCode(status);
|
||||
|
||||
return { status, errorCode, originalMessage };
|
||||
return { status, errorCode, originalMessage, fieldErrors };
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract field errors and a summary message from a ZodValidationException.
|
||||
*/
|
||||
private extractZodFieldErrors(exception: ZodValidationException): {
|
||||
fieldErrors?: Record<string, string>;
|
||||
message: string;
|
||||
} {
|
||||
const zodError = exception.getZodError();
|
||||
const issues = this.extractZodIssues(zodError);
|
||||
|
||||
if (issues.length === 0) {
|
||||
return { message: "Validation failed" };
|
||||
}
|
||||
|
||||
const fieldErrors: Record<string, string> = {};
|
||||
for (const issue of issues) {
|
||||
const fieldKey = issue.path.length > 0 ? issue.path.join(".") : "_form";
|
||||
if (!(fieldKey in fieldErrors)) {
|
||||
fieldErrors[fieldKey] = issue.message;
|
||||
}
|
||||
}
|
||||
|
||||
return { fieldErrors, message: issues[0]!.message };
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract Zod issues from the unknown ZodError returned by getZodError()
|
||||
*/
|
||||
private extractZodIssues(
|
||||
zodError: unknown
|
||||
): Array<{ path: (string | number)[]; message: string }> {
|
||||
if (
|
||||
zodError &&
|
||||
typeof zodError === "object" &&
|
||||
"issues" in zodError &&
|
||||
Array.isArray((zodError as { issues: unknown }).issues)
|
||||
) {
|
||||
return (zodError as { issues: Array<{ path: (string | number)[]; message: string }> }).issues;
|
||||
}
|
||||
return [];
|
||||
}
|
||||
|
||||
/**
|
||||
@ -193,9 +252,21 @@ export class UnifiedExceptionFilter implements ExceptionFilter {
|
||||
}
|
||||
|
||||
/**
|
||||
* Get user-friendly message, with dev details in development mode
|
||||
* Get user-friendly message, with dev details in development mode.
|
||||
* When fieldErrors is present (Zod validation), use the original Zod message
|
||||
* instead of the generic canned message, since Zod messages are developer-authored
|
||||
* and safe to display to users.
|
||||
*/
|
||||
private getUserMessage(errorCode: ErrorCodeType, originalMessage: string): string {
|
||||
private getUserMessage(
|
||||
errorCode: ErrorCodeType,
|
||||
originalMessage: string,
|
||||
fieldErrors?: Record<string, string>
|
||||
): string {
|
||||
// For Zod validation errors, use the original message (developer-authored, safe to display)
|
||||
if (fieldErrors) {
|
||||
return originalMessage;
|
||||
}
|
||||
|
||||
const userMessage = ErrorMessages[errorCode] ?? ErrorMessages[ErrorCode.UNKNOWN];
|
||||
|
||||
if (this.isDevelopment && originalMessage !== userMessage) {
|
||||
@ -228,18 +299,19 @@ export class UnifiedExceptionFilter implements ExceptionFilter {
|
||||
request: Request & { user?: { id?: string }; requestId?: string }
|
||||
): ErrorContext {
|
||||
const userAgentHeader = request.headers["user-agent"];
|
||||
const userAgent =
|
||||
typeof userAgentHeader === "string"
|
||||
? userAgentHeader
|
||||
: Array.isArray(userAgentHeader)
|
||||
? userAgentHeader[0]
|
||||
: undefined;
|
||||
|
||||
return {
|
||||
requestId: request.requestId ?? this.generateRequestId(),
|
||||
userId: request.user?.id,
|
||||
method: request.method,
|
||||
path: request.url,
|
||||
userAgent:
|
||||
typeof userAgentHeader === "string"
|
||||
? userAgentHeader
|
||||
: Array.isArray(userAgentHeader)
|
||||
? userAgentHeader[0]
|
||||
: undefined,
|
||||
userAgent,
|
||||
ip: request.ip,
|
||||
};
|
||||
}
|
||||
@ -302,7 +374,8 @@ export class UnifiedExceptionFilter implements ExceptionFilter {
|
||||
errorCode: ErrorCodeType,
|
||||
message: string,
|
||||
status: number,
|
||||
context: ErrorContext
|
||||
context: ErrorContext,
|
||||
fieldErrors?: Record<string, string>
|
||||
): ApiError {
|
||||
const metadata = ErrorMetadata[errorCode] ?? ErrorMetadata[ErrorCode.UNKNOWN];
|
||||
|
||||
@ -317,6 +390,7 @@ export class UnifiedExceptionFilter implements ExceptionFilter {
|
||||
timestamp: new Date().toISOString(),
|
||||
path: context.path,
|
||||
requestId: context.requestId,
|
||||
...(fieldErrors && { fieldErrors }),
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
@ -193,7 +193,7 @@ export class WhmcsRequestQueueService implements OnModuleInit, OnModuleDestroy {
|
||||
this.metrics.failedRequests++;
|
||||
this.metrics.lastErrorTime = new Date();
|
||||
|
||||
this.logger.error(
|
||||
this.logger.warn(
|
||||
{
|
||||
requestId,
|
||||
waitTime,
|
||||
|
||||
@ -3,15 +3,27 @@
|
||||
*
|
||||
* Public API for Japan Post integration.
|
||||
* Controllers should use this facade instead of internal services directly.
|
||||
*
|
||||
* Adds Redis-backed caching for postal code lookups — address data is
|
||||
* stable and rarely changes, so a 24-hour TTL avoids redundant API calls.
|
||||
*/
|
||||
|
||||
import { Injectable } from "@nestjs/common";
|
||||
import { Inject, Injectable } from "@nestjs/common";
|
||||
import { Logger } from "nestjs-pino";
|
||||
import { JapanPostAddressService } from "../services/japanpost-address.service.js";
|
||||
import { CacheService } from "@bff/infra/cache/cache.service.js";
|
||||
import type { AddressLookupResult } from "@customer-portal/domain/address";
|
||||
|
||||
const CACHE_PREFIX = "japanpost:zip:";
|
||||
const CACHE_TTL_SECONDS = 86_400; // 24 hours
|
||||
|
||||
@Injectable()
|
||||
export class JapanPostFacade {
|
||||
constructor(private readonly addressService: JapanPostAddressService) {}
|
||||
constructor(
|
||||
private readonly addressService: JapanPostAddressService,
|
||||
private readonly cache: CacheService,
|
||||
@Inject(Logger) private readonly logger: Logger
|
||||
) {}
|
||||
|
||||
/**
|
||||
* Lookup address by ZIP code
|
||||
@ -24,7 +36,30 @@ export class JapanPostFacade {
|
||||
zipCode: string,
|
||||
clientIp: string = "127.0.0.1"
|
||||
): Promise<AddressLookupResult> {
|
||||
return this.addressService.lookupByZipCode(zipCode, clientIp);
|
||||
const normalizedZip = zipCode.replace(/-/g, "");
|
||||
const cacheKey = `${CACHE_PREFIX}${normalizedZip}`;
|
||||
|
||||
try {
|
||||
const cached = await this.cache.get<AddressLookupResult>(cacheKey);
|
||||
if (cached) {
|
||||
return cached;
|
||||
}
|
||||
} catch {
|
||||
// Redis unavailable — proceed without cache
|
||||
}
|
||||
|
||||
const result = await this.addressService.lookupByZipCode(zipCode, clientIp);
|
||||
|
||||
// Cache successful lookups with results
|
||||
if (result.count > 0) {
|
||||
try {
|
||||
await this.cache.set(cacheKey, result, CACHE_TTL_SECONDS);
|
||||
} catch {
|
||||
this.logger.debug({ zipCode: normalizedZip }, "Failed to cache Japan Post lookup result");
|
||||
}
|
||||
}
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@ -49,7 +49,7 @@ export class WhmcsHttpClientService {
|
||||
this.stats.failedRequests++;
|
||||
this.stats.lastErrorTime = new Date();
|
||||
|
||||
this.logger.error(`WHMCS HTTP request failed [${action}]`, {
|
||||
this.logger.warn(`WHMCS HTTP request failed [${action}]`, {
|
||||
error: extractErrorMessage(error),
|
||||
action,
|
||||
params: redactForLogs(params),
|
||||
|
||||
@ -18,6 +18,9 @@ import { addressSchema, type Address, type WhmcsClient } from "@customer-portal/
|
||||
|
||||
@Injectable()
|
||||
export class WhmcsClientService {
|
||||
/** In-flight getClientDetails calls — deduplicates concurrent requests for the same client */
|
||||
private readonly inflightDetails = new Map<number, Promise<WhmcsClient>>();
|
||||
|
||||
constructor(
|
||||
private readonly connectionService: WhmcsConnectionFacade,
|
||||
private readonly cacheService: WhmcsCacheService,
|
||||
@ -54,18 +57,35 @@ export class WhmcsClientService {
|
||||
}
|
||||
|
||||
/**
|
||||
* Get client details by ID
|
||||
* Returns WhmcsClient (type inferred from domain mapper)
|
||||
* Get client details by ID.
|
||||
* Concurrent calls for the same clientId share a single API request.
|
||||
*/
|
||||
async getClientDetails(clientId: number): Promise<WhmcsClient> {
|
||||
try {
|
||||
// Try cache first
|
||||
const cached = await this.cacheService.getClientData(clientId);
|
||||
if (cached) {
|
||||
this.logger.debug(`Cache hit for client: ${clientId}`);
|
||||
return cached;
|
||||
}
|
||||
// Try cache first (fast path, no dedup needed)
|
||||
const cached = await this.cacheService.getClientData(clientId);
|
||||
if (cached) {
|
||||
this.logger.debug(`Cache hit for client: ${clientId}`);
|
||||
return cached;
|
||||
}
|
||||
|
||||
// Deduplicate: reuse in-flight request for the same clientId
|
||||
const inflight = this.inflightDetails.get(clientId);
|
||||
if (inflight) {
|
||||
return inflight;
|
||||
}
|
||||
|
||||
const promise = this.fetchClientDetails(clientId);
|
||||
this.inflightDetails.set(clientId, promise);
|
||||
|
||||
try {
|
||||
return await promise;
|
||||
} finally {
|
||||
this.inflightDetails.delete(clientId);
|
||||
}
|
||||
}
|
||||
|
||||
private async fetchClientDetails(clientId: number): Promise<WhmcsClient> {
|
||||
try {
|
||||
const response = await this.connectionService.getClientDetails(clientId);
|
||||
|
||||
if (!response || !response.client) {
|
||||
|
||||
@ -233,7 +233,7 @@ export class WhmcsInvoiceService {
|
||||
limit: number
|
||||
): InvoiceList {
|
||||
if (!response.invoices?.invoice) {
|
||||
this.logger.warn(`No invoices found for client ${clientId}`);
|
||||
this.logger.debug(`No invoices found for client ${clientId}`);
|
||||
return {
|
||||
invoices: [],
|
||||
pagination: {
|
||||
|
||||
@ -33,6 +33,12 @@ export interface ResolveWhmcsAddressParams {
|
||||
|
||||
@Injectable()
|
||||
export class AddressWriterService {
|
||||
/** In-flight Japan Post lookups keyed by postcode — deduplicates concurrent calls */
|
||||
private readonly inflightLookups = new Map<
|
||||
string,
|
||||
Promise<Awaited<ReturnType<JapanPostFacade["lookupByZipCode"]>>>
|
||||
>();
|
||||
|
||||
constructor(
|
||||
private readonly japanPost: JapanPostFacade,
|
||||
private readonly salesforceFacade: SalesforceFacade,
|
||||
@ -63,16 +69,13 @@ export class AddressWriterService {
|
||||
* Resolve English address from postal code via Japan Post API
|
||||
* and prepare WHMCS address fields.
|
||||
*
|
||||
* Flow:
|
||||
* 1. Call Japan Post API with postal code
|
||||
* 2. Match result using townJa (if multiple matches)
|
||||
* 3. Build BilingualAddress with resolved English fields
|
||||
* 4. Return WhmcsAddressFields via prepareWhmcsAddressFields()
|
||||
* Uses in-flight deduplication so concurrent lookups for the same
|
||||
* postcode share a single API call.
|
||||
*/
|
||||
async resolveWhmcsAddress(params: ResolveWhmcsAddressParams): Promise<WhmcsAddressFields> {
|
||||
const { postcode, townJa, streetAddress, buildingName, roomNumber, residenceType } = params;
|
||||
|
||||
const lookupResult = await this.japanPost.lookupByZipCode(postcode);
|
||||
const lookupResult = await this.lookupPostalCode(postcode);
|
||||
|
||||
if (lookupResult.addresses.length === 0) {
|
||||
this.logger.warn({ postcode }, "Japan Post API returned no results for postal code");
|
||||
@ -113,17 +116,73 @@ export class AddressWriterService {
|
||||
}
|
||||
|
||||
/**
|
||||
* Convenience: resolve WHMCS address from a full BilingualAddress.
|
||||
* Re-derives English fields from postal code via Japan Post API.
|
||||
* Deduplicated Japan Post lookup — concurrent calls for the same postcode
|
||||
* share one API request.
|
||||
*/
|
||||
async resolveWhmcsAddressFromBilingual(address: BilingualAddress): Promise<WhmcsAddressFields> {
|
||||
return this.resolveWhmcsAddress({
|
||||
private async lookupPostalCode(
|
||||
postcode: string
|
||||
): Promise<Awaited<ReturnType<JapanPostFacade["lookupByZipCode"]>>> {
|
||||
const inflight = this.inflightLookups.get(postcode);
|
||||
if (inflight) {
|
||||
return inflight;
|
||||
}
|
||||
|
||||
const promise = this.japanPost.lookupByZipCode(postcode);
|
||||
this.inflightLookups.set(postcode, promise);
|
||||
|
||||
try {
|
||||
return await promise;
|
||||
} finally {
|
||||
this.inflightLookups.delete(postcode);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Prepare WHMCS address fields from a full BilingualAddress.
|
||||
* Uses the already-populated English fields — no API call needed.
|
||||
*/
|
||||
resolveWhmcsAddressFromBilingual(address: BilingualAddress): WhmcsAddressFields {
|
||||
return prepareWhmcsAddressFields(address);
|
||||
}
|
||||
|
||||
/**
|
||||
* Convert WhmcsAddressFields to the shape expected by CreateWhmcsClientStep.
|
||||
*/
|
||||
toWhmcsStepAddress(fields: WhmcsAddressFields): {
|
||||
address1: string;
|
||||
address2?: string;
|
||||
city: string;
|
||||
state: string;
|
||||
postcode: string;
|
||||
country: string;
|
||||
} {
|
||||
return {
|
||||
address1: fields.address1 || "",
|
||||
...(fields.address2 && { address2: fields.address2 }),
|
||||
city: fields.city,
|
||||
state: fields.state,
|
||||
postcode: fields.postcode,
|
||||
country: fields.country,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Build case address from bilingual address.
|
||||
* Maps Japanese fields to the eligibility case step's expected format.
|
||||
*/
|
||||
toCaseAddress(address: BilingualAddress): {
|
||||
address1: string;
|
||||
city: string;
|
||||
state: string;
|
||||
postcode: string;
|
||||
country: string;
|
||||
} {
|
||||
return {
|
||||
address1: `${address.townJa}${address.streetAddress}`,
|
||||
city: address.cityJa,
|
||||
state: address.prefectureJa,
|
||||
postcode: address.postcode,
|
||||
townJa: address.townJa,
|
||||
streetAddress: address.streetAddress,
|
||||
buildingName: address.buildingName,
|
||||
roomNumber: address.roomNumber,
|
||||
residenceType: address.residenceType,
|
||||
});
|
||||
country: "Japan",
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
@ -10,6 +10,7 @@ import { DistributedLockService } from "@bff/infra/cache/distributed-lock.servic
|
||||
import { SalesforceAccountService } from "@bff/integrations/salesforce/services/salesforce-account.service.js";
|
||||
import { SalesforceFacade } from "@bff/integrations/salesforce/facades/salesforce.facade.js";
|
||||
import { extractErrorMessage } from "@bff/core/utils/error.util.js";
|
||||
import { safeOperation, OperationCriticality } from "@bff/core/utils/safe-operation.util.js";
|
||||
import {
|
||||
PORTAL_SOURCE_INTERNET_ELIGIBILITY,
|
||||
PORTAL_STATUS_NOT_YET,
|
||||
@ -101,27 +102,20 @@ export class GuestEligibilityWorkflowService {
|
||||
);
|
||||
}
|
||||
|
||||
// Write Japanese address to Salesforce
|
||||
try {
|
||||
await this.addressWriter.writeToSalesforce(sfAccountId, address);
|
||||
} catch (addressError) {
|
||||
this.logger.warn(
|
||||
{ error: extractErrorMessage(addressError), email: normalizedEmail },
|
||||
"SF address write failed (non-critical, continuing)"
|
||||
);
|
||||
}
|
||||
|
||||
// Create eligibility case via shared step
|
||||
const { caseId } = await this.eligibilityCaseStep.execute({
|
||||
sfAccountId,
|
||||
address: {
|
||||
address1: `${address.townJa}${address.streetAddress}`,
|
||||
city: address.cityJa,
|
||||
state: address.prefectureJa,
|
||||
postcode: address.postcode,
|
||||
country: "Japan",
|
||||
},
|
||||
});
|
||||
// SF address write (DEGRADABLE) + eligibility case creation — independent, run in parallel
|
||||
const [, { caseId }] = await Promise.all([
|
||||
safeOperation(async () => this.addressWriter.writeToSalesforce(sfAccountId, address), {
|
||||
criticality: OperationCriticality.OPTIONAL,
|
||||
fallback: undefined,
|
||||
context: "SF address write",
|
||||
logger: this.logger,
|
||||
metadata: { email: normalizedEmail },
|
||||
}),
|
||||
this.eligibilityCaseStep.execute({
|
||||
sfAccountId,
|
||||
address: this.addressWriter.toCaseAddress(address),
|
||||
}),
|
||||
]);
|
||||
|
||||
// If user wants to continue to account creation, generate a handoff token
|
||||
let handoffToken: string | undefined;
|
||||
|
||||
@ -8,8 +8,8 @@ import { type SignupWithEligibilityRequest } from "@customer-portal/domain/get-s
|
||||
import { DistributedLockService } from "@bff/infra/cache/distributed-lock.service.js";
|
||||
import { EmailService } from "@bff/infra/email/email.service.js";
|
||||
import { UsersService } from "@bff/modules/users/application/users.service.js";
|
||||
import { WhmcsAccountDiscoveryService } from "@bff/integrations/whmcs/services/whmcs-account-discovery.service.js";
|
||||
import { extractErrorMessage } from "@bff/core/utils/error.util.js";
|
||||
import { safeOperation, OperationCriticality } from "@bff/core/utils/safe-operation.util.js";
|
||||
import { PORTAL_SOURCE_INTERNET_ELIGIBILITY } from "@bff/modules/auth/constants/portal.constants.js";
|
||||
import type { AuthResultInternal } from "@bff/modules/auth/auth.types.js";
|
||||
import { AddressWriterService } from "@bff/modules/address/address-writer.service.js";
|
||||
@ -39,7 +39,6 @@ export class NewCustomerSignupWorkflowService {
|
||||
private readonly sessionService: GetStartedSessionService,
|
||||
private readonly lockService: DistributedLockService,
|
||||
private readonly usersService: UsersService,
|
||||
private readonly whmcsDiscovery: WhmcsAccountDiscoveryService,
|
||||
private readonly emailService: EmailService,
|
||||
private readonly sfStep: ResolveSalesforceAccountStep,
|
||||
private readonly caseStep: CreateEligibilityCaseStep,
|
||||
@ -148,39 +147,38 @@ export class NewCustomerSignupWorkflowService {
|
||||
updateSourceIfExists: true,
|
||||
});
|
||||
|
||||
// Step 1.5: Write Japanese address to Salesforce (DEGRADABLE)
|
||||
try {
|
||||
await this.addressWriter.writeToSalesforce(sfResult.sfAccountId, address);
|
||||
} catch (addressError) {
|
||||
this.logger.warn(
|
||||
{ error: extractErrorMessage(addressError), email },
|
||||
"SF address write failed (non-critical, continuing)"
|
||||
);
|
||||
}
|
||||
|
||||
// Step 2: Create eligibility case (DEGRADABLE)
|
||||
let eligibilityRequestId: string | undefined;
|
||||
try {
|
||||
const caseResult = await this.caseStep.execute({
|
||||
sfAccountId: sfResult.sfAccountId,
|
||||
address: {
|
||||
address1: `${address.townJa}${address.streetAddress}`,
|
||||
city: address.cityJa,
|
||||
state: address.prefectureJa,
|
||||
postcode: address.postcode,
|
||||
country: "Japan",
|
||||
// Steps 1.5 + 2: SF address write + eligibility case creation (both DEGRADABLE, independent)
|
||||
const [, eligibilityRequestId] = await Promise.all([
|
||||
safeOperation(
|
||||
async () => this.addressWriter.writeToSalesforce(sfResult.sfAccountId, address),
|
||||
{
|
||||
criticality: OperationCriticality.OPTIONAL,
|
||||
fallback: undefined,
|
||||
context: "SF address write",
|
||||
logger: this.logger,
|
||||
metadata: { email },
|
||||
}
|
||||
),
|
||||
safeOperation(
|
||||
async () => {
|
||||
const caseResult = await this.caseStep.execute({
|
||||
sfAccountId: sfResult.sfAccountId,
|
||||
address: this.addressWriter.toCaseAddress(address),
|
||||
});
|
||||
return caseResult.caseId;
|
||||
},
|
||||
});
|
||||
eligibilityRequestId = caseResult.caseId;
|
||||
} catch (caseError) {
|
||||
this.logger.warn(
|
||||
{ error: extractErrorMessage(caseError), email },
|
||||
"Eligibility case creation failed (non-critical, continuing)"
|
||||
);
|
||||
}
|
||||
{
|
||||
criticality: OperationCriticality.OPTIONAL,
|
||||
fallback: undefined as string | undefined,
|
||||
context: "Eligibility case creation",
|
||||
logger: this.logger,
|
||||
metadata: { email },
|
||||
}
|
||||
),
|
||||
]);
|
||||
|
||||
// Step 3: Create WHMCS client (CRITICAL, has rollback)
|
||||
const whmcsAddress = await this.addressWriter.resolveWhmcsAddressFromBilingual(address);
|
||||
const whmcsAddress = this.addressWriter.resolveWhmcsAddressFromBilingual(address);
|
||||
|
||||
const whmcsResult = await this.whmcsStep.execute({
|
||||
email,
|
||||
@ -188,14 +186,7 @@ export class NewCustomerSignupWorkflowService {
|
||||
firstName,
|
||||
lastName,
|
||||
phone: phone ?? "",
|
||||
address: {
|
||||
address1: whmcsAddress.address1 || "",
|
||||
...(whmcsAddress.address2 && { address2: whmcsAddress.address2 }),
|
||||
city: whmcsAddress.city,
|
||||
state: whmcsAddress.state,
|
||||
postcode: whmcsAddress.postcode,
|
||||
country: whmcsAddress.country,
|
||||
},
|
||||
address: this.addressWriter.toWhmcsStepAddress(whmcsAddress),
|
||||
customerNumber: sfResult.customerNumber ?? null,
|
||||
dateOfBirth,
|
||||
gender,
|
||||
@ -217,17 +208,20 @@ export class NewCustomerSignupWorkflowService {
|
||||
}
|
||||
|
||||
// Step 5: Update SF flags (DEGRADABLE)
|
||||
try {
|
||||
await this.sfFlagsStep.execute({
|
||||
sfAccountId: sfResult.sfAccountId,
|
||||
whmcsClientId: whmcsResult.whmcsClientId,
|
||||
});
|
||||
} catch (flagsError) {
|
||||
this.logger.warn(
|
||||
{ error: extractErrorMessage(flagsError), email },
|
||||
"SF flags update failed (non-critical, continuing)"
|
||||
);
|
||||
}
|
||||
await safeOperation(
|
||||
async () =>
|
||||
this.sfFlagsStep.execute({
|
||||
sfAccountId: sfResult.sfAccountId,
|
||||
whmcsClientId: whmcsResult.whmcsClientId,
|
||||
}),
|
||||
{
|
||||
criticality: OperationCriticality.OPTIONAL,
|
||||
fallback: undefined,
|
||||
context: "SF flags update",
|
||||
logger: this.logger,
|
||||
metadata: { email },
|
||||
}
|
||||
);
|
||||
|
||||
// Step 6: Generate auth result + finalize
|
||||
const authResult = await this.authResultStep.execute({
|
||||
@ -255,10 +249,9 @@ export class NewCustomerSignupWorkflowService {
|
||||
private async checkExistingAccounts(
|
||||
email: string
|
||||
): Promise<{ success: false; message: string } | null> {
|
||||
const [portalUser, whmcsClient] = await Promise.all([
|
||||
this.usersService.findByEmailInternal(email),
|
||||
this.whmcsDiscovery.findClientByEmail(email),
|
||||
]);
|
||||
// Only check Portal — the verification phase already checked WHMCS,
|
||||
// and the AddClient step will reject duplicates if one was created in the interim.
|
||||
const portalUser = await this.usersService.findByEmailInternal(email);
|
||||
|
||||
if (portalUser) {
|
||||
return {
|
||||
@ -267,14 +260,6 @@ export class NewCustomerSignupWorkflowService {
|
||||
};
|
||||
}
|
||||
|
||||
if (whmcsClient) {
|
||||
return {
|
||||
success: false,
|
||||
message:
|
||||
"A billing account already exists with this email. Please use account linking instead.",
|
||||
};
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
|
||||
@ -6,8 +6,8 @@ import { type CompleteAccountRequest } from "@customer-portal/domain/get-started
|
||||
|
||||
import { DistributedLockService } from "@bff/infra/cache/distributed-lock.service.js";
|
||||
import { UsersService } from "@bff/modules/users/application/users.service.js";
|
||||
import { WhmcsAccountDiscoveryService } from "@bff/integrations/whmcs/services/whmcs-account-discovery.service.js";
|
||||
import { extractErrorMessage } from "@bff/core/utils/error.util.js";
|
||||
import { safeOperation, OperationCriticality } from "@bff/core/utils/safe-operation.util.js";
|
||||
import { PORTAL_SOURCE_NEW_SIGNUP } from "@bff/modules/auth/constants/portal.constants.js";
|
||||
import type { AuthResultInternal } from "@bff/modules/auth/auth.types.js";
|
||||
import { AddressWriterService } from "@bff/modules/address/address-writer.service.js";
|
||||
@ -35,7 +35,6 @@ export class SfCompletionWorkflowService {
|
||||
private readonly sessionService: GetStartedSessionService,
|
||||
private readonly lockService: DistributedLockService,
|
||||
private readonly usersService: UsersService,
|
||||
private readonly whmcsDiscovery: WhmcsAccountDiscoveryService,
|
||||
private readonly sfStep: ResolveSalesforceAccountStep,
|
||||
private readonly whmcsStep: CreateWhmcsClientStep,
|
||||
private readonly portalUserStep: CreatePortalUserStep,
|
||||
@ -135,30 +134,33 @@ export class SfCompletionWorkflowService {
|
||||
source: PORTAL_SOURCE_NEW_SIGNUP,
|
||||
});
|
||||
|
||||
// Step 1.5: Write Japanese address to Salesforce for new customers (DEGRADABLE)
|
||||
if (isNewCustomer && address.prefectureJa) {
|
||||
try {
|
||||
await this.addressWriter.writeToSalesforce(
|
||||
sfResult.sfAccountId,
|
||||
address as BilingualAddress
|
||||
);
|
||||
} catch (addressError) {
|
||||
this.logger.warn(
|
||||
{ error: extractErrorMessage(addressError), email: session.email },
|
||||
"SF address write failed (non-critical, continuing)"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Step 2: Create WHMCS client (CRITICAL, has rollback)
|
||||
const whmcsAddress = await this.addressWriter.resolveWhmcsAddress({
|
||||
postcode: address.postcode,
|
||||
townJa: address.townJa,
|
||||
streetAddress: address.streetAddress || "",
|
||||
buildingName: address.buildingName,
|
||||
roomNumber: address.roomNumber,
|
||||
residenceType: address.residenceType || "house",
|
||||
});
|
||||
// Steps 1.5 + 2: SF address write (DEGRADABLE) + WHMCS address resolve (CRITICAL) — independent
|
||||
const [, whmcsAddress] = await Promise.all([
|
||||
isNewCustomer && address.prefectureJa
|
||||
? safeOperation(
|
||||
async () =>
|
||||
this.addressWriter.writeToSalesforce(
|
||||
sfResult.sfAccountId,
|
||||
address as BilingualAddress
|
||||
),
|
||||
{
|
||||
criticality: OperationCriticality.OPTIONAL,
|
||||
fallback: undefined,
|
||||
context: "SF address write",
|
||||
logger: this.logger,
|
||||
metadata: { email: session.email },
|
||||
}
|
||||
)
|
||||
: Promise.resolve(),
|
||||
this.addressWriter.resolveWhmcsAddress({
|
||||
postcode: address.postcode,
|
||||
townJa: address.townJa,
|
||||
streetAddress: address.streetAddress || "",
|
||||
buildingName: address.buildingName,
|
||||
roomNumber: address.roomNumber,
|
||||
residenceType: address.residenceType || "house",
|
||||
}),
|
||||
]);
|
||||
|
||||
const whmcsResult = await this.whmcsStep.execute({
|
||||
firstName: finalFirstName,
|
||||
@ -166,14 +168,7 @@ export class SfCompletionWorkflowService {
|
||||
email: session.email,
|
||||
password,
|
||||
phone: phone ?? "",
|
||||
address: {
|
||||
address1: whmcsAddress.address1 || "",
|
||||
...(whmcsAddress.address2 && { address2: whmcsAddress.address2 }),
|
||||
city: whmcsAddress.city,
|
||||
state: whmcsAddress.state,
|
||||
postcode: whmcsAddress.postcode,
|
||||
country: whmcsAddress.country,
|
||||
},
|
||||
address: this.addressWriter.toWhmcsStepAddress(whmcsAddress),
|
||||
customerNumber: sfResult.customerNumber ?? null,
|
||||
dateOfBirth,
|
||||
gender,
|
||||
@ -195,17 +190,20 @@ export class SfCompletionWorkflowService {
|
||||
}
|
||||
|
||||
// Step 4: Update SF flags (DEGRADABLE)
|
||||
try {
|
||||
await this.sfFlagsStep.execute({
|
||||
sfAccountId: sfResult.sfAccountId,
|
||||
whmcsClientId: whmcsResult.whmcsClientId,
|
||||
});
|
||||
} catch (flagsError) {
|
||||
this.logger.warn(
|
||||
{ error: extractErrorMessage(flagsError), email: session.email },
|
||||
"SF flags update failed (non-critical, continuing)"
|
||||
);
|
||||
}
|
||||
await safeOperation(
|
||||
async () =>
|
||||
this.sfFlagsStep.execute({
|
||||
sfAccountId: sfResult.sfAccountId,
|
||||
whmcsClientId: whmcsResult.whmcsClientId,
|
||||
}),
|
||||
{
|
||||
criticality: OperationCriticality.OPTIONAL,
|
||||
fallback: undefined,
|
||||
context: "SF flags update",
|
||||
logger: this.logger,
|
||||
metadata: { email: session.email },
|
||||
}
|
||||
);
|
||||
|
||||
// Step 5: Generate auth result
|
||||
const authResult = await this.authResultStep.execute({
|
||||
@ -226,16 +224,10 @@ export class SfCompletionWorkflowService {
|
||||
}
|
||||
|
||||
private async ensureNoExistingAccounts(email: string): Promise<void> {
|
||||
const [portalUser, whmcsClient] = await Promise.all([
|
||||
this.usersService.findByEmailInternal(email),
|
||||
this.whmcsDiscovery.findClientByEmail(email),
|
||||
]);
|
||||
// Only check Portal — the verification phase already checked WHMCS,
|
||||
// and the AddClient step will reject duplicates if one was created in the interim.
|
||||
const portalUser = await this.usersService.findByEmailInternal(email);
|
||||
|
||||
if (whmcsClient) {
|
||||
throw new ConflictException(
|
||||
"A billing account already exists. Please use the account migration flow."
|
||||
);
|
||||
}
|
||||
if (portalUser) {
|
||||
throw new ConflictException("An account already exists. Please log in.");
|
||||
}
|
||||
|
||||
@ -19,6 +19,7 @@ import { LoginOtpStep } from "../LoginOtpStep";
|
||||
import { useLoginWithOtp } from "../../hooks/use-auth";
|
||||
import { loginRequestSchema } from "@customer-portal/domain/auth";
|
||||
import { useZodForm } from "@/shared/hooks";
|
||||
import { parseError } from "@/shared/utils";
|
||||
import { z } from "zod";
|
||||
import { getSafeRedirect } from "@/features/auth/utils/route-protection";
|
||||
|
||||
@ -94,8 +95,7 @@ export function LoginForm({
|
||||
onSuccess?.();
|
||||
}
|
||||
} catch (err) {
|
||||
const message = err instanceof Error ? err.message : "Login failed";
|
||||
onError?.(message);
|
||||
onError?.(parseError(err).message);
|
||||
throw err;
|
||||
}
|
||||
},
|
||||
@ -112,8 +112,7 @@ export function LoginForm({
|
||||
await verifyOtp(otpState.sessionToken, code, rememberDevice);
|
||||
onSuccess?.();
|
||||
} catch (err) {
|
||||
const message = err instanceof Error ? err.message : "Verification failed";
|
||||
onError?.(message);
|
||||
onError?.(parseError(err).message);
|
||||
throw err;
|
||||
}
|
||||
},
|
||||
|
||||
@ -13,6 +13,7 @@ import { PasswordRequirements, PasswordMatchIndicator } from "..";
|
||||
import { usePasswordReset } from "../../hooks/use-auth";
|
||||
import { usePasswordValidation } from "../../hooks/usePasswordValidation";
|
||||
import { useZodForm } from "@/shared/hooks";
|
||||
import { parseError } from "@/shared/utils";
|
||||
import { passwordResetRequestSchema, passwordResetSchema } from "@customer-portal/domain/auth";
|
||||
import { z } from "zod";
|
||||
|
||||
@ -66,8 +67,7 @@ export function PasswordResetForm({
|
||||
await requestPasswordReset(data.email);
|
||||
onSuccess?.();
|
||||
} catch (err) {
|
||||
const errorMessage = err instanceof Error ? err.message : "Request failed";
|
||||
onError?.(errorMessage);
|
||||
onError?.(parseError(err).message);
|
||||
throw err;
|
||||
}
|
||||
},
|
||||
@ -82,8 +82,7 @@ export function PasswordResetForm({
|
||||
await resetPassword(data.token, data.password);
|
||||
onSuccess?.();
|
||||
} catch (err) {
|
||||
const errorMessage = err instanceof Error ? err.message : "Reset failed";
|
||||
onError?.(errorMessage);
|
||||
onError?.(parseError(err).message);
|
||||
throw err;
|
||||
}
|
||||
},
|
||||
|
||||
@ -12,6 +12,7 @@ import { PasswordRequirements, PasswordMatchIndicator } from "..";
|
||||
import { useAuth } from "../../hooks/use-auth";
|
||||
import { usePasswordValidation } from "../../hooks/usePasswordValidation";
|
||||
import { useZodForm } from "@/shared/hooks";
|
||||
import { parseError } from "@/shared/utils";
|
||||
import { setPasswordRequestSchema, getPasswordStrengthDisplay } from "@customer-portal/domain/auth";
|
||||
import { z } from "zod";
|
||||
|
||||
@ -47,7 +48,7 @@ export function SetPasswordForm({
|
||||
await setPassword(data.email, data.password);
|
||||
onSuccess?.();
|
||||
} catch (err) {
|
||||
onError?.(err instanceof Error ? err.message : "Failed to set password");
|
||||
onError?.(parseError(err).message);
|
||||
throw err;
|
||||
}
|
||||
},
|
||||
|
||||
@ -302,10 +302,8 @@ export const useAuthStore = create<AuthState>()((set, get) => {
|
||||
}
|
||||
applyAuthResponse(parsed.data);
|
||||
} catch (error) {
|
||||
set({
|
||||
loading: false,
|
||||
error: error instanceof Error ? error.message : "Signup failed",
|
||||
});
|
||||
const parsed = parseError(error);
|
||||
set({ loading: false, error: parsed.message });
|
||||
throw error;
|
||||
}
|
||||
},
|
||||
@ -340,10 +338,8 @@ export const useAuthStore = create<AuthState>()((set, get) => {
|
||||
});
|
||||
set({ loading: false });
|
||||
} catch (error) {
|
||||
set({
|
||||
loading: false,
|
||||
error: error instanceof Error ? error.message : "Password reset request failed",
|
||||
});
|
||||
const parsed = parseError(error);
|
||||
set({ loading: false, error: parsed.message });
|
||||
throw error;
|
||||
}
|
||||
},
|
||||
@ -364,10 +360,8 @@ export const useAuthStore = create<AuthState>()((set, get) => {
|
||||
error: null,
|
||||
});
|
||||
} catch (error) {
|
||||
set({
|
||||
loading: false,
|
||||
error: error instanceof Error ? error.message : "Password reset failed",
|
||||
});
|
||||
const parsed = parseError(error);
|
||||
set({ loading: false, error: parsed.message });
|
||||
throw error;
|
||||
}
|
||||
},
|
||||
@ -384,10 +378,8 @@ export const useAuthStore = create<AuthState>()((set, get) => {
|
||||
}
|
||||
applyAuthResponse(parsed.data);
|
||||
} catch (error) {
|
||||
set({
|
||||
loading: false,
|
||||
error: error instanceof Error ? error.message : "Password change failed",
|
||||
});
|
||||
const parsed = parseError(error);
|
||||
set({ loading: false, error: parsed.message });
|
||||
throw error;
|
||||
}
|
||||
},
|
||||
@ -408,10 +400,8 @@ export const useAuthStore = create<AuthState>()((set, get) => {
|
||||
set({ loading: false });
|
||||
return parsed.data;
|
||||
} catch (error) {
|
||||
set({
|
||||
loading: false,
|
||||
error: error instanceof Error ? error.message : "Check failed",
|
||||
});
|
||||
const parsed = parseError(error);
|
||||
set({ loading: false, error: parsed.message });
|
||||
throw error;
|
||||
}
|
||||
},
|
||||
@ -429,10 +419,8 @@ export const useAuthStore = create<AuthState>()((set, get) => {
|
||||
}
|
||||
applyAuthResponse(parsed.data);
|
||||
} catch (error) {
|
||||
set({
|
||||
loading: false,
|
||||
error: error instanceof Error ? error.message : "Set password failed",
|
||||
});
|
||||
const parsed = parseError(error);
|
||||
set({ loading: false, error: parsed.message });
|
||||
throw error;
|
||||
}
|
||||
},
|
||||
|
||||
@ -6,6 +6,7 @@
|
||||
import { useCallback, useMemo, useRef, useState } from "react";
|
||||
import type { FormEvent } from "react";
|
||||
import { ZodError, type ZodIssue, type ZodType } from "zod";
|
||||
import { parseError } from "@/shared/utils/error-handling";
|
||||
|
||||
export type FormErrors<_TValues extends Record<string, unknown>> = Record<
|
||||
string,
|
||||
@ -212,9 +213,23 @@ export function useZodForm<TValues extends Record<string, unknown>>({
|
||||
try {
|
||||
await onSubmit(values);
|
||||
} catch (error) {
|
||||
const message = error instanceof Error ? error.message : String(error);
|
||||
const parsed = parseError(error);
|
||||
const message = parsed.message;
|
||||
setSubmitError(message);
|
||||
setErrors(prev => ({ ...prev, ["_form"]: message }));
|
||||
|
||||
if (parsed.fieldErrors && Object.keys(parsed.fieldErrors).length > 0) {
|
||||
// Merge server field errors into form errors and mark those fields as touched
|
||||
setErrors(prev => ({ ...prev, ...parsed.fieldErrors, ["_form"]: message }));
|
||||
setTouchedState(prev => {
|
||||
const next = { ...prev };
|
||||
for (const key of Object.keys(parsed.fieldErrors!)) {
|
||||
next[key] = true;
|
||||
}
|
||||
return next;
|
||||
});
|
||||
} else {
|
||||
setErrors(prev => ({ ...prev, ["_form"]: message }));
|
||||
}
|
||||
} finally {
|
||||
setIsSubmitting(false);
|
||||
}
|
||||
|
||||
@ -23,6 +23,7 @@ export interface ParsedError {
|
||||
message: string;
|
||||
shouldLogout: boolean;
|
||||
shouldRetry: boolean;
|
||||
fieldErrors?: Record<string, string> | undefined;
|
||||
}
|
||||
|
||||
// ============================================================================
|
||||
@ -78,11 +79,13 @@ function parseApiError(error: ClientApiError): ParsedError {
|
||||
: ErrorCode.UNKNOWN;
|
||||
|
||||
const metadata = ErrorMetadata[resolvedCode] ?? ErrorMetadata[ErrorCode.UNKNOWN];
|
||||
const fieldErrors = extractFieldErrors(domainError.error.details);
|
||||
return {
|
||||
code: resolvedCode,
|
||||
message: domainError.error.message,
|
||||
shouldLogout: metadata.shouldLogout,
|
||||
shouldRetry: metadata.shouldRetry,
|
||||
...(fieldErrors && { fieldErrors }),
|
||||
};
|
||||
}
|
||||
|
||||
@ -97,6 +100,26 @@ function parseApiError(error: ClientApiError): ParsedError {
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract fieldErrors from error details if present and valid.
|
||||
* Validates that the value is a Record<string, string> at runtime.
|
||||
*/
|
||||
function extractFieldErrors(details: unknown): Record<string, string> | undefined {
|
||||
if (!details || typeof details !== "object" || Array.isArray(details)) return undefined;
|
||||
|
||||
const raw = (details as Record<string, unknown>)["fieldErrors"];
|
||||
if (!raw || typeof raw !== "object" || Array.isArray(raw)) return undefined;
|
||||
|
||||
const result: Record<string, string> = {};
|
||||
for (const [key, value] of Object.entries(raw as Record<string, unknown>)) {
|
||||
if (typeof value === "string") {
|
||||
result[key] = value;
|
||||
}
|
||||
}
|
||||
|
||||
return Object.keys(result).length > 0 ? result : undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse native JavaScript errors (network, timeout, etc.)
|
||||
*/
|
||||
|
||||
242
docs/plans/2026-03-03-signup-flow-simplification-design.md
Normal file
242
docs/plans/2026-03-03-signup-flow-simplification-design.md
Normal file
@ -0,0 +1,242 @@
|
||||
# Auth Flow Simplification Design
|
||||
|
||||
**Date**: 2026-03-03
|
||||
**Status**: Approved
|
||||
|
||||
## Problem
|
||||
|
||||
The auth system has accumulated duplication and dead code:
|
||||
|
||||
1. **Duplicate signup workflows**: `SfCompletionWorkflow` and `NewCustomerSignupWorkflow` are 85% identical (~300 lines each). The only difference is eligibility case creation and a welcome email.
|
||||
2. **Confusing names**: `SfCompletionWorkflow` doesn't describe what it does (it creates full accounts, not just "completes SF").
|
||||
3. **Missing SF fields**: `Birthdate` and `Sex__c` are not set on Salesforce PersonContact during account creation. The only code that called `createContact()` lived in dead legacy code.
|
||||
4. **Dead code**: `signup/` directory (6 files), `WhmcsLinkWorkflowService` — all unreferenced or superseded.
|
||||
5. **Legacy duplicate**: `WhmcsLinkWorkflowService` (password-based migration via `POST /auth/migrate`) is superseded by `WhmcsMigrationWorkflowService` (OTP-based migration via get-started).
|
||||
6. **Scattered shared services**: `SignupUserCreationService` and `SignupWhmcsService` live in `signup/` but are used by other workflows via `CreatePortalUserStep`. They should live alongside the steps they serve.
|
||||
|
||||
## Approach — 3 Phases (can stop after any phase)
|
||||
|
||||
### Phase 1: Merge signup workflows + cleanup
|
||||
|
||||
Merge the two near-duplicate workflows. Move shared services. Delete dead code. Fix Birthdate/Sex\_\_c.
|
||||
|
||||
### Phase 2: Remove legacy migrate endpoint
|
||||
|
||||
Delete `WhmcsLinkWorkflowService` and `POST /auth/migrate`. All WHMCS migration goes through the get-started OTP-based flow.
|
||||
|
||||
### Phase 3: Clean up OTP infrastructure
|
||||
|
||||
Extract shared OTP email-sending pattern. Login and signup OTP stay as separate workflows (they serve different purposes: 2FA vs email verification). Keep separate sessions (different data models, TTLs, lifecycle). Improve the PORTAL_EXISTS redirect UX (prefill email on login page).
|
||||
|
||||
**Why login and signup stay separate**: In the ISP/telecom industry, login (password → OTP as 2FA) and signup (OTP as email verification → form) serve fundamentally different security purposes. Forcing them into one flow either adds friction for existing users or weakens the security model. Major ISP portals keep these flows separate.
|
||||
|
||||
---
|
||||
|
||||
## Current Structure
|
||||
|
||||
```
|
||||
workflows/
|
||||
├── verification-workflow.service.ts # OTP + account status detection
|
||||
├── guest-eligibility-workflow.service.ts # Guest ISP check (no account created)
|
||||
├── sf-completion-workflow.service.ts # Creates account (misleading name, 85% overlap)
|
||||
├── new-customer-signup-workflow.service.ts # Creates account + eligibility (85% overlap)
|
||||
├── whmcs-migration-workflow.service.ts # OTP-based WHMCS migration (active)
|
||||
├── whmcs-link-workflow.service.ts # Password-based WHMCS migration (legacy, superseded)
|
||||
├── login-otp-workflow.service.ts # Login OTP 2FA
|
||||
├── password-workflow.service.ts # Password operations
|
||||
├── get-started-coordinator.service.ts # Get-started routing
|
||||
├── workflow-error.util.ts # Error classification
|
||||
├── signup/ # Mixed: some active, some dead
|
||||
│ ├── signup-account-resolver.service.ts # Dead (no active caller)
|
||||
│ ├── signup-validation.service.ts # Dead (no active caller)
|
||||
│ ├── signup-whmcs.service.ts # Active (markClientForCleanup used by CreatePortalUserStep)
|
||||
│ ├── signup-user-creation.service.ts # Active (used by CreatePortalUserStep + WhmcsMigration)
|
||||
│ ├── signup.types.ts # Dead (only used by dead validation service)
|
||||
│ └── index.ts # Barrel
|
||||
└── steps/ # Shared step services
|
||||
├── resolve-salesforce-account.step.ts
|
||||
├── create-whmcs-client.step.ts
|
||||
├── create-portal-user.step.ts # Delegates to SignupUserCreationService
|
||||
├── create-eligibility-case.step.ts
|
||||
├── update-salesforce-flags.step.ts
|
||||
├── generate-auth-result.step.ts
|
||||
└── index.ts
|
||||
```
|
||||
|
||||
## New Structure (after all 3 phases)
|
||||
|
||||
```
|
||||
workflows/
|
||||
├── verification-workflow.service.ts # OTP + account detection (unchanged)
|
||||
├── guest-eligibility-workflow.service.ts # Guest ISP check (unchanged)
|
||||
├── account-creation-workflow.service.ts # MERGED: replaces sf-completion + new-customer-signup
|
||||
├── account-migration-workflow.service.ts # RENAMED: from whmcs-migration
|
||||
├── login-otp-workflow.service.ts # Login OTP 2FA (Phase 3: uses shared OTP orchestration)
|
||||
├── password-workflow.service.ts # Unchanged
|
||||
├── get-started-coordinator.service.ts # Updated imports
|
||||
├── workflow-error.util.ts # Unchanged
|
||||
└── steps/ # Shared steps + moved services
|
||||
├── resolve-salesforce-account.step.ts
|
||||
├── create-whmcs-client.step.ts
|
||||
├── create-portal-user.step.ts
|
||||
├── create-eligibility-case.step.ts
|
||||
├── update-salesforce-flags.step.ts
|
||||
├── generate-auth-result.step.ts
|
||||
├── portal-user-creation.service.ts # MOVED from signup/ (renamed)
|
||||
├── whmcs-cleanup.service.ts # MOVED from signup/ (renamed, trimmed)
|
||||
└── index.ts
|
||||
```
|
||||
|
||||
**Deleted (all phases combined):**
|
||||
|
||||
- `sf-completion-workflow.service.ts` (merged, Phase 1)
|
||||
- `new-customer-signup-workflow.service.ts` (merged, Phase 1)
|
||||
- `whmcs-migration-workflow.service.ts` (renamed, Phase 1)
|
||||
- `whmcs-link-workflow.service.ts` (removed, Phase 2)
|
||||
- `signup/` entire directory (Phase 1)
|
||||
|
||||
---
|
||||
|
||||
## Phase 1: Merge signup workflows + cleanup
|
||||
|
||||
### Merged Workflow: AccountCreationWorkflowService
|
||||
|
||||
**API:**
|
||||
|
||||
```typescript
|
||||
class AccountCreationWorkflowService {
|
||||
execute(
|
||||
request: CompleteAccountRequest | SignupWithEligibilityRequest,
|
||||
options?: { withEligibility?: boolean }
|
||||
): Promise<AuthResultInternal | SignupWithEligibilityResult>;
|
||||
}
|
||||
```
|
||||
|
||||
**Routing:**
|
||||
|
||||
- `POST /auth/get-started/complete-account` → `execute(request)` (no eligibility)
|
||||
- `POST /auth/get-started/signup-with-eligibility` → `execute(request, { withEligibility: true })`
|
||||
|
||||
**Step Sequence:**
|
||||
|
||||
```
|
||||
1. Validate + check existing accounts
|
||||
2. Hash password
|
||||
3. Resolve address + names (from request or session prefill)
|
||||
4. Step 1: Resolve SF Account (CRITICAL)
|
||||
5. Step 1.5: Write SF Address (DEGRADABLE)
|
||||
6. Step 1.6: Create SF Contact — Birthdate + Sex__c (DEGRADABLE) ← NEW, fixes the bug
|
||||
7. [if withEligibility] Step 2: Create Eligibility Case (DEGRADABLE)
|
||||
8. Step 3: Create WHMCS Client (CRITICAL, rollback)
|
||||
9. Step 4: Create Portal User (CRITICAL, rollback WHMCS on fail)
|
||||
10. Step 5: Update SF Flags (DEGRADABLE)
|
||||
11. Step 6: Generate Auth Result
|
||||
12. [if withEligibility] Send welcome email with eligibility ref (DEGRADABLE)
|
||||
13. Invalidate session
|
||||
```
|
||||
|
||||
**Error handling difference between paths:**
|
||||
|
||||
- `completeAccount` path: throws exceptions directly (current sf-completion behavior)
|
||||
- `signupWithEligibility` path: wraps in try/catch, returns `{ success: false, message, errorCategory }` using `classifyError()`
|
||||
|
||||
**Birthdate/Sex\_\_c Fix:**
|
||||
After resolving the SF account, call `SalesforceAccountService.createContact()` on the PersonContact:
|
||||
|
||||
- `Birthdate` → `dateOfBirth` (YYYY-MM-DD)
|
||||
- `Sex__c` → mapped from `gender` (`male`→`男性`, `female`→`女性`, `other`→`Other`)
|
||||
This is degradable — account creation continues if it fails.
|
||||
|
||||
### Rename: AccountMigrationWorkflowService
|
||||
|
||||
Rename `WhmcsMigrationWorkflowService` → `AccountMigrationWorkflowService`. Refactor to use `CreatePortalUserStep` instead of calling `SignupUserCreationService` directly (reuse existing step, same as other workflows).
|
||||
|
||||
### Move signup/ services into steps/
|
||||
|
||||
| Old | New | Changes |
|
||||
| ---------------------------------------- | --------------------------------------- | ------------------------------------------------------------------- |
|
||||
| `signup/signup-user-creation.service.ts` | `steps/portal-user-creation.service.ts` | Rename class to `PortalUserCreationService` |
|
||||
| `signup/signup-whmcs.service.ts` | `steps/whmcs-cleanup.service.ts` | Rename to `WhmcsCleanupService`, keep only `markClientForCleanup()` |
|
||||
|
||||
### Delete dead code
|
||||
|
||||
| File | Reason |
|
||||
| ------------------------------------------- | ------------------------------------ |
|
||||
| `signup/signup-account-resolver.service.ts` | No active caller |
|
||||
| `signup/signup-validation.service.ts` | No active caller |
|
||||
| `signup/signup.types.ts` | Only used by dead validation service |
|
||||
| `signup/index.ts` | Directory being removed |
|
||||
|
||||
---
|
||||
|
||||
## Phase 2: Remove legacy migrate endpoint
|
||||
|
||||
### Delete WhmcsLinkWorkflowService
|
||||
|
||||
`WhmcsLinkWorkflowService` (`POST /auth/migrate`) is superseded by `AccountMigrationWorkflowService` (`POST /auth/get-started/migrate-whmcs-account`). The new flow uses OTP-based identity verification instead of WHMCS password validation — strictly more secure.
|
||||
|
||||
**Files to modify:**
|
||||
|
||||
- `apps/bff/src/modules/auth/auth.module.ts` — remove provider + import
|
||||
- `apps/bff/src/modules/auth/application/auth-orchestrator.service.ts` — remove `linkWhmcsUser()` method + dependency
|
||||
- `apps/bff/src/modules/auth/presentation/http/auth.controller.ts` — remove `POST /auth/migrate` endpoint
|
||||
|
||||
**File to delete:**
|
||||
|
||||
- `apps/bff/src/modules/auth/infra/workflows/whmcs-link-workflow.service.ts`
|
||||
|
||||
---
|
||||
|
||||
## Phase 3: Clean up OTP infrastructure
|
||||
|
||||
### Extract shared OTP email-sending pattern
|
||||
|
||||
Both `LoginOtpWorkflowService` and `VerificationWorkflowService` implement the same pattern:
|
||||
|
||||
1. Look up email template ID from config
|
||||
2. If template exists, send via template with dynamic data
|
||||
3. If no template, send fallback HTML
|
||||
|
||||
Extract this into a shared `OtpEmailService` (or method on existing `OtpService`).
|
||||
|
||||
### Improve PORTAL_EXISTS redirect UX
|
||||
|
||||
When get-started verification detects `PORTAL_EXISTS`, pass the verified email to the login page so it's prefilled. This reduces friction for users who accidentally start on the wrong page.
|
||||
|
||||
### What stays separate (and why)
|
||||
|
||||
- **Login OTP workflow**: OTP serves as 2FA after password validation
|
||||
- **Verification OTP workflow**: OTP serves as email ownership proof before signup
|
||||
- **LoginSessionService**: Simple (5 fields, 10min TTL, no prefill)
|
||||
- **GetStartedSessionService**: Complex (10+ fields, 1hr TTL, prefill data, handoff tokens, idempotency locking)
|
||||
|
||||
These serve fundamentally different security purposes and have different data models. Forcing unification would create a confusing abstraction.
|
||||
|
||||
---
|
||||
|
||||
## Module Updates (all phases)
|
||||
|
||||
### GetStartedModule (Phase 1)
|
||||
|
||||
- Remove: `NewCustomerSignupWorkflowService`, `SfCompletionWorkflowService`, `WhmcsMigrationWorkflowService`
|
||||
- Remove: `SignupAccountResolverService`, `SignupValidationService`, `SignupWhmcsService`, `SignupUserCreationService`
|
||||
- Add: `AccountCreationWorkflowService`, `AccountMigrationWorkflowService`
|
||||
- Add: `PortalUserCreationService`, `WhmcsCleanupService`
|
||||
|
||||
### GetStartedCoordinator (Phase 1)
|
||||
|
||||
- `completeAccount()` → `accountCreation.execute(request)`
|
||||
- `signupWithEligibility()` → `accountCreation.execute(request, { withEligibility: true })`
|
||||
- `migrateWhmcsAccount()` → `accountMigration.execute(request)`
|
||||
|
||||
### AuthModule (Phase 2)
|
||||
|
||||
- Remove: `WhmcsLinkWorkflowService` from providers and imports
|
||||
|
||||
### AuthOrchestrator (Phase 2)
|
||||
|
||||
- Remove: `linkWhmcsUser()` method and `WhmcsLinkWorkflowService` dependency
|
||||
|
||||
### AuthController (Phase 2)
|
||||
|
||||
- Remove: `POST /auth/migrate` endpoint
|
||||
508
docs/plans/2026-03-03-signup-flow-simplification-plan.md
Normal file
508
docs/plans/2026-03-03-signup-flow-simplification-plan.md
Normal file
@ -0,0 +1,508 @@
|
||||
# Auth Flow Simplification Implementation Plan
|
||||
|
||||
> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task.
|
||||
|
||||
**Goal:** Simplify auth flows: merge duplicate signup workflows, fix missing SF Birthdate/Sex\_\_c, remove legacy migrate endpoint, clean up OTP infrastructure, delete dead code.
|
||||
|
||||
**Architecture:** 3 phases (can stop after any phase). Phase 1 merges signup workflows + cleans up. Phase 2 removes legacy WHMCS migration. Phase 3 extracts shared OTP pattern.
|
||||
|
||||
**Tech Stack:** NestJS 11, TypeScript, Prisma, Salesforce jsforce, WHMCS API, argon2, BullMQ
|
||||
|
||||
**Design doc:** `docs/plans/2026-03-03-signup-flow-simplification-design.md`
|
||||
|
||||
---
|
||||
|
||||
# Phase 1: Merge signup workflows + cleanup
|
||||
|
||||
## Task 1: Move signup shared services into steps/
|
||||
|
||||
Move `SignupUserCreationService` and `SignupWhmcsService` from `signup/` into `steps/`, renaming to remove the "Signup" prefix. Update all imports.
|
||||
|
||||
**Files:**
|
||||
|
||||
- Create: `apps/bff/src/modules/auth/infra/workflows/steps/portal-user-creation.service.ts`
|
||||
- Create: `apps/bff/src/modules/auth/infra/workflows/steps/whmcs-cleanup.service.ts`
|
||||
- Modify: `apps/bff/src/modules/auth/infra/workflows/steps/create-portal-user.step.ts`
|
||||
- Modify: `apps/bff/src/modules/auth/infra/workflows/steps/index.ts`
|
||||
- Modify: `apps/bff/src/modules/auth/infra/workflows/whmcs-migration-workflow.service.ts`
|
||||
- Modify: `apps/bff/src/modules/auth/get-started/get-started.module.ts`
|
||||
|
||||
**Step 1: Create `portal-user-creation.service.ts`**
|
||||
|
||||
Copy `signup/signup-user-creation.service.ts` to `steps/portal-user-creation.service.ts`:
|
||||
|
||||
- Rename class: `SignupUserCreationService` → `PortalUserCreationService`
|
||||
- Update import: `SignupWhmcsService` → `WhmcsCleanupService` from `./whmcs-cleanup.service.js`
|
||||
|
||||
**Step 2: Create `whmcs-cleanup.service.ts`**
|
||||
|
||||
Copy `signup/signup-whmcs.service.ts` to `steps/whmcs-cleanup.service.ts`:
|
||||
|
||||
- Rename class: `SignupWhmcsService` → `WhmcsCleanupService`
|
||||
- Keep only `markClientForCleanup()` method
|
||||
- Delete: `createClient()`, `checkExistingClient()`, `validateAddressData()` (unused by active code)
|
||||
- Remove unused imports: `SignupRequest`, `serializeWhmcsKeyValueMap`, `DomainHttpException`, `ErrorCode`, `ConfigService`, `MappingsService`, `WhmcsAccountDiscoveryService`
|
||||
- Constructor only needs: `WhmcsClientService` + `Logger`
|
||||
|
||||
**Step 3: Update `create-portal-user.step.ts` import**
|
||||
|
||||
```
|
||||
Line 3 — FROM: import { SignupUserCreationService } from "../signup/signup-user-creation.service.js";
|
||||
Line 3 — TO: import { PortalUserCreationService } from "./portal-user-creation.service.js";
|
||||
```
|
||||
|
||||
Update constructor param: `signupUserCreation` → `portalUserCreation`, update usage on line 36.
|
||||
|
||||
**Step 4: Update `steps/index.ts` barrel**
|
||||
|
||||
Add at end:
|
||||
|
||||
```typescript
|
||||
export { PortalUserCreationService } from "./portal-user-creation.service.js";
|
||||
export type { UserCreationParams, CreatedUserResult } from "./portal-user-creation.service.js";
|
||||
export { WhmcsCleanupService } from "./whmcs-cleanup.service.js";
|
||||
```
|
||||
|
||||
**Step 5: Update `whmcs-migration-workflow.service.ts` import**
|
||||
|
||||
```
|
||||
Line 24 — FROM: import { SignupUserCreationService } from "./signup/signup-user-creation.service.js";
|
||||
Line 24 — TO: import { PortalUserCreationService } from "./steps/portal-user-creation.service.js";
|
||||
```
|
||||
|
||||
Update constructor param and usage accordingly. (Task 3 will refactor this further to use `CreatePortalUserStep`.)
|
||||
|
||||
**Step 6: Update `get-started.module.ts`**
|
||||
|
||||
- Remove imports: `SignupAccountResolverService`, `SignupValidationService`, `SignupWhmcsService`, `SignupUserCreationService`
|
||||
- Add imports: `PortalUserCreationService`, `WhmcsCleanupService` from `../infra/workflows/steps/index.js`
|
||||
- Remove from providers: `SignupAccountResolverService`, `SignupValidationService`, `SignupWhmcsService`, `SignupUserCreationService`
|
||||
- Add to providers: `PortalUserCreationService`, `WhmcsCleanupService`
|
||||
|
||||
**Step 7: Run type-check**
|
||||
|
||||
Run: `pnpm type-check`
|
||||
Expected: PASS
|
||||
|
||||
**Step 8: Commit**
|
||||
|
||||
```bash
|
||||
git add apps/bff/src/modules/auth/infra/workflows/steps/portal-user-creation.service.ts \
|
||||
apps/bff/src/modules/auth/infra/workflows/steps/whmcs-cleanup.service.ts \
|
||||
apps/bff/src/modules/auth/infra/workflows/steps/create-portal-user.step.ts \
|
||||
apps/bff/src/modules/auth/infra/workflows/steps/index.ts \
|
||||
apps/bff/src/modules/auth/infra/workflows/whmcs-migration-workflow.service.ts \
|
||||
apps/bff/src/modules/auth/get-started/get-started.module.ts
|
||||
git commit -m "refactor: move signup shared services into steps directory"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 2: Create merged AccountCreationWorkflowService
|
||||
|
||||
Merge `SfCompletionWorkflowService` + `NewCustomerSignupWorkflowService` into one workflow. Fix Birthdate/Sex\_\_c gap.
|
||||
|
||||
**Files:**
|
||||
|
||||
- Create: `apps/bff/src/modules/auth/infra/workflows/account-creation-workflow.service.ts`
|
||||
|
||||
**Step 1: Create the merged workflow**
|
||||
|
||||
Constructor dependencies (union of both old workflows):
|
||||
|
||||
- `ConfigService` (email template IDs, APP_BASE_URL)
|
||||
- `GetStartedSessionService`
|
||||
- `DistributedLockService`
|
||||
- `UsersService`
|
||||
- `EmailService` (welcome email in eligibility path)
|
||||
- `SalesforceAccountService` (for `createContact()` — Birthdate/Sex\_\_c)
|
||||
- `AddressWriterService`
|
||||
- `ResolveSalesforceAccountStep`
|
||||
- `CreateEligibilityCaseStep`
|
||||
- `CreateWhmcsClientStep`
|
||||
- `CreatePortalUserStep`
|
||||
- `UpdateSalesforceFlagsStep`
|
||||
- `GenerateAuthResultStep`
|
||||
- `Logger`
|
||||
|
||||
Public API:
|
||||
|
||||
```typescript
|
||||
execute(
|
||||
request: CompleteAccountRequest | SignupWithEligibilityRequest,
|
||||
options?: { withEligibility?: boolean }
|
||||
): Promise<AuthResultInternal | SignupWithEligibilityResult>
|
||||
```
|
||||
|
||||
Step sequence in `executeCreation()`:
|
||||
|
||||
1. Check existing accounts (Portal user duplicate check)
|
||||
2. Hash password (argon2)
|
||||
3. Resolve address + names (from request or session prefill — port `resolveAddress()` and `resolveNames()` from sf-completion)
|
||||
4. **Step 1: Resolve SF Account** (CRITICAL) — `source` depends on `withEligibility`
|
||||
5. **Step 1.5: Write SF Address** (DEGRADABLE via `safeOperation`)
|
||||
6. **Step 1.6: Create SF Contact** (DEGRADABLE via `safeOperation`) — **NEW: fixes Birthdate/Sex\_\_c**
|
||||
- Call `salesforceAccountService.createContact({ accountId, firstName, lastName, email, phone, gender, dateOfBirth })`
|
||||
7. **Step 2** (conditional): Create Eligibility Case (DEGRADABLE, only if `withEligibility`)
|
||||
8. **Step 3: Create WHMCS Client** (CRITICAL)
|
||||
9. **Step 4: Create Portal User** (CRITICAL, rollback WHMCS on fail)
|
||||
10. **Step 5: Update SF Flags** (DEGRADABLE)
|
||||
11. **Step 6: Generate Auth Result**
|
||||
12. (conditional) Send welcome email (DEGRADABLE, only if `withEligibility`) — port `sendWelcomeWithEligibilityEmail()` from new-customer-signup
|
||||
13. Invalidate session
|
||||
|
||||
Error handling per path:
|
||||
|
||||
- `withEligibility=true`: wrap in try/catch, return `{ success: false, errorCategory, message }` using `classifyError()` (from `workflow-error.util.ts`)
|
||||
- `withEligibility=false` (default): let exceptions propagate directly (matching current sf-completion behavior)
|
||||
|
||||
Session handling:
|
||||
|
||||
- Use `acquireAndMarkAsUsed(sessionToken, withEligibility ? "signup_with_eligibility" : "complete_account")`
|
||||
- `isNewCustomer = !session.sfAccountId` (same logic as sf-completion)
|
||||
- For `isNewCustomer`: require firstName, lastName, address from request
|
||||
- For SF_UNMAPPED: use session prefill data as fallback
|
||||
|
||||
Port private helpers from sf-completion:
|
||||
|
||||
- `validateRequest()` — checks isNewCustomer needs firstName/lastName/address
|
||||
- `ensureNoExistingAccounts()` — Portal user duplicate check
|
||||
- `resolveAddress()` — from request or session
|
||||
- `resolveNames()` — from request or session
|
||||
|
||||
**Step 2: Run type-check**
|
||||
|
||||
Run: `pnpm type-check`
|
||||
Expected: PASS
|
||||
|
||||
**Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add apps/bff/src/modules/auth/infra/workflows/account-creation-workflow.service.ts
|
||||
git commit -m "feat: add merged AccountCreationWorkflowService with Birthdate/Sex__c fix"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 3: Rename and refactor AccountMigrationWorkflowService
|
||||
|
||||
Rename `WhmcsMigrationWorkflowService` → `AccountMigrationWorkflowService`. Refactor to use `CreatePortalUserStep` instead of `SignupUserCreationService` directly.
|
||||
|
||||
**Files:**
|
||||
|
||||
- Create: `apps/bff/src/modules/auth/infra/workflows/account-migration-workflow.service.ts`
|
||||
|
||||
**Step 1: Create renamed + refactored file**
|
||||
|
||||
Copy `whmcs-migration-workflow.service.ts` → `account-migration-workflow.service.ts`:
|
||||
|
||||
- Rename class: `WhmcsMigrationWorkflowService` → `AccountMigrationWorkflowService`
|
||||
- Replace `PortalUserCreationService` (or `SignupUserCreationService` if Task 1 import not yet updated) with `CreatePortalUserStep`
|
||||
- Constructor: remove `userCreation: PortalUserCreationService`, add `portalUserStep: CreatePortalUserStep`
|
||||
- In `executeMigration()`: replace `this.userCreation.createUserWithMapping({...})` with `this.portalUserStep.execute({...})`
|
||||
- Update imports accordingly
|
||||
|
||||
**Step 2: Run type-check**
|
||||
|
||||
Run: `pnpm type-check`
|
||||
Expected: PASS
|
||||
|
||||
**Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add apps/bff/src/modules/auth/infra/workflows/account-migration-workflow.service.ts
|
||||
git commit -m "refactor: rename WhmcsMigration to AccountMigration, use CreatePortalUserStep"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 4: Update coordinator and module
|
||||
|
||||
Wire new services, remove old references.
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `apps/bff/src/modules/auth/infra/workflows/get-started-coordinator.service.ts`
|
||||
- Modify: `apps/bff/src/modules/auth/get-started/get-started.module.ts`
|
||||
|
||||
**Step 1: Update coordinator**
|
||||
|
||||
Imports:
|
||||
|
||||
- Remove: `NewCustomerSignupWorkflowService`, `SfCompletionWorkflowService`, `WhmcsMigrationWorkflowService`
|
||||
- Add: `AccountCreationWorkflowService`, `AccountMigrationWorkflowService`
|
||||
|
||||
Constructor:
|
||||
|
||||
- Remove: `newCustomerSignup`, `sfCompletion`, `whmcsMigration`
|
||||
- Add: `accountCreation: AccountCreationWorkflowService`, `accountMigration: AccountMigrationWorkflowService`
|
||||
|
||||
Methods:
|
||||
|
||||
- `completeAccount()`: `return this.accountCreation.execute(request);`
|
||||
- `signupWithEligibility()`: `return this.accountCreation.execute(request, { withEligibility: true });`
|
||||
- `migrateWhmcsAccount()`: `return this.accountMigration.execute(request);`
|
||||
|
||||
**Step 2: Update module providers**
|
||||
|
||||
Imports:
|
||||
|
||||
- Remove: `NewCustomerSignupWorkflowService`, `SfCompletionWorkflowService`, `WhmcsMigrationWorkflowService`
|
||||
- Add: `AccountCreationWorkflowService`, `AccountMigrationWorkflowService`
|
||||
|
||||
Providers:
|
||||
|
||||
- Remove: `NewCustomerSignupWorkflowService`, `SfCompletionWorkflowService`, `WhmcsMigrationWorkflowService`
|
||||
- Add: `AccountCreationWorkflowService`, `AccountMigrationWorkflowService`
|
||||
|
||||
**Step 3: Run type-check**
|
||||
|
||||
Run: `pnpm type-check`
|
||||
Expected: PASS
|
||||
|
||||
**Step 4: Commit**
|
||||
|
||||
```bash
|
||||
git add apps/bff/src/modules/auth/infra/workflows/get-started-coordinator.service.ts \
|
||||
apps/bff/src/modules/auth/get-started/get-started.module.ts
|
||||
git commit -m "refactor: wire AccountCreation and AccountMigration into coordinator and module"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 5: Delete old files
|
||||
|
||||
**Step 1: Delete old workflow files**
|
||||
|
||||
```bash
|
||||
rm apps/bff/src/modules/auth/infra/workflows/sf-completion-workflow.service.ts
|
||||
rm apps/bff/src/modules/auth/infra/workflows/new-customer-signup-workflow.service.ts
|
||||
rm apps/bff/src/modules/auth/infra/workflows/whmcs-migration-workflow.service.ts
|
||||
```
|
||||
|
||||
**Step 2: Delete entire signup/ directory**
|
||||
|
||||
```bash
|
||||
rm -rf apps/bff/src/modules/auth/infra/workflows/signup/
|
||||
```
|
||||
|
||||
**Step 3: Run type-check and tests**
|
||||
|
||||
Run: `pnpm type-check && pnpm --filter @customer-portal/bff test`
|
||||
Expected: PASS
|
||||
|
||||
**Step 4: Commit**
|
||||
|
||||
```bash
|
||||
git add -A
|
||||
git commit -m "chore: delete merged/deprecated signup workflow files and signup/ directory"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 6: Verify no stale references (Phase 1 checkpoint)
|
||||
|
||||
**Step 1: Search for stale references in source code**
|
||||
|
||||
Search `apps/` and `packages/` for:
|
||||
|
||||
- `SfCompletionWorkflow`, `NewCustomerSignupWorkflow`, `WhmcsMigrationWorkflow`
|
||||
- `SignupAccountResolver`, `SignupValidation`, `SignupWhmcsService`, `SignupUserCreation`
|
||||
- `signup-account-resolver`, `signup-validation`, `signup-whmcs`, `signup-user-creation`
|
||||
|
||||
Expected: No matches in `apps/` or `packages/` (docs/ matches are fine)
|
||||
|
||||
**Step 2: Fix any remaining references**
|
||||
|
||||
**Step 3: Run full type-check and tests**
|
||||
|
||||
Run: `pnpm type-check && pnpm --filter @customer-portal/bff test`
|
||||
Expected: PASS
|
||||
|
||||
**Step 4: Commit if fixes were needed**
|
||||
|
||||
```bash
|
||||
git add -A
|
||||
git commit -m "fix: update remaining references to old workflow service names"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
# Phase 2: Remove legacy migrate endpoint
|
||||
|
||||
## Task 7: Remove WhmcsLinkWorkflowService and /auth/migrate
|
||||
|
||||
Remove the legacy password-based WHMCS migration. All WHMCS migration now goes through get-started OTP-based flow.
|
||||
|
||||
**Files:**
|
||||
|
||||
- Delete: `apps/bff/src/modules/auth/infra/workflows/whmcs-link-workflow.service.ts`
|
||||
- Modify: `apps/bff/src/modules/auth/auth.module.ts` (remove provider + import)
|
||||
- Modify: `apps/bff/src/modules/auth/application/auth-orchestrator.service.ts` (remove `linkWhmcsUser()` + dependency)
|
||||
- Modify: `apps/bff/src/modules/auth/presentation/http/auth.controller.ts` (remove `POST /auth/migrate` endpoint)
|
||||
|
||||
**Step 1: Update AuthOrchestrator**
|
||||
|
||||
- Remove `linkWhmcsUser()` method
|
||||
- Remove `WhmcsLinkWorkflowService` from constructor injection
|
||||
- Remove import
|
||||
|
||||
**Step 2: Update AuthController**
|
||||
|
||||
- Remove the `POST /auth/migrate` endpoint handler
|
||||
- Remove related imports (request DTO, etc.)
|
||||
|
||||
**Step 3: Update AuthModule**
|
||||
|
||||
- Remove `WhmcsLinkWorkflowService` from providers
|
||||
- Remove import
|
||||
|
||||
**Step 4: Delete the workflow file**
|
||||
|
||||
```bash
|
||||
rm apps/bff/src/modules/auth/infra/workflows/whmcs-link-workflow.service.ts
|
||||
```
|
||||
|
||||
**Step 5: Search for stale references**
|
||||
|
||||
Search for `WhmcsLinkWorkflow`, `linkWhmcsUser`, `/auth/migrate` in source code.
|
||||
|
||||
**Step 6: Run type-check and tests**
|
||||
|
||||
Run: `pnpm type-check && pnpm --filter @customer-portal/bff test`
|
||||
Expected: PASS
|
||||
|
||||
**Step 7: Commit**
|
||||
|
||||
```bash
|
||||
git add -A
|
||||
git commit -m "refactor: remove legacy /auth/migrate endpoint and WhmcsLinkWorkflowService"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
# Phase 3: Clean up OTP infrastructure
|
||||
|
||||
## Task 8: Extract shared OTP email-sending pattern
|
||||
|
||||
Both `LoginOtpWorkflowService.sendOtpEmail()` and `VerificationWorkflowService.sendOtpEmail()` implement the same pattern: look up template ID from config, send via template or fallback HTML.
|
||||
|
||||
**Files:**
|
||||
|
||||
- Create: `apps/bff/src/modules/auth/infra/otp/otp-email.service.ts`
|
||||
- Modify: `apps/bff/src/modules/auth/infra/workflows/login-otp-workflow.service.ts`
|
||||
- Modify: `apps/bff/src/modules/auth/infra/workflows/verification-workflow.service.ts`
|
||||
- Modify: `apps/bff/src/modules/auth/otp/otp.module.ts` (add provider)
|
||||
|
||||
**Step 1: Create `OtpEmailService`**
|
||||
|
||||
Extract the shared email-sending pattern:
|
||||
|
||||
```typescript
|
||||
@Injectable()
|
||||
export class OtpEmailService {
|
||||
constructor(
|
||||
private readonly config: ConfigService,
|
||||
private readonly emailService: EmailService
|
||||
) {}
|
||||
|
||||
async sendOtpCode(email: string, code: string, context: "login" | "verification"): Promise<void> {
|
||||
// Look up template ID based on context
|
||||
// Send via template or fallback HTML
|
||||
// Both current implementations do the same thing with different template IDs
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Step 2: Refactor LoginOtpWorkflowService**
|
||||
|
||||
Replace inline `sendOtpEmail()` with `otpEmailService.sendOtpCode(email, code, 'login')`.
|
||||
|
||||
**Step 3: Refactor VerificationWorkflowService**
|
||||
|
||||
Replace inline `sendOtpEmail()` with `otpEmailService.sendOtpCode(email, code, 'verification')`.
|
||||
|
||||
**Step 4: Update OtpModule**
|
||||
|
||||
Add `OtpEmailService` to providers and exports.
|
||||
|
||||
**Step 5: Run type-check and tests**
|
||||
|
||||
Run: `pnpm type-check && pnpm --filter @customer-portal/bff test`
|
||||
Expected: PASS
|
||||
|
||||
**Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add -A
|
||||
git commit -m "refactor: extract shared OTP email service from login and verification workflows"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 9: Improve PORTAL_EXISTS redirect UX
|
||||
|
||||
When get-started verification detects `PORTAL_EXISTS`, the frontend redirects to login without context. Improve this by passing the verified email so it can be prefilled.
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `apps/portal/src/features/get-started/machines/get-started.machine.ts`
|
||||
- Potentially: login page component (to accept prefilled email from query param or state)
|
||||
|
||||
**Step 1: Update get-started machine**
|
||||
|
||||
When `accountStatus === PORTAL_EXISTS`, redirect to login with email as query param:
|
||||
|
||||
```typescript
|
||||
// Instead of: router.push('/login')
|
||||
// Do: router.push(`/login?email=${encodeURIComponent(verifiedEmail)}`)
|
||||
```
|
||||
|
||||
**Step 2: Update login page to accept prefilled email**
|
||||
|
||||
Read `email` from URL query params and prefill the email input field.
|
||||
|
||||
**Step 3: Test manually**
|
||||
|
||||
1. Go to get-started with an existing account email
|
||||
2. Verify via OTP
|
||||
3. Should redirect to login with email prefilled
|
||||
|
||||
**Step 4: Commit**
|
||||
|
||||
```bash
|
||||
git add -A
|
||||
git commit -m "feat: prefill email on login page when redirected from get-started"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Notes
|
||||
|
||||
### Error handling difference between merged paths
|
||||
|
||||
The `completeAccount` endpoint throws exceptions directly. The `signupWithEligibility` endpoint returns `{ success: false, message, errorCategory }`. The merged workflow preserves this:
|
||||
|
||||
```typescript
|
||||
if (options?.withEligibility) {
|
||||
try { return await this.executeCreation(...); }
|
||||
catch (error) {
|
||||
const classified = classifyError(error);
|
||||
return { success: false, errorCategory: classified.errorCategory, message: classified.message };
|
||||
}
|
||||
} else {
|
||||
return await this.executeCreation(...); // exceptions propagate
|
||||
}
|
||||
```
|
||||
|
||||
### createContact() for Birthdate/Sex\_\_c
|
||||
|
||||
New degradable step using existing `SalesforceAccountService.createContact()`. Gender mapping:
|
||||
|
||||
- `male` → `男性`
|
||||
- `female` → `女性`
|
||||
- `other` → `Other`
|
||||
|
||||
### Sessions stay separate
|
||||
|
||||
Login sessions (`LoginSessionService`) and get-started sessions (`GetStartedSessionService`) have fundamentally different data models, TTLs, and lifecycle requirements. Only the OTP email-sending pattern is worth extracting.
|
||||
Loading…
x
Reference in New Issue
Block a user