Merge pull request #894 from rasmi/conditions-refactor

Conditions refactor

Author: vivtsai

frontend/src/components/stages/condition_editor.ts

@@ -16,25 +16,15 @@ import {
   ComparisonCondition,
   ConditionOperator,
   ComparisonOperator,
+  ConditionTarget,
+  ConditionTargetReference,
   createConditionGroup,
   createComparisonCondition,
   getComparisonOperatorLabel,
-  ConditionTargetReference,
-  SurveyQuestion,
-  SurveyQuestionKind,
-  MultipleChoiceSurveyQuestion,
 } from '@deliberation-lab/utils';
 
 import {styles} from './condition_editor.scss';
 
-export interface ConditionTarget {
-  ref: ConditionTargetReference; // Structured reference
-  label: string;
-  type: 'text' | 'number' | 'boolean' | 'choice';
-  choices?: Array<{id: string; label: string}>;
-  stageName?: string; // Optional stage name for display
-}
-
 /** Reusable condition editor component */
 @customElement('condition-editor')
 export class ConditionEditor extends MobxLitElement {
@@ -446,51 +436,3 @@ export class ConditionEditor extends MobxLitElement {
     this.onConditionChange(this.condition);
   }
 }
-
-/** Helper to convert survey questions to condition targets */
-export function surveyQuestionsToConditionTargets(
-  questions: SurveyQuestion[],
-  stageId: string,
-  stageName?: string,
-): ConditionTarget[] {
-  return questions.map((q) => {
-    let type: ConditionTarget['type'] = 'text';
-    let choices: ConditionTarget['choices'] = undefined;
-
-    switch (q.kind) {
-      case SurveyQuestionKind.TEXT:
-        type = 'text';
-        break;
-      case SurveyQuestionKind.CHECK:
-        type = 'boolean';
-        break;
-      case SurveyQuestionKind.SCALE:
-        type = 'number';
-        break;
-      case SurveyQuestionKind.MULTIPLE_CHOICE:
-        type = 'choice';
-        const mcQuestion = q as MultipleChoiceSurveyQuestion;
-        choices = mcQuestion.options.map((opt) => ({
-          id: opt.id,
-          label: opt.text || `Option ${opt.id}`,
-        }));
-        break;
-    }
-
-    // Create structured reference
-    const ref: ConditionTargetReference = {
-      stageId: stageId,
-      questionId: q.id,
-    };
-
-    const label = q.questionTitle || `Question ${q.id}`;
-
-    return {
-      ref,
-      label,
-      type,
-      choices,
-      stageName,
-    };
-  });
-}

frontend/src/components/stages/survey_editor.ts

@@ -3,28 +3,23 @@ import {CSSResultGroup, html, nothing} from 'lit';
 import {customElement, property} from 'lit/decorators.js';
 import '../stages/survey_editor_menu';
 import '@material/web/checkbox/checkbox.js';
-import './condition_editor';
 import '../../pair-components/textarea_template';
-import {
-  ConditionTarget,
-  surveyQuestionsToConditionTargets,
-} from './condition_editor';
 
 import {core} from '../../core/core';
 import {AuthService} from '../../services/auth.service';
 import {ExperimentEditor} from '../../services/experiment.editor';
-
+import {renderConditionEditor} from '../../shared/condition_editor.utils';
 import {
   CheckSurveyQuestion,
   Condition,
+  getConditionTargetsFromStages,
   MultipleChoiceItem,
   MultipleChoiceSurveyQuestion,
   ScaleSurveyQuestion,
   SurveyPerParticipantStageConfig,
   SurveyStageConfig,
   SurveyQuestion,
   SurveyQuestionKind,
-  StageKind,
   TextSurveyQuestion,
   createMultipleChoiceItem,
 } from '@deliberation-lab/utils';
