diff --git a/.claude/commands/review-branch.md b/.claude/commands/review-branch.md index 39ce009a3..6fb4e31c8 100644 --- a/.claude/commands/review-branch.md +++ b/.claude/commands/review-branch.md @@ -8,4 +8,6 @@ Do a thorough code review of this branch. If an argument is passed and it is a g If there is no argument, you should review the current changes on this branch (you can diff against the dev branch). Always do this in planning mode and present the review at the end. +Additionally, once you have reviewed the branch: Review all tests updated in this branch. Making sure they test what they say they test and provide good coverage over the functionality. + Arguments: $ARGUMENTS diff --git a/apps/app/src/components/decisions/CurrentPhaseSurface.tsx b/apps/app/src/components/decisions/CurrentPhaseSurface.tsx index e6b8d06e6..36e724075 100644 --- a/apps/app/src/components/decisions/CurrentPhaseSurface.tsx +++ b/apps/app/src/components/decisions/CurrentPhaseSurface.tsx @@ -5,15 +5,12 @@ import { formatCurrency, formatDateRange, } from '@/utils/formatting'; -import type { processPhaseSchema } from '@op/api/encoders'; +import { type ProcessPhase } from '@op/api/encoders'; import { Surface } from '@op/ui/Surface'; import { useLocale } from 'next-intl'; -import type { z } from 'zod'; import { useTranslations } from '@/lib/i18n'; -type ProcessPhase = z.infer; - interface CurrentPhaseSurfaceProps { currentPhase?: ProcessPhase; budget?: number; diff --git a/apps/app/src/components/decisions/DecisionHeader.tsx b/apps/app/src/components/decisions/DecisionHeader.tsx index f0341f49a..546355850 100644 --- a/apps/app/src/components/decisions/DecisionHeader.tsx +++ b/apps/app/src/components/decisions/DecisionHeader.tsx @@ -1,3 +1,4 @@ +import { type ProcessPhase } from '@op/api/encoders'; import { createClient } from '@op/api/serverClient'; import type { DecisionInstanceData } from '@op/common'; import { cn } from '@op/ui/utils'; @@ -6,7 +7,6 @@ import { ReactNode } from 'react'; import { DecisionInstanceHeader } from '@/components/decisions/DecisionInstanceHeader'; import { DecisionProcessStepper } from '@/components/decisions/DecisionProcessStepper'; -import { ProcessPhase } from '@/components/decisions/types'; interface DecisionHeaderProps { instanceId: string; diff --git a/apps/app/src/components/decisions/DecisionProcessStepper.tsx b/apps/app/src/components/decisions/DecisionProcessStepper.tsx index 99a75c87c..0f45f5234 100644 --- a/apps/app/src/components/decisions/DecisionProcessStepper.tsx +++ b/apps/app/src/components/decisions/DecisionProcessStepper.tsx @@ -1,19 +1,8 @@ 'use client'; +import { type ProcessPhase } from '@op/api/encoders'; import { type Phase, PhaseStepper } from '@op/ui/PhaseStepper'; -interface ProcessPhase { - id: string; - name: string; - description?: string; - phase?: { - startDate?: string; - endDate?: string; - sortOrder?: number; - }; - type?: 'initial' | 'intermediate' | 'final'; -} - interface DecisionProcessStepperProps { phases: ProcessPhase[]; currentStateId: string; diff --git a/apps/app/src/components/decisions/DecisionStats.tsx b/apps/app/src/components/decisions/DecisionStats.tsx index 3c1c54659..13b4bb0a7 100644 --- a/apps/app/src/components/decisions/DecisionStats.tsx +++ b/apps/app/src/components/decisions/DecisionStats.tsx @@ -1,10 +1,7 @@ 'use client'; import { formatCurrency, formatDateRange } from '@/utils/formatting'; -import type { processPhaseSchema } from '@op/api/encoders'; -import type { z } from 'zod'; - -type ProcessPhase = z.infer; +import { type ProcessPhase } from '@op/api/encoders'; interface DecisionStatsProps { currentPhase?: ProcessPhase; @@ -29,11 +26,11 @@ export function DecisionStats({

{currentPhase?.name || 'Proposal Submissions'}

- {currentPhase?.phase && ( + {(currentPhase?.phase?.startDate || currentPhase?.phase?.endDate) && (

{formatDateRange( - currentPhase.phase.startDate, - currentPhase.phase.endDate, + currentPhase.phase?.startDate, + currentPhase.phase?.endDate, ) || 'Timeline not set'}

)} diff --git a/apps/app/src/components/decisions/types.ts b/apps/app/src/components/decisions/types.ts deleted file mode 100644 index b27319a55..000000000 --- a/apps/app/src/components/decisions/types.ts +++ /dev/null @@ -1,14 +0,0 @@ -export interface ProcessPhase { - id: string; - name: string; - description?: string; - phase?: { - startDate?: string; - endDate?: string; - sortOrder?: number; - }; - type?: 'initial' | 'intermediate' | 'final'; - config?: { - allowProposals?: boolean; - }; -} diff --git a/packages/common/src/services/decision/createInstance.ts b/packages/common/src/services/decision/createInstance.ts index 5bb9fe71a..abe62b20a 100644 --- a/packages/common/src/services/decision/createInstance.ts +++ b/packages/common/src/services/decision/createInstance.ts @@ -11,7 +11,6 @@ import { User } from '@op/supabase/lib'; import { CommonError, NotFoundError, UnauthorizedError } from '../../utils'; import { assertUserByAuthId } from '../assert'; import { generateUniqueProfileSlug } from '../profile/utils'; -import { createTransitionsForProcess } from './createTransitionsForProcess'; import type { InstanceData, ProcessSchema } from './types'; export interface CreateInstanceInput { @@ -93,9 +92,8 @@ export const createInstance = async ({ return newInstance; }); - // Create transitions for the process phases - // This is critical - if transitions can't be created, the process won't auto-advance - await createTransitionsForProcess({ processInstance: instance }); + // Note: Transitions are created when the instance is published (status changes from DRAFT to PUBLISHED) + // Draft instances don't need transitions since they won't be processed return instance; } catch (error) { diff --git a/packages/common/src/services/decision/createInstanceFromTemplate.ts b/packages/common/src/services/decision/createInstanceFromTemplate.ts index dc48203aa..4966de14a 100644 --- a/packages/common/src/services/decision/createInstanceFromTemplate.ts +++ b/packages/common/src/services/decision/createInstanceFromTemplate.ts @@ -12,7 +12,6 @@ import type { User } from '@op/supabase/lib'; import { CommonError, UnauthorizedError } from '../../utils'; import { assertUserByAuthId } from '../assert'; import { generateUniqueProfileSlug } from '../profile/utils'; -import { createTransitionsForProcess } from './createTransitionsForProcess'; import { getTemplate } from './getTemplate'; import { type PhaseOverride, @@ -123,22 +122,8 @@ export const createInstanceFromTemplateCore = async ({ return newInstance; }); - // Create scheduled transitions for phases that have date-based advancement AND actual dates set - const hasScheduledDatePhases = instanceData.phases.some( - (phase) => phase.rules?.advancement?.method === 'date' && phase.startDate, - ); - - if (hasScheduledDatePhases) { - try { - await createTransitionsForProcess({ processInstance: instance }); - } catch (error) { - // Log but don't fail instance creation if transitions can't be created - console.error( - 'Failed to create transitions for process instance:', - error, - ); - } - } + // Note: Transitions are NOT created here because the instance is created as DRAFT. + // Transitions are created when the instance is published via updateDecisionInstance. // Fetch the profile with processInstance joined for the response // profileId is guaranteed to be set since we just created it above diff --git a/packages/common/src/services/decision/createTransitionsForProcess.ts b/packages/common/src/services/decision/createTransitionsForProcess.ts index 1ccbf01a2..9d171b7bc 100644 --- a/packages/common/src/services/decision/createTransitionsForProcess.ts +++ b/packages/common/src/services/decision/createTransitionsForProcess.ts @@ -1,32 +1,38 @@ -import { db } from '@op/db/client'; +import { type TransactionType, db } from '@op/db/client'; import { decisionProcessTransitions } from '@op/db/schema'; import type { ProcessInstance } from '@op/db/schema'; import { CommonError } from '../../utils'; -import type { InstanceData, PhaseConfiguration } from './types'; +import type { DecisionInstanceData } from './schemas/instanceData'; +import type { ScheduledTransition } from './types'; -export interface CreateTransitionsInput { +/** + * Creates scheduled transition records for phases with date-based advancement. + * Each transition fires when the current phase's end date arrives. + * + * All phase rules are expected to be present in instanceData.phases. Rules are + * populated when creating an instance from a template and preserved during updates. + */ +export async function createTransitionsForProcess({ + processInstance, + tx, +}: { processInstance: ProcessInstance; -} - -export interface CreateTransitionsResult { + tx?: TransactionType; +}): Promise<{ transitions: Array<{ id: string; fromStateId: string | null; toStateId: string; scheduledDate: Date; }>; -} +}> { + const dbClient = tx ?? db; -/** - * Creates transition records for all phases in a process instance. - * Each transition represents the end of one phase and the start of the next. - */ -export async function createTransitionsForProcess({ - processInstance, -}: CreateTransitionsInput): Promise { try { - const instanceData = processInstance.instanceData as InstanceData; + // Type assertion: instanceData is `unknown` in DB to support legacy formats for viewing, + // but this function is only called for new DecisionInstanceData processes + const instanceData = processInstance.instanceData as DecisionInstanceData; const phases = instanceData.phases; if (!phases || phases.length === 0) { @@ -35,39 +41,55 @@ export async function createTransitionsForProcess({ ); } - const transitionsToCreate = phases.map( - (phase: PhaseConfiguration, index: number) => { - const fromStateId = index > 0 ? phases[index - 1]?.phaseId : null; - const toStateId = phase.phaseId; - // For phases like 'results' that only have a start date (no end), use the start date - const scheduledDate = phase.startDate; + // Create transitions for phases that use date-based advancement + // A transition is created FROM a phase (when it ends) TO the next phase + const transitionsToCreate: ScheduledTransition[] = []; - if (!scheduledDate) { - throw new CommonError( - `Phase ${index + 1} (${toStateId}) must have either a scheduled end date or start date`, - ); - } + phases.forEach((currentPhase, index) => { + const nextPhase = phases[index + 1]; + // Skip last phase (no next phase to transition to) + if (!nextPhase) { + return; + } - return { - processInstanceId: processInstance.id, - fromStateId, - toStateId, - scheduledDate: new Date(scheduledDate).toISOString(), - }; - }, - ); + // Only create transition if current phase uses date-based advancement + if (currentPhase.rules?.advancement?.method !== 'date') { + return; + } + + // Schedule transition when the current phase ends + const scheduledDate = currentPhase.endDate; + + if (!scheduledDate) { + throw new CommonError( + `Phase "${currentPhase.phaseId}" must have an end date for date-based advancement (instance: ${processInstance.id})`, + ); + } + + // DB columns are named fromStateId/toStateId but store phase IDs + transitionsToCreate.push({ + processInstanceId: processInstance.id, + fromStateId: currentPhase.phaseId, + toStateId: nextPhase.phaseId, + scheduledDate: new Date(scheduledDate).toISOString(), + }); + }); + + if (transitionsToCreate.length === 0) { + return { transitions: [] }; + } - const createdTransitions = await db + const createdTransitions = await dbClient .insert(decisionProcessTransitions) .values(transitionsToCreate) .returning(); return { - transitions: createdTransitions.map((t) => ({ - id: t.id, - fromStateId: t.fromStateId, - toStateId: t.toStateId, - scheduledDate: new Date(t.scheduledDate), + transitions: createdTransitions.map((transition) => ({ + id: transition.id, + fromStateId: transition.fromStateId, + toStateId: transition.toStateId, + scheduledDate: new Date(transition.scheduledDate), })), }; } catch (error) { diff --git a/packages/common/src/services/decision/index.ts b/packages/common/src/services/decision/index.ts index fec96b927..dc64c65d0 100644 --- a/packages/common/src/services/decision/index.ts +++ b/packages/common/src/services/decision/index.ts @@ -68,3 +68,9 @@ export type { DecisionInstanceData, PhaseInstanceData, } from './schemas/instanceData'; +export type { + DecisionSchemaDefinition, + PhaseDefinition, + PhaseRules, + ProcessConfig, +} from './schemas/types'; diff --git a/packages/common/src/services/decision/transitionEngine.ts b/packages/common/src/services/decision/transitionEngine.ts index 105c1e0b6..04c91dee6 100644 --- a/packages/common/src/services/decision/transitionEngine.ts +++ b/packages/common/src/services/decision/transitionEngine.ts @@ -73,12 +73,6 @@ export class TransitionEngine { const process = instance.process as any; const processSchema = process.processSchema as ProcessSchema; const instanceData = instance.instanceData as InstanceData; - console.log( - 'TRANSITION', - instanceData.currentPhaseId, - instance.currentStateId, - instanceData, - ); const currentStateId = instanceData.currentPhaseId || instance.currentStateId || ''; diff --git a/packages/common/src/services/decision/transitionMonitor.ts b/packages/common/src/services/decision/transitionMonitor.ts index fc436f6af..c9cc70f6b 100644 --- a/packages/common/src/services/decision/transitionMonitor.ts +++ b/packages/common/src/services/decision/transitionMonitor.ts @@ -1,14 +1,31 @@ -import { db, eq, lte, sql } from '@op/db/client'; +import { and, db, eq, isNull, lte, sql } from '@op/db/client'; import { + type DecisionProcess, + type DecisionProcessTransition, + type ProcessInstance, + ProcessStatus, decisionProcessTransitions, - decisionProcesses, processInstances, } from '@op/db/schema'; import pMap from 'p-map'; import { CommonError } from '../../utils'; -// import { processResults } from './processResults'; -import type { ProcessSchema, StateDefinition } from './types'; +import { processResults } from './processResults'; +import type { DecisionInstanceData } from './schemas/instanceData'; + +/** Transition with nested process instance and process relations */ +type TransitionWithRelations = DecisionProcessTransition & { + processInstance: ProcessInstance & { + process: DecisionProcess; + }; +}; + +/** Result of processing a single transition */ +interface ProcessedTransition { + toStateId: string; + isTransitioningToFinalState: boolean; + processInstanceId: string; +} export interface ProcessDecisionsTransitionsResult { processed: number; @@ -33,14 +50,30 @@ export async function processDecisionsTransitions(): Promise + // Query due transitions, filtering to only include published instances + // Draft, completed, and cancelled instances should not have their transitions processed + const dueTransitions = await db + .select({ + id: decisionProcessTransitions.id, + processInstanceId: decisionProcessTransitions.processInstanceId, + fromStateId: decisionProcessTransitions.fromStateId, + toStateId: decisionProcessTransitions.toStateId, + scheduledDate: decisionProcessTransitions.scheduledDate, + completedAt: decisionProcessTransitions.completedAt, + }) + .from(decisionProcessTransitions) + .innerJoin( + processInstances, + eq(decisionProcessTransitions.processInstanceId, processInstances.id), + ) + .where( and( - isNull(transitions.completedAt), - lte(transitions.scheduledDate, now), + isNull(decisionProcessTransitions.completedAt), + lte(decisionProcessTransitions.scheduledDate, now), + eq(processInstances.status, ProcessStatus.PUBLISHED), ), - orderBy: (transitions, { asc }) => [asc(transitions.scheduledDate)], - }); + ) + .orderBy(decisionProcessTransitions.scheduledDate); // Group transitions by processInstanceId to avoid race conditions // within the same process instance @@ -57,16 +90,18 @@ export async function processDecisionsTransitions(): Promise { + async ([processInstanceId, transitions]) => { + let lastSuccessfulTransition: ProcessedTransition | null = null; + // Process this process's transitions sequentially for (const transition of transitions) { try { - await processTransition(transition.id); - + // Process transition (marks completedAt, returns state info) + lastSuccessfulTransition = await processTransition(transition.id); result.processed++; } catch (error) { result.failed++; @@ -81,10 +116,52 @@ export async function processDecisionsTransitions(): Promise { - const transition = await db._query.decisionProcessTransitions.findFirst({ - where: eq(decisionProcessTransitions.id, transitionId), - }); +async function processTransition( + transitionId: string, +): Promise { + // Fetch transition with related process instance and process in a single query + const transitionResult = await db._query.decisionProcessTransitions.findFirst( + { + where: eq(decisionProcessTransitions.id, transitionId), + with: { + processInstance: { + with: { + process: true, + }, + }, + }, + }, + ); - if (!transition) { + if (!transitionResult) { throw new CommonError( `Transition not found: ${transitionId}. It may have been deleted or the ID is invalid.`, ); } - if (transition.completedAt) { - return; - } - - // Get the process instance to check if we're transitioning to a final state - const processInstance = await db._query.processInstances.findFirst({ - where: eq(processInstances.id, transition.processInstanceId), - }); + // Type assertion for the nested relations (Drizzle's type inference doesn't handle nested `with` well) + const transition = transitionResult as TransitionWithRelations; + const processInstance = transition.processInstance; if (!processInstance) { throw new CommonError( `Process instance not found: ${transition.processInstanceId}`, ); } - // Get the process schema to check the state type - const process = await db._query.decisionProcesses.findFirst({ - where: eq(decisionProcesses.id, processInstance.processId), - }); - + const process = processInstance.process; if (!process) { throw new CommonError(`Process not found: ${processInstance.processId}`); } - const processSchema = process.processSchema as ProcessSchema; - const toState = processSchema.states.find( - (state: StateDefinition) => state.id === transition.toStateId, - ); + // Determine if transitioning to final state using instanceData.phases + // In the new schema format, the last phase is always the final state + const instanceData = processInstance.instanceData as DecisionInstanceData; + const phases = instanceData.phases; + const lastPhaseId = phases[phases.length - 1]?.phaseId; + const isTransitioningToFinalState = transition.toStateId === lastPhaseId; - const isTransitioningToFinalState = toState?.type === 'final'; - - // Update both the process instance and transition in a single transaction - // to ensure atomicity and prevent partial state updates - // Note: We rely on foreign key constraints to ensure the process instance exists - await db.transaction(async (tx) => { - // Update the process instance to the new state - // Use jsonb_set to update only the currentStateId field without reading the entire instanceData - await tx - .update(processInstances) - .set({ - currentStateId: transition.toStateId, - instanceData: sql`jsonb_set( - ${processInstances.instanceData}, - '{currentStateId}', - to_jsonb(${transition.toStateId}::text) - )`, - }) - .where(eq(processInstances.id, transition.processInstanceId)); + // Only mark the transition as completed (state update happens after all transitions) + await db + .update(decisionProcessTransitions) + .set({ + completedAt: new Date().toISOString(), + }) + .where(eq(decisionProcessTransitions.id, transitionId)); - // Mark the transition as completed - await tx - .update(decisionProcessTransitions) - .set({ - completedAt: new Date().toISOString(), - }) - .where(eq(decisionProcessTransitions.id, transition.id)); - }); - - // If transitioning to a final state (results phase), process the results - // This is done AFTER the transition is marked complete to ensure the state change - // is committed even if results processing fails - if (isTransitioningToFinalState) { - try { - console.log( - `Processing results for process instance ${transition.processInstanceId}`, - ); - console.log('RESULT PROCESSING DISABLED'); - // Disabling for now as none are defined in production yet. We need to migrate current processes first. - /* - const result = await processResults({ - processInstanceId: transition.processInstanceId, - }); - - if (!result.success) { - console.error( - `Results processing failed for process instance ${transition.processInstanceId}:`, - result.error, - ); - } else { - console.log( - `Results processed successfully for process instance ${transition.processInstanceId}. Selected ${result.selectedProposalIds.length} proposals.`, - ); - } - */ - } catch (error) { - // Log the error but don't fail the transition - // The transition to the results phase has already been completed - console.error( - `Error processing results for process instance ${transition.processInstanceId}:`, - error, - ); - } - } + // Return info needed for state update + return { + toStateId: transition.toStateId, + isTransitioningToFinalState, + processInstanceId: transition.processInstanceId, + }; +} + +/** + * Updates the process instance state to the specified state. + * Called after all transitions for an instance have been processed. + */ +async function updateInstanceState( + processInstanceId: string, + toStateId: string, +): Promise { + await db + .update(processInstances) + .set({ + currentStateId: toStateId, + instanceData: sql`jsonb_set( + ${processInstances.instanceData}, + '{currentPhaseId}', + to_jsonb(${toStateId}::text) + )`, + }) + .where(eq(processInstances.id, processInstanceId)); } diff --git a/packages/common/src/services/decision/types.ts b/packages/common/src/services/decision/types.ts index 9f9a53569..6fac6c690 100644 --- a/packages/common/src/services/decision/types.ts +++ b/packages/common/src/services/decision/types.ts @@ -1,3 +1,4 @@ +import type { DecisionProcessTransition } from '@op/db/schema'; import type { JSONSchema7 } from 'json-schema'; import type { SelectionPipeline } from './selectionPipeline/types'; @@ -161,3 +162,9 @@ export interface DecisionData { // Decision content should match the decisionDefinition schema [key: string]: unknown; } + +/** A scheduled phase transition (fromStateId/toStateId store phase IDs) */ +export type ScheduledTransition = Pick< + DecisionProcessTransition, + 'processInstanceId' | 'fromStateId' | 'toStateId' | 'scheduledDate' +>; diff --git a/packages/common/src/services/decision/updateDecisionInstance.ts b/packages/common/src/services/decision/updateDecisionInstance.ts index 96f98272c..f6ad79dcb 100644 --- a/packages/common/src/services/decision/updateDecisionInstance.ts +++ b/packages/common/src/services/decision/updateDecisionInstance.ts @@ -10,6 +10,7 @@ import { assertAccess, permission } from 'access-zones'; import { CommonError, NotFoundError } from '../../utils'; import { getProfileAccessUser } from '../access'; +import { createTransitionsForProcess } from './createTransitionsForProcess'; import type { DecisionInstanceData, PhaseOverride, @@ -179,14 +180,23 @@ export const updateDecisionInstance = async ({ // Determine the final status (updated or existing) const finalStatus = status ?? existingInstance.status; + const isBeingPublished = + status === ProcessStatus.PUBLISHED && + existingInstance.status === ProcessStatus.DRAFT; // If status is DRAFT, remove all transitions if (finalStatus === ProcessStatus.DRAFT) { await tx .delete(decisionProcessTransitions) .where(eq(decisionProcessTransitions.processInstanceId, instanceId)); + } else if (isBeingPublished) { + // When publishing a draft, create transitions for all date-based phases + await createTransitionsForProcess({ + processInstance: updatedInstance, + tx, + }); } else if (phases && phases.length > 0) { - // If phases were updated and not DRAFT, update the corresponding transitions + // If phases were updated and already published, update the corresponding transitions await updateTransitionsForProcess({ processInstance: updatedInstance, tx, diff --git a/packages/common/src/services/decision/updateTransitionsForProcess.ts b/packages/common/src/services/decision/updateTransitionsForProcess.ts index 91c7859bc..833edfc09 100644 --- a/packages/common/src/services/decision/updateTransitionsForProcess.ts +++ b/packages/common/src/services/decision/updateTransitionsForProcess.ts @@ -4,7 +4,8 @@ import type { ProcessInstance } from '@op/db/schema'; import pMap from 'p-map'; import { CommonError } from '../../utils'; -import type { InstanceData, PhaseConfiguration } from './types'; +import type { DecisionInstanceData } from './schemas/instanceData'; +import type { ScheduledTransition } from './types'; export interface UpdateTransitionsInput { processInstance: ProcessInstance; @@ -19,10 +20,11 @@ export interface UpdateTransitionsResult { /** * Updates transition records for a process instance when phase dates change. + * Only handles phases with date-based advancement (rules.advancement.method === 'date'). * This function: * - Updates existing transitions with new scheduled dates - * - Creates new transitions for newly added phases - * - Deletes transitions for removed phases + * - Creates new transitions for newly added date-based phases + * - Deletes transitions for phases that no longer use date-based advancement * - Prevents updates to completed transitions (results phase is locked) */ export async function updateTransitionsForProcess({ @@ -32,7 +34,9 @@ export async function updateTransitionsForProcess({ const dbClient = tx ?? db; try { - const instanceData = processInstance.instanceData as InstanceData; + // Type assertion: instanceData is `unknown` in DB to support legacy formats for viewing, + // but this function is only called for new DecisionInstanceData processes + const instanceData = processInstance.instanceData as DecisionInstanceData; const phases = instanceData.phases; if (!phases || phases.length === 0) { @@ -57,94 +61,122 @@ export async function updateTransitionsForProcess({ deleted: 0, }; - // Build a map of expected transitions from the current phases - const expectedTransitions = phases.map( - (phase: PhaseConfiguration, index: number) => { - const fromStateId = index > 0 ? phases[index - 1]?.phaseId : null; - const toStateId = phase.phaseId; - // For phases like 'results' that only have a start date (no end), use the start date - const scheduledDate = phase.startDate; - - if (!scheduledDate) { - throw new CommonError( - `Phase ${index + 1} (${toStateId}) must have either a scheduled end date or start date`, - ); - } - - return { - fromStateId, - toStateId, - scheduledDate: new Date(scheduledDate).toISOString(), - }; - }, - ); + // Build expected transitions for phases with date-based advancement + // A transition is created FROM a phase (when it ends) TO the next phase + const expectedTransitions: ScheduledTransition[] = []; - // Process each expected transition in parallel - const updateResults = await pMap( - expectedTransitions, - async (expected) => { - // Find matching existing transition by toStateId - const existing = existingTransitions.find( - (t) => t.toStateId === expected.toStateId, - ); + phases.forEach((currentPhase, index) => { + const nextPhase = phases[index + 1]; + // Skip last phase (no next phase to transition to) + if (!nextPhase) { + return; + } - if (existing) { - // If transition is already completed, don't update it (results phase is locked) - if (existing.completedAt) { - return { action: 'skipped' as const }; - } + // Only create transition if current phase uses date-based advancement + if (currentPhase.rules?.advancement?.method !== 'date') { + return; + } - // Update the scheduled date if it changed - if (existing.scheduledDate !== expected.scheduledDate) { - await dbClient - .update(decisionProcessTransitions) - .set({ - scheduledDate: expected.scheduledDate, - }) - .where(eq(decisionProcessTransitions.id, existing.id)); + // Schedule transition when the current phase ends + const scheduledDate = currentPhase.endDate; - return { action: 'updated' as const }; - } - - return { action: 'unchanged' as const }; - } else { - // Create new transition for this phase - await dbClient.insert(decisionProcessTransitions).values({ - processInstanceId: processInstance.id, - fromStateId: expected.fromStateId, - toStateId: expected.toStateId, - scheduledDate: expected.scheduledDate, - }); - - return { action: 'created' as const }; - } - }, - { concurrency: 5 }, - ); - - // Aggregate results - result.updated = updateResults.filter((r) => r.action === 'updated').length; - result.created = updateResults.filter((r) => r.action === 'created').length; - - // Delete transitions that are no longer in the phases list - // But only delete uncompleted transitions - const expectedStateIds = new Set( - expectedTransitions.map((t) => t.toStateId), + if (!scheduledDate) { + throw new CommonError( + `Phase "${currentPhase.phaseId}" must have an end date for date-based advancement (instance: ${processInstance.id})`, + ); + } + + // DB columns are named fromStateId/toStateId but store phase IDs + expectedTransitions.push({ + processInstanceId: processInstance.id, + fromStateId: currentPhase.phaseId, + toStateId: nextPhase.phaseId, + scheduledDate: new Date(scheduledDate).toISOString(), + }); + }); + + // Calculate transitions to delete upfront (those not in expected set and not completed) + // Use composite key (fromStateId:toStateId) to match both fields + const expectedTransitionKeys = new Set( + expectedTransitions.map( + (transition) => `${transition.fromStateId}:${transition.toStateId}`, + ), ); const transitionsToDelete = existingTransitions.filter( - (t) => !expectedStateIds.has(t.toStateId) && !t.completedAt, + (transition) => + !expectedTransitionKeys.has( + `${transition.fromStateId}:${transition.toStateId}`, + ) && !transition.completedAt, ); - await pMap( - transitionsToDelete, - async (transition) => { - await dbClient - .delete(decisionProcessTransitions) - .where(eq(decisionProcessTransitions.id, transition.id)); - }, - { concurrency: 5 }, - ); + // Run update/create and delete operations in parallel since they operate on mutually exclusive sets + const [updateResults] = await Promise.all([ + // Update existing transitions or create new ones + pMap( + expectedTransitions, + async (expected) => { + // Find matching existing transition by both fromStateId and toStateId + const existing = existingTransitions.find( + (transition) => + transition.fromStateId === expected.fromStateId && + transition.toStateId === expected.toStateId, + ); + + if (existing) { + // If transition is already completed, don't update it (results phase is locked) + if (existing.completedAt) { + return { action: 'skipped' as const }; + } + + // Update the scheduled date if it changed (compare as timestamps to handle format differences) + if ( + new Date(existing.scheduledDate).getTime() !== + new Date(expected.scheduledDate).getTime() + ) { + await dbClient + .update(decisionProcessTransitions) + .set({ + scheduledDate: expected.scheduledDate, + }) + .where(eq(decisionProcessTransitions.id, existing.id)); + + return { action: 'updated' as const }; + } + + return { action: 'unchanged' as const }; + } else { + // Create new transition for this phase + await dbClient.insert(decisionProcessTransitions).values({ + processInstanceId: expected.processInstanceId, + fromStateId: expected.fromStateId, + toStateId: expected.toStateId, + scheduledDate: expected.scheduledDate, + }); + + return { action: 'created' as const }; + } + }, + { concurrency: 5 }, + ), + // Delete transitions that are no longer in the expected set + pMap( + transitionsToDelete, + async (transition) => { + await dbClient + .delete(decisionProcessTransitions) + .where(eq(decisionProcessTransitions.id, transition.id)); + }, + { concurrency: 5 }, + ), + ]); + // Aggregate results + result.updated = updateResults.filter( + (update) => update.action === 'updated', + ).length; + result.created = updateResults.filter( + (update) => update.action === 'created', + ).length; result.deleted = transitionsToDelete.length; return result; diff --git a/packages/common/src/test/setup.ts b/packages/common/src/test/setup.ts index 28be37978..72f412e8e 100644 --- a/packages/common/src/test/setup.ts +++ b/packages/common/src/test/setup.ts @@ -20,6 +20,10 @@ export const mockDb = { decisions: { findFirst: vi.fn(), }, + decisionProcessTransitions: { + findFirst: vi.fn(), + findMany: vi.fn(), + }, }, insert: vi.fn().mockReturnValue({ values: vi.fn().mockReturnValue({ diff --git a/services/api/src/encoders/decision.ts b/services/api/src/encoders/decision.ts index 23f120ab4..e92b5070a 100644 --- a/services/api/src/encoders/decision.ts +++ b/services/api/src/encoders/decision.ts @@ -19,11 +19,37 @@ import { baseProfileEncoder } from './profiles'; // JSON Schema types const jsonSchemaEncoder = z.record(z.string(), z.unknown()); +// ============================================================================ +// ProcessPhase encoder (for frontend UI components) +// ============================================================================ + +/** Process phase encoder for UI display (stepper, stats, etc.) */ +export const processPhaseSchema = z.object({ + id: z.string(), + name: z.string(), + description: z.string().optional(), + phase: z + .object({ + startDate: z.string().optional(), + endDate: z.string().optional(), + sortOrder: z.number().optional(), + }) + .optional(), + type: z.enum(['initial', 'intermediate', 'final']).optional(), + config: z + .object({ + allowProposals: z.boolean().optional(), + }) + .optional(), +}); + +export type ProcessPhase = z.infer; + // ============================================================================ // DecisionSchemaDefinition format encoders // ============================================================================ -/** Phase behavior rules */ +/** Phase behavior rules */ const phaseRulesEncoder = z.object({ proposals: z .object({ @@ -127,7 +153,7 @@ export const decisionProcessWithSchemaEncoder = createSelectSchema( createdBy: baseProfileEncoder.optional(), }); -/** List encoder for decision processes with new schema format */ +/** List encoder for decision processes */ export const decisionProcessWithSchemaListEncoder = z.object({ processes: z.array(decisionProcessWithSchemaEncoder), total: z.number(), @@ -225,90 +251,40 @@ export const decisionProfileWithSchemaFilterSchema = z.object({ stewardProfileId: z.uuid().optional(), }); -// ============================================================================ -// Legacy format encoders (for backwards compatibility) -// ============================================================================ - -// Shared process phase schema -export const processPhaseSchema = z.object({ - id: z.string(), - name: z.string(), - description: z.string().optional(), - phase: z - .object({ - startDate: z.string().optional(), - endDate: z.string().optional(), - sortOrder: z.number().optional(), - }) - .optional(), - type: z.enum(['initial', 'intermediate', 'final']).optional(), -}); - -// Process Schema Encoder -const processSchemaEncoder = z.object({ - name: z.string(), - description: z.string().optional(), - budget: z.number().optional(), - fields: jsonSchemaEncoder.optional(), - states: z.array( - processPhaseSchema.extend({ - fields: jsonSchemaEncoder.optional(), - config: z - .object({ - allowProposals: z.boolean().optional(), - allowDecisions: z.boolean().optional(), - visibleComponents: z.array(z.string()).optional(), - }) - .optional(), - }), - ), - transitions: z.array( - z.object({ - id: z.string(), - name: z.string(), - from: z.union([z.string(), z.array(z.string())]), - to: z.string(), - rules: z - .object({ - type: z.enum(['manual', 'automatic']), - conditions: z - .array( - z.object({ - type: z.enum([ - 'time', - 'proposalCount', - 'participationCount', - 'approvalRate', - 'customField', - ]), - operator: z.enum([ - 'equals', - 'greaterThan', - 'lessThan', - 'between', - ]), - value: z.unknown().optional(), - field: z.string().optional(), - }), - ) - .optional(), - requireAll: z.boolean().optional(), - }) - .optional(), - actions: z - .array( - z.object({ - type: z.enum(['notify', 'updateField', 'createRecord']), - config: z.record(z.string(), z.unknown()), - }), - ) - .optional(), - }), - ), - initialState: z.string(), - decisionDefinition: jsonSchemaEncoder, - proposalTemplate: jsonSchemaEncoder, -}); +// ============================================================================= +// Process Schema Encoder (new format with passthrough for flexibility) +// ============================================================================= +const processSchemaEncoder = z + .object({ + name: z.string(), + description: z.string().optional(), + id: z.string().optional(), + version: z.string().optional(), + config: z + .object({ + hideBudget: z.boolean().optional(), + }) + .passthrough() + .optional(), + phases: z + .array( + z + .object({ + id: z.string(), + name: z.string(), + description: z.string().optional(), + rules: phaseRulesEncoder.optional(), + selectionPipeline: selectionPipelineEncoder.optional(), + settings: jsonSchemaEncoder.optional(), + startDate: z.string().optional(), + endDate: z.string().optional(), + }) + .passthrough(), + ) + .optional(), + proposalTemplate: jsonSchemaEncoder.optional(), + }) + .passthrough(); // Instance Data Encoder that supports both new and legacy field names const instanceDataEncoder = z.preprocess( diff --git a/services/api/src/routers/decision/instances/createInstanceFromTemplate.test.ts b/services/api/src/routers/decision/instances/createInstanceFromTemplate.test.ts index 2c0602eae..d852f197e 100644 --- a/services/api/src/routers/decision/instances/createInstanceFromTemplate.test.ts +++ b/services/api/src/routers/decision/instances/createInstanceFromTemplate.test.ts @@ -102,7 +102,7 @@ describe.concurrent('createInstanceFromTemplate', () => { expect(instanceData.currentPhaseId).toBe('submission'); }); - it('should create transitions for phases with date-based advancement', async ({ + it('should NOT create transitions for DRAFT instances (transitions are created on publish)', async ({ task, onTestFinished, }) => { @@ -152,37 +152,18 @@ describe.concurrent('createInstanceFromTemplate', () => { testData.trackProfileForCleanup(result.id); expect(result.processInstance.id).toBeDefined(); + expect(result.processInstance.status).toBe('draft'); - // Verify transitions were created + // Verify NO transitions were created for DRAFT instance + // Transitions are only created when the instance is published const transitions = await db._query.decisionProcessTransitions.findMany({ where: eq( decisionProcessTransitions.processInstanceId, result.processInstance.id, ), - orderBy: (transitions, { asc }) => [asc(transitions.scheduledDate)], }); - // The simple template has 4 phases with date-based advancement - // Each phase that has a date-based advancement gets a transition created - expect(transitions.length).toBeGreaterThanOrEqual(3); - - // Verify transition details - check that key transitions exist - const submissionToReview = transitions.find( - (t) => t.fromStateId === 'submission' && t.toStateId === 'review', - ); - const reviewToVoting = transitions.find( - (t) => t.fromStateId === 'review' && t.toStateId === 'voting', - ); - const votingToResults = transitions.find( - (t) => t.fromStateId === 'voting' && t.toStateId === 'results', - ); - - expect(submissionToReview).toBeDefined(); - expect(submissionToReview!.completedAt).toBeNull(); - - expect(reviewToVoting).toBeDefined(); - - expect(votingToResults).toBeDefined(); + expect(transitions.length).toBe(0); }); it('should accept phase settings', async ({ task, onTestFinished }) => { diff --git a/services/api/src/routers/decision/transitions/processTransitions.test.ts b/services/api/src/routers/decision/transitions/processTransitions.test.ts new file mode 100644 index 000000000..f326ef09e --- /dev/null +++ b/services/api/src/routers/decision/transitions/processTransitions.test.ts @@ -0,0 +1,2020 @@ +import { processDecisionsTransitions } from '@op/common'; +import type { DecisionSchemaDefinition } from '@op/common'; +import { db, eq } from '@op/db/client'; +import { + ProcessStatus, + ProposalStatus, + decisionProcessResultSelections, + decisionProcessResults, + decisionProcessTransitions, + decisionProcesses, + processInstances, + proposals, +} from '@op/db/schema'; +import { describe, expect, it } from 'vitest'; + +import { TestDecisionsDataManager } from '../../../test/helpers/TestDecisionsDataManager'; +import { + createIsolatedSession, + createTestContextWithSession, +} from '../../../test/supabase-utils'; +import { createCallerFactory } from '../../../trpcFactory'; +import { appRouter } from '../../index'; + +const createCaller = createCallerFactory(appRouter); + +async function createAuthenticatedCaller(email: string) { + const { session } = await createIsolatedSession(email); + return createCaller(await createTestContextWithSession(session)); +} + +// Helper to create dates relative to now +const createPastDate = (daysAgo: number) => { + const date = new Date(); + date.setDate(date.getDate() - daysAgo); + return date.toISOString(); +}; + +const createFutureDate = (daysFromNow: number) => { + const date = new Date(); + date.setDate(date.getDate() + daysFromNow); + return date.toISOString(); +}; + +// Standard 4-phase schema for transition tests using DecisionSchemaDefinition +const createDecisionSchema = (): DecisionSchemaDefinition => ({ + id: 'test-transition-schema', + version: '1.0.0', + name: 'Transition Test Process', + phases: [ + { + id: 'submission', + name: 'Submission', + rules: { advancement: { method: 'date' } }, + }, + { + id: 'review', + name: 'Review', + rules: { advancement: { method: 'date' } }, + }, + { + id: 'voting', + name: 'Voting', + rules: { advancement: { method: 'date' } }, + }, + { id: 'results', name: 'Results', rules: {} }, + ], +}); + +// Schema with all manual advancement phases (no automatic transitions) +const createManualAdvancementSchema = (): DecisionSchemaDefinition => ({ + id: 'test-manual-schema', + version: '1.0.0', + name: 'Manual Advancement Test Process', + phases: [ + { + id: 'submission', + name: 'Submission', + rules: { advancement: { method: 'manual' } }, + }, + { + id: 'review', + name: 'Review', + rules: { advancement: { method: 'manual' } }, + }, + { + id: 'voting', + name: 'Voting', + rules: { advancement: { method: 'manual' } }, + }, + { id: 'results', name: 'Results', rules: {} }, + ], +}); + +// Simple instance data (rules are omitted since tests manually insert transitions) +const createSimpleInstanceData = (currentPhaseId: string) => ({ + currentPhaseId, + phases: [ + { + phaseId: 'submission', + startDate: createPastDate(14), + endDate: createPastDate(7), + }, + { + phaseId: 'review', + startDate: createPastDate(7), + endDate: createPastDate(1), + }, + { + phaseId: 'voting', + startDate: createPastDate(1), + endDate: createFutureDate(7), + }, + { phaseId: 'results', startDate: createFutureDate(7) }, + ], +}); + +/** + * Helper to create a test instance with a manually inserted due transition. + * Transitions are manually inserted to control test scenarios precisely. + * By default, creates instances with 'published' status so transitions are processed. + */ +/** + * Helper to get the current phase ID for an instance via direct DB query. + * Avoids API encoder validation issues with new schema format. + */ +async function getInstanceCurrentPhaseId(instanceId: string): Promise { + const [instance] = await db + .select() + .from(processInstances) + .where(eq(processInstances.id, instanceId)); + + if (!instance) { + throw new Error(`Instance not found: ${instanceId}`); + } + + const instanceData = instance.instanceData as { currentPhaseId: string }; + return instanceData.currentPhaseId; +} + +async function createInstanceWithDueTransition( + testData: TestDecisionsDataManager, + setup: Awaited>, + caller: Awaited>, + options: { + name: string; + currentPhaseId: string; + fromStateId: string; + toStateId: string; + scheduledDate: string; + status?: ProcessStatus; + }, +) { + // Create instance without status first (avoids triggering createTransitionsForProcess + // before we update instanceData with our custom phase dates) + const instance = await testData.createInstanceForProcess({ + caller, + processId: setup.process.id, + name: options.name, + }); + + // Update instanceData with custom phase dates and status for test scenarios + await db + .update(processInstances) + .set({ + instanceData: createSimpleInstanceData(options.currentPhaseId), + currentStateId: options.currentPhaseId, + status: options.status ?? ProcessStatus.PUBLISHED, + }) + .where(eq(processInstances.id, instance.instance.id)); + + // Manually insert the transition record (we control transitions explicitly in tests) + await db.insert(decisionProcessTransitions).values({ + processInstanceId: instance.instance.id, + fromStateId: options.fromStateId, + toStateId: options.toStateId, + scheduledDate: options.scheduledDate, + }); + + await testData.grantProfileAccess( + instance.profileId, + setup.user.id, + setup.userEmail, + ); + + return instance; +} + +describe.concurrent('processDecisionsTransitions integration', () => { + describe('processing due transitions', () => { + it('should advance phase when transition is due', async ({ + task, + onTestFinished, + }) => { + const testData = new TestDecisionsDataManager(task.id, onTestFinished); + + const setup = await testData.createDecisionSetup({ + processName: 'Transition Test', + instanceCount: 0, + }); + + await db + .update(decisionProcesses) + .set({ processSchema: createDecisionSchema() }) + .where(eq(decisionProcesses.id, setup.process.id)); + + const caller = await createAuthenticatedCaller(setup.userEmail); + + // Use API-based helper - transitions are created automatically when publishing + const instance = await testData.createPublishedInstanceWithDueDates({ + caller, + processId: setup.process.id, + name: 'Due Transition Test', + phaseDates: [ + { + phaseId: 'submission', + startDate: createPastDate(14), + endDate: createPastDate(1), // Due yesterday - triggers transition + }, + { + phaseId: 'review', + startDate: createPastDate(1), + endDate: createFutureDate(7), + }, + { + phaseId: 'voting', + startDate: createFutureDate(7), + endDate: createFutureDate(14), + }, + { phaseId: 'results', startDate: createFutureDate(14) }, + ], + }); + + // Verify transitions were created for all date-based phases + const transitions = await db + .select() + .from(decisionProcessTransitions) + .where( + eq( + decisionProcessTransitions.processInstanceId, + instance.instance.id, + ), + ); + + // Should have 3 transitions: submission->review, review->voting, voting->results + expect(transitions).toHaveLength(3); + + // Find the submission->review transition (our due transition) + const dueTransition = transitions.find( + (t) => t.fromStateId === 'submission' && t.toStateId === 'review', + ); + expect(dueTransition).toBeDefined(); + + // If not yet processed by a concurrent test, process now + if (!dueTransition!.completedAt) { + const result = await processDecisionsTransitions(); + expect(result.failed).toBe(0); + } + + // Verify the instance state advanced (either by publish flow or processDecisionsTransitions) + const currentPhaseId = await getInstanceCurrentPhaseId( + instance.instance.id, + ); + expect(currentPhaseId).toBe('review'); + + // Verify the transition is now completed + const [updatedTransition] = await db + .select() + .from(decisionProcessTransitions) + .where(eq(decisionProcessTransitions.id, dueTransition!.id)); + expect(updatedTransition!.completedAt).not.toBeNull(); + }); + + it('should update both currentStateId and currentPhaseId', async ({ + task, + onTestFinished, + }) => { + const testData = new TestDecisionsDataManager(task.id, onTestFinished); + + const setup = await testData.createDecisionSetup({ + processName: 'State Update Test', + instanceCount: 0, + }); + + await db + .update(decisionProcesses) + .set({ processSchema: createDecisionSchema() }) + .where(eq(decisionProcesses.id, setup.process.id)); + + const caller = await createAuthenticatedCaller(setup.userEmail); + + // Use API-based helper - transitions are created automatically when publishing + const instance = await testData.createPublishedInstanceWithDueDates({ + caller, + processId: setup.process.id, + name: 'State Update Test', + phaseDates: [ + { + phaseId: 'submission', + startDate: createPastDate(14), + endDate: createPastDate(1), // Due yesterday + }, + { + phaseId: 'review', + startDate: createPastDate(1), + endDate: createFutureDate(7), + }, + { + phaseId: 'voting', + startDate: createFutureDate(7), + endDate: createFutureDate(14), + }, + { phaseId: 'results', startDate: createFutureDate(14) }, + ], + }); + + // Process due transitions (if not already processed by concurrent test) + await processDecisionsTransitions(); + + // Verify via direct DB query that both fields are updated + const [updatedInstance] = await db + .select() + .from(processInstances) + .where(eq(processInstances.id, instance.instance.id)); + + expect(updatedInstance).toBeDefined(); + expect(updatedInstance!.currentStateId).toBe('review'); + const instanceData = updatedInstance!.instanceData as { + currentPhaseId: string; + }; + expect(instanceData.currentPhaseId).toBe('review'); + }); + + it('should mark transition as completed', async ({ + task, + onTestFinished, + }) => { + const testData = new TestDecisionsDataManager(task.id, onTestFinished); + + const setup = await testData.createDecisionSetup({ + processName: 'Completion Test', + instanceCount: 0, + }); + + await db + .update(decisionProcesses) + .set({ processSchema: createDecisionSchema() }) + .where(eq(decisionProcesses.id, setup.process.id)); + + const caller = await createAuthenticatedCaller(setup.userEmail); + + // Use API-based helper - transitions are created automatically when publishing + const instance = await testData.createPublishedInstanceWithDueDates({ + caller, + processId: setup.process.id, + name: 'Completion Test', + phaseDates: [ + { + phaseId: 'submission', + startDate: createPastDate(14), + endDate: createPastDate(1), // Due yesterday + }, + { + phaseId: 'review', + startDate: createPastDate(1), + endDate: createFutureDate(7), + }, + { + phaseId: 'voting', + startDate: createFutureDate(7), + endDate: createFutureDate(14), + }, + { phaseId: 'results', startDate: createFutureDate(14) }, + ], + }); + + // Find the submission->review transition + const dueTransition = + await db._query.decisionProcessTransitions.findFirst({ + where: (t, { and, eq }) => + and( + eq(t.processInstanceId, instance.instance.id), + eq(t.fromStateId, 'submission'), + eq(t.toStateId, 'review'), + ), + }); + + expect(dueTransition).toBeDefined(); + + // Process due transitions (if not already processed by concurrent test) + if (!dueTransition!.completedAt) { + await processDecisionsTransitions(); + } + + // Verify the transition is marked as completed + const [transitionAfter] = await db + .select() + .from(decisionProcessTransitions) + .where(eq(decisionProcessTransitions.id, dueTransition!.id)); + + expect(transitionAfter).toBeDefined(); + expect(transitionAfter!.completedAt).not.toBeNull(); + }); + }); + + describe('final phase transitions', () => { + it('should correctly transition to final phase (results)', async ({ + task, + onTestFinished, + }) => { + const testData = new TestDecisionsDataManager(task.id, onTestFinished); + + const setup = await testData.createDecisionSetup({ + processName: 'Final Phase Test', + instanceCount: 0, + }); + + await db + .update(decisionProcesses) + .set({ processSchema: createDecisionSchema() }) + .where(eq(decisionProcesses.id, setup.process.id)); + + const caller = await createAuthenticatedCaller(setup.userEmail); + + const instance = await createInstanceWithDueTransition( + testData, + setup, + caller, + { + name: 'Final Phase Test', + currentPhaseId: 'voting', + fromStateId: 'voting', + toStateId: 'results', + scheduledDate: createPastDate(1), + }, + ); + + const result = await processDecisionsTransitions(); + + expect(result.processed).toBeGreaterThanOrEqual(1); + expect(result.failed).toBe(0); + + // Verify instance advanced to results (final phase) + const currentPhaseId = await getInstanceCurrentPhaseId( + instance.instance.id, + ); + expect(currentPhaseId).toBe('results'); + }); + }); + + describe('skipping non-due transitions', () => { + it('should not process future transitions', async ({ + task, + onTestFinished, + }) => { + const testData = new TestDecisionsDataManager(task.id, onTestFinished); + + const setup = await testData.createDecisionSetup({ + processName: 'Future Transitions', + instanceCount: 0, + }); + + await db + .update(decisionProcesses) + .set({ processSchema: createDecisionSchema() }) + .where(eq(decisionProcesses.id, setup.process.id)); + + const caller = await createAuthenticatedCaller(setup.userEmail); + + // Create instance with a future transition + // Don't pass status to avoid triggering createTransitionsForProcess with default phase data + const instance = await testData.createInstanceForProcess({ + caller, + processId: setup.process.id, + name: 'Future Instance', + }); + + // Update instanceData with custom phase dates and status for test scenario + await db + .update(processInstances) + .set({ + instanceData: createSimpleInstanceData('submission'), + currentStateId: 'submission', + status: ProcessStatus.PUBLISHED, // Must be published for meaningful test + }) + .where(eq(processInstances.id, instance.instance.id)); + + // Insert a future transition + await db.insert(decisionProcessTransitions).values({ + processInstanceId: instance.instance.id, + fromStateId: 'submission', + toStateId: 'review', + scheduledDate: createFutureDate(7), // Not due yet + }); + + await testData.grantProfileAccess( + instance.profileId, + setup.user.id, + setup.userEmail, + ); + + // Process transitions - future transition should not be processed + await processDecisionsTransitions(); + + // Verify the instance state has not changed + const currentPhaseId = await getInstanceCurrentPhaseId( + instance.instance.id, + ); + expect(currentPhaseId).toBe('submission'); + + // Verify transition is still pending + const [transition] = await db + .select() + .from(decisionProcessTransitions) + .where( + eq( + decisionProcessTransitions.processInstanceId, + instance.instance.id, + ), + ); + + expect(transition).toBeDefined(); + expect(transition!.completedAt).toBeNull(); + }); + + it('should skip already completed transitions', async ({ + task, + onTestFinished, + }) => { + const testData = new TestDecisionsDataManager(task.id, onTestFinished); + + const setup = await testData.createDecisionSetup({ + processName: 'Completed Transitions', + instanceCount: 0, + }); + + await db + .update(decisionProcesses) + .set({ processSchema: createDecisionSchema() }) + .where(eq(decisionProcesses.id, setup.process.id)); + + const caller = await createAuthenticatedCaller(setup.userEmail); + + // Don't pass status to avoid triggering createTransitionsForProcess with default phase data + const instance = await testData.createInstanceForProcess({ + caller, + processId: setup.process.id, + name: 'Completed Test', + }); + + // Update instanceData with custom phase dates and status for test scenario + await db + .update(processInstances) + .set({ + instanceData: createSimpleInstanceData('submission'), + currentStateId: 'submission', + status: ProcessStatus.PUBLISHED, // Must be published for meaningful test + }) + .where(eq(processInstances.id, instance.instance.id)); + + const originalCompletedAt = createPastDate(1); + + // Insert an already completed transition + await db.insert(decisionProcessTransitions).values({ + processInstanceId: instance.instance.id, + fromStateId: 'submission', + toStateId: 'review', + scheduledDate: createPastDate(1), + completedAt: originalCompletedAt, // Already completed + }); + + await testData.grantProfileAccess( + instance.profileId, + setup.user.id, + setup.userEmail, + ); + + // Process transitions - completed transition should be skipped + const result = await processDecisionsTransitions(); + + // This instance's transition should not contribute to the processed count + expect(result.failed).toBe(0); + + // Verify instance state has NOT changed (still in submission phase) + const currentPhaseId = await getInstanceCurrentPhaseId( + instance.instance.id, + ); + expect(currentPhaseId).toBe('submission'); + + // Verify completedAt timestamp wasn't modified (transition wasn't re-processed) + const [transitionAfter] = await db + .select() + .from(decisionProcessTransitions) + .where( + eq( + decisionProcessTransitions.processInstanceId, + instance.instance.id, + ), + ); + expect(transitionAfter).toBeDefined(); + expect(new Date(transitionAfter!.completedAt!).toISOString()).toBe( + originalCompletedAt, + ); + }); + }); + + describe('multi-instance processing', () => { + it('should process transitions across multiple instances', async ({ + task, + onTestFinished, + }) => { + const testData = new TestDecisionsDataManager(task.id, onTestFinished); + + const setup = await testData.createDecisionSetup({ + processName: 'Multi-Instance Test', + instanceCount: 0, + }); + + await db + .update(decisionProcesses) + .set({ processSchema: createDecisionSchema() }) + .where(eq(decisionProcesses.id, setup.process.id)); + + const caller = await createAuthenticatedCaller(setup.userEmail); + + const phaseDates = [ + { + phaseId: 'submission', + startDate: createPastDate(14), + endDate: createPastDate(1), // Due yesterday + }, + { + phaseId: 'review', + startDate: createPastDate(1), + endDate: createFutureDate(7), + }, + { + phaseId: 'voting', + startDate: createFutureDate(7), + endDate: createFutureDate(14), + }, + { phaseId: 'results', startDate: createFutureDate(14) }, + ]; + + // Create two instances with due transitions via API + const instance1 = await testData.createPublishedInstanceWithDueDates({ + caller, + processId: setup.process.id, + name: 'Multi Instance 1', + phaseDates, + }); + + const instance2 = await testData.createPublishedInstanceWithDueDates({ + caller, + processId: setup.process.id, + name: 'Multi Instance 2', + phaseDates, + }); + + // Process all due transitions + const result = await processDecisionsTransitions(); + + // Result includes transitions from all concurrent tests, so just verify no failures + expect(result.failed).toBe(0); + + // Verify BOTH of our specific instances advanced (this is the important check) + const currentPhaseId1 = await getInstanceCurrentPhaseId( + instance1.instance.id, + ); + const currentPhaseId2 = await getInstanceCurrentPhaseId( + instance2.instance.id, + ); + + expect(currentPhaseId1).toBe('review'); + expect(currentPhaseId2).toBe('review'); + + // Verify both submission->review transitions were marked completed + const transition1 = await db._query.decisionProcessTransitions.findFirst({ + where: (t, { and, eq }) => + and( + eq(t.processInstanceId, instance1.instance.id), + eq(t.fromStateId, 'submission'), + ), + }); + const transition2 = await db._query.decisionProcessTransitions.findFirst({ + where: (t, { and, eq }) => + and( + eq(t.processInstanceId, instance2.instance.id), + eq(t.fromStateId, 'submission'), + ), + }); + + expect(transition1?.completedAt).toBeTruthy(); + expect(transition2?.completedAt).toBeTruthy(); + }); + + it('should process multiple sequential transitions within same instance', async ({ + task, + onTestFinished, + }) => { + const testData = new TestDecisionsDataManager(task.id, onTestFinished); + + const setup = await testData.createDecisionSetup({ + processName: 'Sequential Transitions', + instanceCount: 0, + }); + + await db + .update(decisionProcesses) + .set({ processSchema: createDecisionSchema() }) + .where(eq(decisionProcesses.id, setup.process.id)); + + const caller = await createAuthenticatedCaller(setup.userEmail); + + // Create instance with TWO due transitions via API + // Both submission and review phases have past end dates + const instance = await testData.createPublishedInstanceWithDueDates({ + caller, + processId: setup.process.id, + name: 'Sequential Test', + phaseDates: [ + { + phaseId: 'submission', + startDate: createPastDate(21), + endDate: createPastDate(14), // Due 2 weeks ago + }, + { + phaseId: 'review', + startDate: createPastDate(14), + endDate: createPastDate(7), // Due 1 week ago + }, + { + phaseId: 'voting', + startDate: createPastDate(7), + endDate: createFutureDate(7), + }, + { phaseId: 'results', startDate: createFutureDate(7) }, + ], + }); + + // Process transitions - should process both sequentially + const result = await processDecisionsTransitions(); + + expect(result.failed).toBe(0); + + // Verify the instance advanced through both transitions to voting + const currentPhaseId = await getInstanceCurrentPhaseId( + instance.instance.id, + ); + expect(currentPhaseId).toBe('voting'); + }); + }); + + describe('result structure', () => { + it('should return result object with correct structure', async ({ + task, + onTestFinished, + }) => { + const testData = new TestDecisionsDataManager(task.id, onTestFinished); + + const setup = await testData.createDecisionSetup({ + processName: 'Count Test', + instanceCount: 0, + }); + + await db + .update(decisionProcesses) + .set({ processSchema: createDecisionSchema() }) + .where(eq(decisionProcesses.id, setup.process.id)); + + const caller = await createAuthenticatedCaller(setup.userEmail); + + // Create instance with a due transition via API + await testData.createPublishedInstanceWithDueDates({ + caller, + processId: setup.process.id, + name: 'Count Test', + phaseDates: [ + { + phaseId: 'submission', + startDate: createPastDate(14), + endDate: createPastDate(1), // Due yesterday + }, + { + phaseId: 'review', + startDate: createPastDate(1), + endDate: createFutureDate(7), + }, + { + phaseId: 'voting', + startDate: createFutureDate(7), + endDate: createFutureDate(14), + }, + { phaseId: 'results', startDate: createFutureDate(14) }, + ], + }); + + const result = await processDecisionsTransitions(); + + // Verify result object structure + expect(typeof result.processed).toBe('number'); + expect(typeof result.failed).toBe('number'); + expect(Array.isArray(result.errors)).toBe(true); + // Note: Can't guarantee processed count due to concurrent tests potentially processing first + }); + }); + + describe('transition creation rules', () => { + it('should not create transitions for manual advancement phases', async ({ + task, + onTestFinished, + }) => { + const testData = new TestDecisionsDataManager(task.id, onTestFinished); + + const setup = await testData.createDecisionSetup({ + processName: 'Manual Advancement Test', + instanceCount: 0, + }); + + // Use schema with all manual advancement phases + await db + .update(decisionProcesses) + .set({ processSchema: createManualAdvancementSchema() }) + .where(eq(decisionProcesses.id, setup.process.id)); + + const caller = await createAuthenticatedCaller(setup.userEmail); + + // Publish instance via API - this should trigger createTransitionsForProcess + // but since all phases use method: 'manual', no transitions should be created + const instance = await testData.createPublishedInstanceWithDueDates({ + caller, + processId: setup.process.id, + name: 'Manual Advancement Instance', + phaseDates: [ + { + phaseId: 'submission', + startDate: createPastDate(14), + endDate: createPastDate(1), // Would be "due" if it were date-based + }, + { + phaseId: 'review', + startDate: createPastDate(1), + endDate: createFutureDate(7), + }, + { + phaseId: 'voting', + startDate: createFutureDate(7), + endDate: createFutureDate(14), + }, + { phaseId: 'results', startDate: createFutureDate(14) }, + ], + }); + + // Verify NO transitions were created for this instance + const transitions = await db + .select() + .from(decisionProcessTransitions) + .where( + eq( + decisionProcessTransitions.processInstanceId, + instance.instance.id, + ), + ); + + expect(transitions).toHaveLength(0); + + // Verify instance remains in submission phase (not advanced) + const currentPhaseId = await getInstanceCurrentPhaseId( + instance.instance.id, + ); + expect(currentPhaseId).toBe('submission'); + }); + + it('should have no pending transitions for instance in final phase', async ({ + task, + onTestFinished, + }) => { + const testData = new TestDecisionsDataManager(task.id, onTestFinished); + + const setup = await testData.createDecisionSetup({ + processName: 'No Final Transitions', + instanceCount: 0, + }); + + await db + .update(decisionProcesses) + .set({ processSchema: createDecisionSchema() }) + .where(eq(decisionProcesses.id, setup.process.id)); + + const caller = await createAuthenticatedCaller(setup.userEmail); + + // Create instance already in results phase (no transition inserted) + // Don't pass status to avoid triggering createTransitionsForProcess with default phase data + const instance = await testData.createInstanceForProcess({ + caller, + processId: setup.process.id, + name: 'Already Final', + }); + + // Update instanceData with custom phase dates and status for test scenario + await db + .update(processInstances) + .set({ + instanceData: { + currentPhaseId: 'results', + phases: [ + { + phaseId: 'submission', + startDate: createPastDate(21), + endDate: createPastDate(14), + }, + { + phaseId: 'review', + startDate: createPastDate(14), + endDate: createPastDate(7), + }, + { + phaseId: 'voting', + startDate: createPastDate(7), + endDate: createPastDate(1), + }, + { phaseId: 'results', startDate: createPastDate(1) }, + ], + }, + currentStateId: 'results', + status: ProcessStatus.PUBLISHED, + }) + .where(eq(processInstances.id, instance.instance.id)); + + await testData.grantProfileAccess( + instance.profileId, + setup.user.id, + setup.userEmail, + ); + + // Verify no transitions exist for this instance + const transitions = await db._query.decisionProcessTransitions.findMany({ + where: eq( + decisionProcessTransitions.processInstanceId, + instance.instance.id, + ), + }); + + expect(transitions).toHaveLength(0); + + // Verify instance remains in results phase + const currentPhaseId = await getInstanceCurrentPhaseId( + instance.instance.id, + ); + expect(currentPhaseId).toBe('results'); + }); + }); + + describe('deferred state update behavior', () => { + /** + * Tests that the instance state is only updated ONCE after all transitions + * for that instance are processed, using the toStateId of the LAST transition. + * + * This is important because: + * - Orphaned transitions from earlier phases may still be pending + * - Multiple transitions may be due at the same time + * - The final state should reflect the most recent scheduled transition + */ + it('should update instance state only once after all transitions are processed', async ({ + task, + onTestFinished, + }) => { + const testData = new TestDecisionsDataManager(task.id, onTestFinished); + + const setup = await testData.createDecisionSetup({ + processName: 'Deferred State Update Test', + instanceCount: 0, + }); + + await db + .update(decisionProcesses) + .set({ processSchema: createDecisionSchema() }) + .where(eq(decisionProcesses.id, setup.process.id)); + + const caller = await createAuthenticatedCaller(setup.userEmail); + + // Create instance starting in 'submission' phase + const instance = await testData.createInstanceForProcess({ + caller, + processId: setup.process.id, + name: 'Deferred State Test', + }); + + // Update instanceData to be in 'submission' phase + await db + .update(processInstances) + .set({ + instanceData: createSimpleInstanceData('submission'), + currentStateId: 'submission', + status: ProcessStatus.PUBLISHED, + }) + .where(eq(processInstances.id, instance.instance.id)); + + // Insert TWO due transitions that should both be processed + // Transition 1: submission -> review (scheduled earlier) + await db.insert(decisionProcessTransitions).values({ + processInstanceId: instance.instance.id, + fromStateId: 'submission', + toStateId: 'review', + scheduledDate: createPastDate(7), // Scheduled 7 days ago + }); + + // Transition 2: review -> voting (scheduled more recently) + await db.insert(decisionProcessTransitions).values({ + processInstanceId: instance.instance.id, + fromStateId: 'review', + toStateId: 'voting', + scheduledDate: createPastDate(1), // Scheduled 1 day ago + }); + + await testData.grantProfileAccess( + instance.profileId, + setup.user.id, + setup.userEmail, + ); + + // Process transitions - both should be processed, state updated to final transition's toStateId + const result = await processDecisionsTransitions(); + + // Check that THIS instance's transitions didn't fail + // (Other concurrent tests may have failing transitions) + const thisInstanceErrors = result.errors.filter( + (e) => e.processInstanceId === instance.instance.id, + ); + expect(thisInstanceErrors).toHaveLength(0); + + // Instance should be in 'voting' (the toStateId of the last transition) + const currentPhaseId = await getInstanceCurrentPhaseId( + instance.instance.id, + ); + expect(currentPhaseId).toBe('voting'); + + // Both transitions should be marked complete + const transitions = await db + .select() + .from(decisionProcessTransitions) + .where( + eq( + decisionProcessTransitions.processInstanceId, + instance.instance.id, + ), + ); + + expect(transitions).toHaveLength(2); + expect(transitions.every((t) => t.completedAt !== null)).toBe(true); + }); + + /** + * Tests that orphaned transitions from earlier phases are processed correctly + * when they exist alongside newer transitions. + * + * Scenario: An older transition (submission->review) wasn't processed before, + * and now both it and a newer transition (review->voting) are due. + * The instance should end up in 'voting' (the final transition's toStateId). + */ + it('should handle orphaned transitions from earlier phases correctly', async ({ + task, + onTestFinished, + }) => { + const testData = new TestDecisionsDataManager(task.id, onTestFinished); + + const setup = await testData.createDecisionSetup({ + processName: 'Orphaned Transition Test', + instanceCount: 0, + }); + + await db + .update(decisionProcesses) + .set({ processSchema: createDecisionSchema() }) + .where(eq(decisionProcesses.id, setup.process.id)); + + const caller = await createAuthenticatedCaller(setup.userEmail); + + // Create instance - it's currently in 'review' phase (maybe advanced manually) + // but there's still an old orphaned transition from submission->review + const instance = await testData.createInstanceForProcess({ + caller, + processId: setup.process.id, + name: 'Orphaned Transition Test', + }); + + // Update instanceData to be in 'review' phase + await db + .update(processInstances) + .set({ + instanceData: createSimpleInstanceData('review'), + currentStateId: 'review', + status: ProcessStatus.PUBLISHED, + }) + .where(eq(processInstances.id, instance.instance.id)); + + // Insert orphaned transition from earlier phase (scheduled long ago) + await db.insert(decisionProcessTransitions).values({ + processInstanceId: instance.instance.id, + fromStateId: 'submission', // Orphaned - instance is already in 'review' + toStateId: 'review', + scheduledDate: createPastDate(14), // Scheduled 14 days ago + }); + + // Insert current transition (scheduled more recently) + await db.insert(decisionProcessTransitions).values({ + processInstanceId: instance.instance.id, + fromStateId: 'review', + toStateId: 'voting', + scheduledDate: createPastDate(1), // Scheduled 1 day ago + }); + + await testData.grantProfileAccess( + instance.profileId, + setup.user.id, + setup.userEmail, + ); + + // Process transitions + const result = await processDecisionsTransitions(); + + // Check that THIS instance's transitions didn't fail + // (Other concurrent tests may have failing transitions) + const thisInstanceErrors = result.errors.filter( + (e) => e.processInstanceId === instance.instance.id, + ); + expect(thisInstanceErrors).toHaveLength(0); + + // Instance should be in 'voting' (the final transition's toStateId) + // NOT 'review' (which would happen if we updated state after each transition) + const currentPhaseId = await getInstanceCurrentPhaseId( + instance.instance.id, + ); + expect(currentPhaseId).toBe('voting'); + + // Both transitions should be marked complete + const transitions = await db + .select() + .from(decisionProcessTransitions) + .where( + eq( + decisionProcessTransitions.processInstanceId, + instance.instance.id, + ), + ); + + expect(transitions).toHaveLength(2); + expect(transitions.every((t) => t.completedAt !== null)).toBe(true); + }); + + /** + * Documents current behavior: the implementation does NOT validate that + * fromStateId matches the instance's currentStateId before processing. + * + * When a mismatched transition is processed: + * - The transition is marked as completed + * - The instance state is updated to the final transition's toStateId + */ + it('should process transition even when currentPhaseId does not match fromStateId', async ({ + task, + onTestFinished, + }) => { + const testData = new TestDecisionsDataManager(task.id, onTestFinished); + + const setup = await testData.createDecisionSetup({ + processName: 'Phase Mismatch Test', + instanceCount: 0, + }); + + await db + .update(decisionProcesses) + .set({ processSchema: createDecisionSchema() }) + .where(eq(decisionProcesses.id, setup.process.id)); + + const caller = await createAuthenticatedCaller(setup.userEmail); + + // Create instance in 'review' phase + const instance = await testData.createInstanceForProcess({ + caller, + processId: setup.process.id, + name: 'Phase Mismatch Test', + }); + + // Update instanceData to be in 'review' phase + await db + .update(processInstances) + .set({ + instanceData: createSimpleInstanceData('review'), + currentStateId: 'review', + status: ProcessStatus.PUBLISHED, + }) + .where(eq(processInstances.id, instance.instance.id)); + + // Insert a mismatched transition: fromStateId='submission' but instance is in 'review' + // In this case, toStateId also happens to be 'review', so the end state is the same + await db.insert(decisionProcessTransitions).values({ + processInstanceId: instance.instance.id, + fromStateId: 'submission', // Mismatched - instance is actually in 'review' + toStateId: 'review', + scheduledDate: createPastDate(1), // Due + }); + + await testData.grantProfileAccess( + instance.profileId, + setup.user.id, + setup.userEmail, + ); + + // Process transitions - current behavior: processes regardless of mismatch + const result = await processDecisionsTransitions(); + + // Check that THIS instance's transitions didn't fail + // (Other concurrent tests may have failing transitions) + const thisInstanceErrors = result.errors.filter( + (e) => e.processInstanceId === instance.instance.id, + ); + expect(thisInstanceErrors).toHaveLength(0); + + // Instance remains in 'review' (toStateId matches current state, so no visible change) + const currentPhaseId = await getInstanceCurrentPhaseId( + instance.instance.id, + ); + expect(currentPhaseId).toBe('review'); + + // Current behavior: transition IS processed and marked complete + const [transition] = await db + .select() + .from(decisionProcessTransitions) + .where( + eq( + decisionProcessTransitions.processInstanceId, + instance.instance.id, + ), + ); + + expect(transition).toBeDefined(); + expect(transition!.completedAt).not.toBeNull(); + }); + }); + + describe('error handling', () => { + /** + * Tests that when a transition fails to process, the error is captured + * and the instance state is not updated. + * + * This test corrupts the instanceData to cause processTransition to fail. + */ + it('should capture errors and not update state when transition processing fails', async ({ + task, + onTestFinished, + }) => { + const testData = new TestDecisionsDataManager(task.id, onTestFinished); + + const setup = await testData.createDecisionSetup({ + processName: 'Error Handling Test', + instanceCount: 0, + }); + + await db + .update(decisionProcesses) + .set({ processSchema: createDecisionSchema() }) + .where(eq(decisionProcesses.id, setup.process.id)); + + const caller = await createAuthenticatedCaller(setup.userEmail); + + // Create instance + const instance = await testData.createInstanceForProcess({ + caller, + processId: setup.process.id, + name: 'Error Test Instance', + }); + + // Set up instance with CORRUPTED instanceData (missing phases array) + // This will cause processTransition to fail when it tries to access phases + await db + .update(processInstances) + .set({ + instanceData: { + currentPhaseId: 'submission', + // phases array is MISSING - this will cause an error + }, + currentStateId: 'submission', + status: ProcessStatus.PUBLISHED, + }) + .where(eq(processInstances.id, instance.instance.id)); + + // Insert a due transition + await db.insert(decisionProcessTransitions).values({ + processInstanceId: instance.instance.id, + fromStateId: 'submission', + toStateId: 'review', + scheduledDate: createPastDate(1), + }); + + await testData.grantProfileAccess( + instance.profileId, + setup.user.id, + setup.userEmail, + ); + + // Process transitions - should fail due to corrupted instanceData + const result = await processDecisionsTransitions(); + + // Verify error was captured + const instanceError = result.errors.find( + (e) => e.processInstanceId === instance.instance.id, + ); + expect(instanceError).toBeDefined(); + expect(result.failed).toBeGreaterThanOrEqual(1); + + // Verify instance state was NOT updated (still in submission) + const currentPhaseId = await getInstanceCurrentPhaseId( + instance.instance.id, + ); + expect(currentPhaseId).toBe('submission'); + + // Verify transition was NOT marked as completed + const [transition] = await db + .select() + .from(decisionProcessTransitions) + .where( + eq( + decisionProcessTransitions.processInstanceId, + instance.instance.id, + ), + ); + expect(transition).toBeDefined(); + expect(transition!.completedAt).toBeNull(); + }); + + /** + * Tests that a successful instance and a failing instance are handled independently. + * The successful instance should advance, while the failing instance should not. + * + * NOTE: In concurrent tests, result counts may vary because processDecisionsTransitions + * is a global operation. We verify behavior by checking actual instance states. + */ + it('should handle successful and failing instances independently', async ({ + task, + onTestFinished, + }) => { + const testData = new TestDecisionsDataManager(task.id, onTestFinished); + + const setup = await testData.createDecisionSetup({ + processName: 'Mixed Success Test', + instanceCount: 0, + }); + + await db + .update(decisionProcesses) + .set({ processSchema: createDecisionSchema() }) + .where(eq(decisionProcesses.id, setup.process.id)); + + const caller = await createAuthenticatedCaller(setup.userEmail); + + // Create first instance with valid transition (will succeed) + const instance1 = await testData.createInstanceForProcess({ + caller, + processId: setup.process.id, + name: 'Success Instance', + }); + + await db + .update(processInstances) + .set({ + instanceData: createSimpleInstanceData('submission'), + currentStateId: 'submission', + status: ProcessStatus.PUBLISHED, + }) + .where(eq(processInstances.id, instance1.instance.id)); + + await db.insert(decisionProcessTransitions).values({ + processInstanceId: instance1.instance.id, + fromStateId: 'submission', + toStateId: 'review', + scheduledDate: createPastDate(1), + }); + + await testData.grantProfileAccess( + instance1.profileId, + setup.user.id, + setup.userEmail, + ); + + // Create second instance with corrupted data (will fail) + const instance2 = await testData.createInstanceForProcess({ + caller, + processId: setup.process.id, + name: 'Failure Instance', + }); + + await db + .update(processInstances) + .set({ + instanceData: { + currentPhaseId: 'submission', + // Missing phases - will cause error + }, + currentStateId: 'submission', + status: ProcessStatus.PUBLISHED, + }) + .where(eq(processInstances.id, instance2.instance.id)); + + await db.insert(decisionProcessTransitions).values({ + processInstanceId: instance2.instance.id, + fromStateId: 'submission', + toStateId: 'review', + scheduledDate: createPastDate(1), + }); + + await testData.grantProfileAccess( + instance2.profileId, + setup.user.id, + setup.userEmail, + ); + + // Process all transitions + const result = await processDecisionsTransitions(); + + // Verify no unexpected errors (instance2's error is expected) + expect(result.failed).toBeGreaterThanOrEqual(0); + + // Instance 1 should have advanced to review (success) + // Either processed by this call or a concurrent test's call + const phase1 = await getInstanceCurrentPhaseId(instance1.instance.id); + expect(phase1).toBe('review'); + + // Instance 2 should still be in submission (failure) + const phase2 = await getInstanceCurrentPhaseId(instance2.instance.id); + expect(phase2).toBe('submission'); + + // Instance 1's transition should be completed + const [transition1] = await db + .select() + .from(decisionProcessTransitions) + .where( + eq( + decisionProcessTransitions.processInstanceId, + instance1.instance.id, + ), + ); + expect(transition1!.completedAt).not.toBeNull(); + + // Instance 2's transition should NOT be completed + const [transition2] = await db + .select() + .from(decisionProcessTransitions) + .where( + eq( + decisionProcessTransitions.processInstanceId, + instance2.instance.id, + ), + ); + expect(transition2!.completedAt).toBeNull(); + + // Verify error was captured for instance2 + const instance2Error = result.errors.find( + (e) => e.processInstanceId === instance2.instance.id, + ); + // Note: In concurrent tests, the error might be captured by another test's call + // So we just verify the instance state is correct (checked above) + if (instance2Error) { + expect(instance2Error.error).toContain('length'); + } + }); + }); + + describe('instance status filtering', () => { + it('should not process transitions for draft instances', async ({ + task, + onTestFinished, + }) => { + const testData = new TestDecisionsDataManager(task.id, onTestFinished); + + const setup = await testData.createDecisionSetup({ + processName: 'Draft Status Test', + instanceCount: 0, + }); + + await db + .update(decisionProcesses) + .set({ processSchema: createDecisionSchema() }) + .where(eq(decisionProcesses.id, setup.process.id)); + + const caller = await createAuthenticatedCaller(setup.userEmail); + + // Create a DRAFT instance with a due transition + const instance = await createInstanceWithDueTransition( + testData, + setup, + caller, + { + name: 'Draft Instance Test', + currentPhaseId: 'submission', + fromStateId: 'submission', + toStateId: 'review', + scheduledDate: createPastDate(1), + status: ProcessStatus.DRAFT, // Explicitly set to draft + }, + ); + + // Process transitions - draft instance should be skipped + await processDecisionsTransitions(); + + // Verify the instance state has NOT changed + const currentPhaseId = await getInstanceCurrentPhaseId( + instance.instance.id, + ); + expect(currentPhaseId).toBe('submission'); + + // Verify the transition is still pending (not completed) + const [transition] = await db + .select() + .from(decisionProcessTransitions) + .where( + eq( + decisionProcessTransitions.processInstanceId, + instance.instance.id, + ), + ); + + expect(transition).toBeDefined(); + expect(transition!.completedAt).toBeNull(); + }); + + it('should not process transitions for completed instances', async ({ + task, + onTestFinished, + }) => { + const testData = new TestDecisionsDataManager(task.id, onTestFinished); + + const setup = await testData.createDecisionSetup({ + processName: 'Completed Status Test', + instanceCount: 0, + }); + + await db + .update(decisionProcesses) + .set({ processSchema: createDecisionSchema() }) + .where(eq(decisionProcesses.id, setup.process.id)); + + const caller = await createAuthenticatedCaller(setup.userEmail); + + // Create a COMPLETED instance with a due transition + const instance = await createInstanceWithDueTransition( + testData, + setup, + caller, + { + name: 'Completed Instance Test', + currentPhaseId: 'submission', + fromStateId: 'submission', + toStateId: 'review', + scheduledDate: createPastDate(1), + status: ProcessStatus.COMPLETED, // Explicitly set to completed + }, + ); + + // Process transitions - completed instance should be skipped + await processDecisionsTransitions(); + + // Verify the instance state has NOT changed + const currentPhaseId = await getInstanceCurrentPhaseId( + instance.instance.id, + ); + expect(currentPhaseId).toBe('submission'); + + // Verify the transition is still pending (not completed) + const [transition] = await db + .select() + .from(decisionProcessTransitions) + .where( + eq( + decisionProcessTransitions.processInstanceId, + instance.instance.id, + ), + ); + + expect(transition).toBeDefined(); + expect(transition!.completedAt).toBeNull(); + }); + + it('should not process transitions for cancelled instances', async ({ + task, + onTestFinished, + }) => { + const testData = new TestDecisionsDataManager(task.id, onTestFinished); + + const setup = await testData.createDecisionSetup({ + processName: 'Cancelled Status Test', + instanceCount: 0, + }); + + await db + .update(decisionProcesses) + .set({ processSchema: createDecisionSchema() }) + .where(eq(decisionProcesses.id, setup.process.id)); + + const caller = await createAuthenticatedCaller(setup.userEmail); + + // Create a CANCELLED instance with a due transition + const instance = await createInstanceWithDueTransition( + testData, + setup, + caller, + { + name: 'Cancelled Instance Test', + currentPhaseId: 'submission', + fromStateId: 'submission', + toStateId: 'review', + scheduledDate: createPastDate(1), + status: ProcessStatus.CANCELLED, // Explicitly set to cancelled + }, + ); + + // Process transitions - cancelled instance should be skipped + await processDecisionsTransitions(); + + // Verify the instance state has NOT changed + const currentPhaseId = await getInstanceCurrentPhaseId( + instance.instance.id, + ); + expect(currentPhaseId).toBe('submission'); + + // Verify the transition is still pending (not completed) + const [transition] = await db + .select() + .from(decisionProcessTransitions) + .where( + eq( + decisionProcessTransitions.processInstanceId, + instance.instance.id, + ), + ); + + expect(transition).toBeDefined(); + expect(transition!.completedAt).toBeNull(); + }); + }); + + describe('results processing on final phase transition', () => { + /** + * Tests that when transitioning to the final (results) phase, + * the processResults function is called and creates a result record. + */ + it('should create a result record when transitioning to final phase', async ({ + task, + onTestFinished, + }) => { + const testData = new TestDecisionsDataManager(task.id, onTestFinished); + + const setup = await testData.createDecisionSetup({ + processName: 'Results Processing Test', + instanceCount: 0, + }); + + await db + .update(decisionProcesses) + .set({ processSchema: createDecisionSchema() }) + .where(eq(decisionProcesses.id, setup.process.id)); + + const caller = await createAuthenticatedCaller(setup.userEmail); + + // Create instance in 'voting' phase with transition to 'results' (final phase) + const instance = await createInstanceWithDueTransition( + testData, + setup, + caller, + { + name: 'Results Test Instance', + currentPhaseId: 'voting', + fromStateId: 'voting', + toStateId: 'results', // Final phase + scheduledDate: createPastDate(1), + }, + ); + + // Process transitions - should transition to results and trigger processResults + const result = await processDecisionsTransitions(); + + // Check that THIS instance's transitions didn't fail + const thisInstanceErrors = result.errors.filter( + (e) => e.processInstanceId === instance.instance.id, + ); + expect(thisInstanceErrors).toHaveLength(0); + + // Verify instance advanced to results (final phase) + const currentPhaseId = await getInstanceCurrentPhaseId( + instance.instance.id, + ); + expect(currentPhaseId).toBe('results'); + + // Verify a result record was created for this instance + const [resultRecord] = await db + .select() + .from(decisionProcessResults) + .where( + eq(decisionProcessResults.processInstanceId, instance.instance.id), + ); + + expect(resultRecord).toBeDefined(); + expect(resultRecord!.processInstanceId).toBe(instance.instance.id); + expect(resultRecord!.success).toBe(true); + expect(resultRecord!.selectedCount).toBe(0); // No proposals + expect(resultRecord!.voterCount).toBe(0); // No voters + }); + + /** + * Tests that when transitioning to the final phase with proposals, + * the proposals are selected based on the pipeline and marked as SELECTED. + */ + it('should select proposals and mark them as SELECTED when transitioning to final phase', async ({ + task, + onTestFinished, + }) => { + const testData = new TestDecisionsDataManager(task.id, onTestFinished); + + const setup = await testData.createDecisionSetup({ + processName: 'Results With Proposals Test', + instanceCount: 0, + }); + + // Create schema with proposal submission enabled in earlier phases + const schemaWithProposals: DecisionSchemaDefinition = { + id: 'test-results-schema', + version: '1.0.0', + name: 'Results Test Process', + phases: [ + { + id: 'submission', + name: 'Submission', + rules: { + advancement: { method: 'date' }, + proposals: { submit: true }, + }, + }, + { + id: 'review', + name: 'Review', + rules: { advancement: { method: 'date' } }, + }, + { + id: 'voting', + name: 'Voting', + rules: { advancement: { method: 'date' } }, + }, + { id: 'results', name: 'Results', rules: {} }, + ], + }; + + await db + .update(decisionProcesses) + .set({ processSchema: schemaWithProposals }) + .where(eq(decisionProcesses.id, setup.process.id)); + + const caller = await createAuthenticatedCaller(setup.userEmail); + + // Create instance in 'voting' phase + const instance = await testData.createInstanceForProcess({ + caller, + processId: setup.process.id, + name: 'Proposals Test Instance', + }); + + // Update instanceData to be in 'voting' phase + await db + .update(processInstances) + .set({ + instanceData: createSimpleInstanceData('voting'), + currentStateId: 'voting', + status: ProcessStatus.PUBLISHED, + }) + .where(eq(processInstances.id, instance.instance.id)); + + // Create some proposals for this instance + const proposal1 = await testData.createProposal({ + callerEmail: setup.userEmail, + processInstanceId: instance.instance.id, + proposalData: { + title: 'Test Proposal 1', + description: 'First test proposal', + }, + }); + + const proposal2 = await testData.createProposal({ + callerEmail: setup.userEmail, + processInstanceId: instance.instance.id, + proposalData: { + title: 'Test Proposal 2', + description: 'Second test proposal', + }, + }); + + // Set proposal status to 'approved' so the default pipeline will select them + // (The default pipeline filters for status='approved') + await db + .update(proposals) + .set({ status: ProposalStatus.APPROVED }) + .where(eq(proposals.processInstanceId, instance.instance.id)); + + // Insert transition to results phase + await db.insert(decisionProcessTransitions).values({ + processInstanceId: instance.instance.id, + fromStateId: 'voting', + toStateId: 'results', + scheduledDate: createPastDate(1), + }); + + await testData.grantProfileAccess( + instance.profileId, + setup.user.id, + setup.userEmail, + ); + + // Process transitions - should transition to results and process proposals + const result = await processDecisionsTransitions(); + + // Check that THIS instance's transitions didn't fail + const thisInstanceErrors = result.errors.filter( + (e) => e.processInstanceId === instance.instance.id, + ); + expect(thisInstanceErrors).toHaveLength(0); + + // Verify instance advanced to results + const currentPhaseId = await getInstanceCurrentPhaseId( + instance.instance.id, + ); + expect(currentPhaseId).toBe('results'); + + // Verify a result record was created + const [resultRecord] = await db + .select() + .from(decisionProcessResults) + .where( + eq(decisionProcessResults.processInstanceId, instance.instance.id), + ); + + expect(resultRecord).toBeDefined(); + expect(resultRecord!.success).toBe(true); + // Default pipeline selects all proposals + expect(resultRecord!.selectedCount).toBe(2); + + // Verify selections were recorded + const selections = await db + .select() + .from(decisionProcessResultSelections) + .where( + eq(decisionProcessResultSelections.processResultId, resultRecord!.id), + ); + + expect(selections).toHaveLength(2); + const selectedProposalIds = selections.map((s) => s.proposalId); + expect(selectedProposalIds).toContain(proposal1.id); + expect(selectedProposalIds).toContain(proposal2.id); + + // Verify proposals were marked as SELECTED + const updatedProposals = await db + .select() + .from(proposals) + .where(eq(proposals.processInstanceId, instance.instance.id)); + + expect(updatedProposals).toHaveLength(2); + expect( + updatedProposals.every((p) => p.status === ProposalStatus.SELECTED), + ).toBe(true); + }); + + /** + * Tests that results processing errors don't fail the transition. + * The transition should still complete even if results processing fails. + */ + it('should complete transition even if results processing encounters an error', async ({ + task, + onTestFinished, + }) => { + const testData = new TestDecisionsDataManager(task.id, onTestFinished); + + const setup = await testData.createDecisionSetup({ + processName: 'Results Error Test', + instanceCount: 0, + }); + + // Create schema with an invalid selection pipeline that will fail + // Note: We use a plain object here because DecisionSchemaDefinition doesn't + // include selectionPipeline, but the database stores it as JSONB + const schemaWithBadPipeline = { + id: 'test-bad-pipeline-schema', + version: '1.0.0', + name: 'Bad Pipeline Test Process', + phases: [ + { + id: 'submission', + name: 'Submission', + rules: { advancement: { method: 'date' } }, + }, + { + id: 'voting', + name: 'Voting', + rules: { advancement: { method: 'date' } }, + }, + { id: 'results', name: 'Results', rules: {} }, + ], + // Invalid pipeline that should cause an error (not an array of blocks) + selectionPipeline: { + invalid: 'this-will-fail', + }, + }; + + await db + .update(decisionProcesses) + .set({ processSchema: schemaWithBadPipeline }) + .where(eq(decisionProcesses.id, setup.process.id)); + + const caller = await createAuthenticatedCaller(setup.userEmail); + + // Create instance in 'voting' phase with transition to 'results' + const instance = await createInstanceWithDueTransition( + testData, + setup, + caller, + { + name: 'Error Test Instance', + currentPhaseId: 'voting', + fromStateId: 'voting', + toStateId: 'results', + scheduledDate: createPastDate(1), + }, + ); + + // Process transitions + const result = await processDecisionsTransitions(); + + // Check that THIS instance's transition didn't fail + // (results processing error is caught and logged, but transition completes) + const thisInstanceErrors = result.errors.filter( + (e) => e.processInstanceId === instance.instance.id, + ); + expect(thisInstanceErrors).toHaveLength(0); + + // Verify instance still advanced to results despite pipeline error + const currentPhaseId = await getInstanceCurrentPhaseId( + instance.instance.id, + ); + expect(currentPhaseId).toBe('results'); + + // Verify a result record was created (with success=false due to pipeline error) + const [resultRecord] = await db + .select() + .from(decisionProcessResults) + .where( + eq(decisionProcessResults.processInstanceId, instance.instance.id), + ); + + expect(resultRecord).toBeDefined(); + // The result may show success=false if pipeline failed, or success=true with 0 selections + // Either way, the transition completed + expect(resultRecord!.processInstanceId).toBe(instance.instance.id); + }); + + /** + * Tests that results are NOT processed when transitioning to a non-final phase. + */ + it('should not create result record when transitioning to non-final phase', async ({ + task, + onTestFinished, + }) => { + const testData = new TestDecisionsDataManager(task.id, onTestFinished); + + const setup = await testData.createDecisionSetup({ + processName: 'Non-Final Transition Test', + instanceCount: 0, + }); + + await db + .update(decisionProcesses) + .set({ processSchema: createDecisionSchema() }) + .where(eq(decisionProcesses.id, setup.process.id)); + + const caller = await createAuthenticatedCaller(setup.userEmail); + + // Create instance with transition to 'review' (not final phase) + const instance = await createInstanceWithDueTransition( + testData, + setup, + caller, + { + name: 'Non-Final Test Instance', + currentPhaseId: 'submission', + fromStateId: 'submission', + toStateId: 'review', // Not final phase + scheduledDate: createPastDate(1), + }, + ); + + // Process transitions + const result = await processDecisionsTransitions(); + + // Check that THIS instance's transitions didn't fail + const thisInstanceErrors = result.errors.filter( + (e) => e.processInstanceId === instance.instance.id, + ); + expect(thisInstanceErrors).toHaveLength(0); + + // Verify instance advanced to review + const currentPhaseId = await getInstanceCurrentPhaseId( + instance.instance.id, + ); + expect(currentPhaseId).toBe('review'); + + // Verify NO result record was created (not final phase) + const resultRecords = await db + .select() + .from(decisionProcessResults) + .where( + eq(decisionProcessResults.processInstanceId, instance.instance.id), + ); + + expect(resultRecords).toHaveLength(0); + }); + }); +}); diff --git a/services/api/src/test/helpers/TestDecisionsDataManager.ts b/services/api/src/test/helpers/TestDecisionsDataManager.ts index f6bdde6a4..d299129e2 100644 --- a/services/api/src/test/helpers/TestDecisionsDataManager.ts +++ b/services/api/src/test/helpers/TestDecisionsDataManager.ts @@ -1,6 +1,7 @@ +import { createTransitionsForProcess } from '@op/common'; import { db } from '@op/db/client'; -import type { ProcessStatus } from '@op/db/schema'; import { + ProcessStatus, decisionProcesses, organizationUserToAccessRoles, organizationUsers, @@ -340,6 +341,16 @@ export class TestDecisionsDataManager { .update(processInstances) .set({ status }) .where(eq(processInstances.id, profile.processInstance.id)); + + // If publishing, create transitions for the instance + if (status === ProcessStatus.PUBLISHED) { + const fullInstance = await db._query.processInstances.findFirst({ + where: eq(processInstances.id, profile.processInstance.id), + }); + if (fullInstance) { + await createTransitionsForProcess({ processInstance: fullInstance }); + } + } } // Update budget if needed (instanceData may not include budget from template) @@ -402,6 +413,64 @@ export class TestDecisionsDataManager { this.createdProfileIds.push(profileId); } + /** + * Creates a published instance with custom phase dates via the API. + * This uses createInstanceFromTemplate followed by updateDecisionInstance with status: 'published'. + * The API automatically creates transitions for date-based phases when publishing. + * + * Note: createInstanceFromTemplate already grants Admin access to the caller, + * so no additional grantProfileAccess call is needed. + * + * @param caller - The authenticated tRPC caller + * @param processId - The process template ID + * @param name - Instance name + * @param phaseDates - Array of { phaseId, startDate, endDate } for each phase + * @returns The created instance with profile information + */ + async createPublishedInstanceWithDueDates({ + caller, + processId, + name, + phaseDates, + }: { + caller: Awaited< + ReturnType + >; + processId: string; + name: string; + phaseDates: Array<{ + phaseId: string; + startDate?: string; + endDate?: string; + }>; + }): Promise { + this.ensureCleanupRegistered(); + + // Create instance with custom phase dates (created as DRAFT) + // Note: createInstanceFromTemplate automatically grants Admin access to the caller + const profile = await caller.decision.createInstanceFromTemplate({ + templateId: processId, + name: this.generateUniqueName(name), + description: `Test instance ${name}`, + phases: phaseDates, + }); + + const profileId = profile.id; + this.createdProfileIds.push(profileId); + + // Publish the instance - this triggers createTransitionsForProcess + const publishedProfile = await caller.decision.updateDecisionInstance({ + instanceId: profile.processInstance.id, + status: ProcessStatus.PUBLISHED, + }); + + return { + instance: publishedProfile.processInstance, + profileId, + slug: publishedProfile.slug, + }; + } + /** * Creates a member user (non-admin) for an organization with proper setup. * This creates: @@ -541,10 +610,12 @@ export class TestDecisionsDataManager { } /** - * Generates a unique name with UUID first to avoid truncation issues with slug generation + * Generates a unique name with UUID first to survive slug truncation. + * Profile slugs are truncated to ~30 chars, so UUID must be at the start. */ private generateUniqueName(baseName: string): string { - return `${randomUUID()}-${baseName}-${this.testId}`; + // UUID first ensures uniqueness survives slug truncation + return `${randomUUID().substring(0, 8)}-${baseName}`; } /**