@@ -193,9 +188,11 @@ export class SurveyEditor extends MobxLitElement {
     `;
   }
 
-  private renderConditionEditor(question: SurveyQuestion, index: number) {
+  private renderQuestionConditionEditor(
+    question: SurveyQuestion,
+    index: number,
+  ) {
     if (!this.stage) return nothing;
-    if (!this.authService.showAlphaFeatures) return nothing;
 
     const onConditionChange = (condition: Condition | undefined) => {
       this.updateQuestion(
@@ -207,53 +204,20 @@ export class SurveyEditor extends MobxLitElement {
       );
     };
 
-    // Get all questions before this one in current stage (can only reference previous questions)
-    const currentStageQuestions = this.stage.questions.slice(0, index);
-    const currentTargets = surveyQuestionsToConditionTargets(
-      currentStageQuestions,
+    // Get condition targets from all preceding stages and questions before this one
+    const targets = getConditionTargetsFromStages(
+      this.experimentEditor.stages,
       this.stage.id,
-      this.stage.name,
+      {includeCurrentStage: true, currentStageQuestionIndex: index},
     );
 
-    // Get questions from all previous stages
-    const allTargets: ConditionTarget[] = [...currentTargets];
-    const currentStageIndex = this.experimentEditor.stages.findIndex(
-      (s) => s.id === this.stage?.id,
-    );
-
-    if (currentStageIndex > 0) {
-      // Add questions from previous stages
-      for (let i = 0; i < currentStageIndex; i++) {
-        const prevStage = this.experimentEditor.stages[i];
-        if (
-          prevStage.kind === StageKind.SURVEY ||
-          prevStage.kind === StageKind.SURVEY_PER_PARTICIPANT
-        ) {
-          const prevSurveyStage = prevStage as
-            | SurveyStageConfig
-            | SurveyPerParticipantStageConfig;
-          const prevTargets = surveyQuestionsToConditionTargets(
-            prevSurveyStage.questions,
-            prevStage.id,
-            prevStage.name,
-          );
-          allTargets.push(...prevTargets);
-        }
-      }
-    }
-
-    if (allTargets.length === 0) {
-      return nothing; // No questions to reference
-    }
-
-    return html`
-      <condition-editor
-        .condition=${question.condition}
-        .targets=${allTargets}
-        .disabled=${!this.experimentEditor.canEditStages}
-        .onConditionChange=${onConditionChange}
-      ></condition-editor>
-    `;
+    return renderConditionEditor({
+      condition: question.condition,
+      targets,
+      showAlphaFeatures: this.authService.showAlphaFeatures,
+      canEdit: this.experimentEditor.canEditStages,
+      onConditionChange,
+    });
   }
 
   private renderQuestionNav(question: SurveyQuestion, index: number) {
@@ -329,7 +293,7 @@ export class SurveyEditor extends MobxLitElement {
           >Make this question required for participants</span
         >
       </label>
-      ${this.renderConditionEditor(question, index)}
+      ${this.renderQuestionConditionEditor(question, index)}
     `;
   }
 
@@ -406,7 +370,7 @@ export class SurveyEditor extends MobxLitElement {
       >
         Add multiple choice item
       </pr-button>
-      ${this.renderConditionEditor(question, index)}
+      ${this.renderQuestionConditionEditor(question, index)}
     `;
   }
 
@@ -588,7 +552,7 @@ export class SurveyEditor extends MobxLitElement {
           Optional: Display the scale as a slider instead of radio buttons
         </span>
       </label>
-      ${this.renderConditionEditor(question, index)}
+      ${this.renderQuestionConditionEditor(question, index)}
     `;
   }
 
@@ -640,7 +604,7 @@ export class SurveyEditor extends MobxLitElement {
           />
         </div>
       </div>
-      ${this.renderConditionEditor(question, index)}
+      ${this.renderQuestionConditionEditor(question, index)}
     `;
   }
 }

frontend/src/shared/condition_editor.utils.ts

@@ -0,0 +1,34 @@
+import {html, nothing} from 'lit';
+import {Condition, ConditionTarget} from '@deliberation-lab/utils';
+
+// Ensure condition-editor component is registered
+import '../components/stages/condition_editor';
+
+export interface RenderConditionEditorOptions {
+  condition: Condition | undefined;
+  targets: ConditionTarget[];
+  showAlphaFeatures: boolean | undefined;
+  canEdit: boolean;
+  onConditionChange: (condition: Condition | undefined) => void;
+}
+
+/**
+ * Render a condition editor with standard checks.
+ * Returns nothing if alpha features are disabled or there are no valid targets.
+ */
+export function renderConditionEditor(options: RenderConditionEditorOptions) {
+  const {condition, targets, showAlphaFeatures, canEdit, onConditionChange} =
+    options;
+
+  if (!showAlphaFeatures) return nothing; // Treats undefined as false
+  if (targets.length === 0) return nothing;
+
+  return html`
+    <condition-editor
+      .condition=${condition}
+      .targets=${targets}
+      .disabled=${!canEdit}
+      .onConditionChange=${onConditionChange}
+    ></condition-editor>
+  `;
+}

utils/src/index.ts

@@ -145,6 +145,7 @@ export * from './stages/transfer_stage.validation';
 export * from './utils/algebraic.utils';
 export * from './utils/cache.utils';
 export * from './utils/condition';
+export * from './utils/condition.utils';
 export * from './utils/condition.validation';
 export * from './utils/object.utils';
 export * from './utils/random.utils';

utils/src/stages/survey_stage.ts

@@ -1,11 +1,11 @@
 import {generateId} from '../shared';
 import {
   Condition,
-  ConditionTargetReference,
   evaluateCondition,
   extractConditionDependencies,
   extractMultipleConditionDependencies,
 } from '../utils/condition';
+import {getConditionDependencyValuesWithCurrentStage} from '../utils/condition.utils';
 import {
   BaseStageConfig,
   BaseStageParticipantAnswer,
@@ -326,7 +326,7 @@ export function getVisibleSurveyQuestions(
   );
 
   // Get the actual values for all condition targets
-  const targetValues = getConditionDependencyValues(
+  const targetValues = getConditionDependencyValuesWithCurrentStage(
     allDependencies,
     currentStageId,
     currentStageAnswers,
@@ -358,7 +358,7 @@ export function isQuestionVisible(
   const dependencies = extractConditionDependencies(question.condition);
 
   // Get the actual values for this question's condition targets
-  const targetValues = getConditionDependencyValues(
+  const targetValues = getConditionDependencyValuesWithCurrentStage(
     dependencies,
     currentStageId,
     currentStageAnswers,
@@ -369,65 +369,8 @@ export function isQuestionVisible(
   return evaluateCondition(question.condition, targetValues);
 }
 
-/** Get the actual answer values for the specified dependencies. */
-function getConditionDependencyValues(
-  dependencies: ConditionTargetReference[],
-  currentStageId: string,
-  currentStageAnswers: Record<string, SurveyAnswer>,
-  allStageAnswers?: Record<string, StageParticipantAnswer>,
-  targetParticipantId?: string, // For survey-per-participant: which participant is being evaluated
-): Record<string, unknown> {
-  const values: Record<string, unknown> = {};
-
-  for (const targetRef of dependencies) {
-    // Build the key for this dependency
-    // Using :: as separator since it's unlikely to appear in IDs
-    const dataKey = `${targetRef.stageId}::${targetRef.questionId}`;
-
-    if (targetRef.stageId === currentStageId) {
-      // Reference to current stage
-      const answer = currentStageAnswers[targetRef.questionId];
-      if (answer) {
-        values[dataKey] = extractAnswerValue(answer);
-      }
-    } else if (allStageAnswers && allStageAnswers[targetRef.stageId]) {
-      // Reference to another stage
-      const stageAnswer = allStageAnswers[targetRef.stageId];
-      if (stageAnswer && 'answerMap' in stageAnswer) {
-        if (stageAnswer.kind === StageKind.SURVEY) {
-          // Regular survey stage
-          const surveyAnswer = stageAnswer as SurveyStageParticipantAnswer;
-          const answer = surveyAnswer.answerMap[targetRef.questionId];
-          if (answer) {
-            values[dataKey] = extractAnswerValue(answer);
-          }
-        } else if (stageAnswer.kind === StageKind.SURVEY_PER_PARTICIPANT) {
-          // Survey-per-participant: get answer about the target participant
-          const surveyAnswer =
-            stageAnswer as SurveyPerParticipantStageParticipantAnswer;
-
-          // For cross-stage references, we need targetParticipantId to know which answer to use
-          if (
-            targetParticipantId &&
-            surveyAnswer.answerMap[targetRef.questionId]
-          ) {
-            const participantAnswers =
-              surveyAnswer.answerMap[targetRef.questionId];
-            if (participantAnswers && participantAnswers[targetParticipantId]) {
-              const answer = participantAnswers[targetParticipantId];
-              values[dataKey] = extractAnswerValue(answer);
-            }
-          }
-        }
-      }
-    }
-  }
-
-  return values;
-}
-
 /** Extract the value from a survey answer */
-function extractAnswerValue(
+export function extractAnswerValue(
   answer: SurveyAnswer,
 ): string | number | boolean | undefined {
   switch (answer.kind) {