apps/web/app/(app)/(survey-editor)/environments/[environmentId]/surveys/[surveyId]/edit/components/LogicEditor.tsx (2)

38-65: Enhance type safety and simplify the implementation.

The useMemo implementation is correct, but could benefit from some improvements:

Consider applying these enhancements:

+ type FallbackOption = {
+   icon?: ReactElement;
+   label: string;
+   value: string;
+ };

  const fallbackOptions = useMemo(() => {
-   let options: {
-     icon?: ReactElement;
-     label: string;
-     value: string;
-   }[] = [];
+   const options: FallbackOption[] = [];

    localSurvey.questions.forEach((ques) => {
-     if (ques.id === question.id) return null;
+     if (ques.id === question.id) return;
      options.push({
        icon: QUESTIONS_ICON_MAP[ques.type],
        label: getLocalizedValue(ques.headline, "default"),
        value: ques.id,
      });
    });

---

Line range hint 37-115: Consider adding validation and error handling.

The implementation could benefit from additional safeguards:

1. Add null checks for localSurvey.questions and localSurvey.endings arrays
2. Validate that the logicFallback value exists in the available options before applying it
3. Consider adding error boundaries to handle potential rendering failures

Would you like me to provide a code example implementing these safeguards?

apps/web/app/(app)/(survey-editor)/environments/[environmentId]/surveys/[surveyId]/edit/components/ConditionalLogic.tsx (1)

Line range hint 1-190: Consider extracting logic operations to a custom hook.

The component currently handles multiple logic operations (add, remove, move, duplicate). To improve maintainability and reusability, consider:

1. Extracting these operations into a custom hook (e.g., useSurveyLogic)
2. Moving the logic transformation utilities to a separate service
3. Creating a dedicated LogicOperations context if these operations are used across multiple components

Example structure:

// hooks/useSurveyLogic.ts
export function useSurveyLogic(question, questionIdx, updateQuestion) {
  const addLogic = useCallback(() => {
    // Current addLogic implementation
  }, [question, questionIdx, updateQuestion]);

  const removeLogic = useCallback((logicItemIdx) => {
    // Current handleRemoveLogic implementation
  }, [question, questionIdx, updateQuestion]);

  // ... other operations

  return { addLogic, removeLogic, moveLogic, duplicateLogic };
}

packages/types/surveys/validation.ts (2)

229-229: Consider documenting the fallback behavior.

The comment "Handle default behavior" could be more explicit about the precedence rules between jump actions, fallback logic, and default next question behavior.

-      // Handle default behavior: move to the next question if no jump actions or fallback logic is defined
+      // Precedence: 1. Jump actions (if conditions met)
+      //            2. Fallback logic (if specified)
+      //            3. Default: move to next question

---

217-229: Consider performance optimization for deep fallback chains.

While the implementation is correct, deep fallback chains could impact performance during validation. Consider:

1. Adding a maximum depth limit for fallback chains
2. Caching visited paths to optimize repeated validations
3. Adding telemetry to monitor chain depths in production

Example implementation:

const MAX_FALLBACK_DEPTH = 10;
const visitedPaths = new Map<string, Set<string>>();

const checkForCyclicLogic = (questionId: TSurveyQuestionId, depth = 0): boolean => {
  if (depth > MAX_FALLBACK_DEPTH) {
    console.warn(`Maximum fallback depth exceeded for question ${questionId}`);
    return false;
  }
  
  // Cache hit: return early if we've seen this path before
  const pathKey = Array.from(recStack).filter(([_, v]) => v).map(([k]) => k).join(',');
  if (visitedPaths.has(pathKey)) {
    return visitedPaths.get(pathKey)!.has(questionId);
  }
  
  // Rest of the existing logic...
}

packages/surveys/src/components/general/Survey.tsx (2)

Line range hint 258-275: Add validation for logicFallback value

The current implementation should validate the logicFallback value to ensure it references a valid question or ending ID.

Add validation to prevent navigation to invalid questions:

    // Use logicFallback if no jump target was set
    if (!firstJumpTarget && currQuesTemp.logicFallback) {
+     // Validate that logicFallback points to a valid question or ending
+     const isValidQuestion = questions.some(q => q.id === currQuesTemp.logicFallback);
+     const isValidEnding = survey.endings.some(e => e.id === currQuesTemp.logicFallback);
+     if (!isValidQuestion && !isValidEnding) {
+       console.warn(`Invalid logicFallback target: ${currQuesTemp.logicFallback}`);
+       firstJumpTarget = questions[currentQuestionIndex + 1]?.id || firstEndingId;
+     } else {
        firstJumpTarget = currQuesTemp.logicFallback;
+     }
    }

---

Line range hint 245-260: Enhance error handling in logic evaluation

The current error handling is minimal and could be improved to provide better debugging information and handle edge cases more gracefully.

Consider enhancing the error handling:

    if (questionId === "start")
      return { nextQuestionId: questions[0]?.id || firstEndingId, calculatedVariables: {} };

-   if (!currQuesTemp) throw new Error("Question not found");
+   if (!currQuesTemp) {
+     console.error(`Failed to find question with ID: ${questionId}`);
+     throw new Error(`Question not found: ${questionId}`);
+   }

    let firstJumpTarget: string | undefined;
    const allRequiredQuestionIds: string[] = [];

    let calculationResults = { ...currentVariables };
    const localResponseData = { ...responseData, ...data };

+   try {
      if (currQuesTemp.logic && currQuesTemp.logic.length > 0) {
        for (const logic of currQuesTemp.logic) {
          if (
            evaluateLogic(
              localSurvey,
              localResponseData,
              calculationResults,
              logic.conditions,
              selectedLanguage
            )
          ) {
            // ... existing logic ...
          }
        }
      }
+   } catch (error) {
+     console.error(`Logic evaluation failed for question ${questionId}:`, error);
+     // Fallback to next question in sequence
+     return {
+       nextQuestionId: questions[currentQuestionIndex + 1]?.id || firstEndingId,
+       calculatedVariables: calculationResults
+     };
+   }

apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/lib/surveySummary.ts (3)

122-125: Add documentation for the fallback logic behavior.

While the implementation is correct, adding a comment explaining when and why the fallback logic is triggered would improve code maintainability.

-  // If no jump target was set, check for a fallback logic
+  // If no conditions in the logic array evaluate to true or no jump target is set,
+  // use the fallback logic to determine the next question. This ensures a default
+  // path when no specific logic conditions are met.
   if (!firstJumpTarget && currQuesTemp.logicFallback) {

---

Line range hint 1-1000: Consider adding more specific error handling.

The error handling is good but could be enhanced by adding specific error types for different scenarios. This would help in better error reporting and debugging.

Consider creating and handling specific error types for:

1. Invalid filter criteria
2. Cache-related errors
3. Data transformation errors

Example implementation:

class InvalidFilterError extends Error {
  constructor(message: string) {
    super(message);
    this.name = 'InvalidFilterError';
  }
}

class CacheError extends Error {
  constructor(message: string) {
    super(message);
    this.name = 'CacheError';
  }
}

---

Line range hint 1-1000: Consider splitting the file into smaller modules.

The file has grown quite large and handles multiple responsibilities. Consider breaking it down into smaller, more focused modules for better maintainability.

Suggested structure:

1. Create a separate module for each question type handler
2. Move utility functions to a separate file
3. Create a dedicated types file
4. Consider using the Strategy pattern for question type handling

Example reorganization:

// questionHandlers/
//   - multipleChoice.ts
//   - rating.ts
//   - openText.ts
//   etc.

// utils/
//   - validation.ts
//   - transformation.ts
//   - cache.ts

// types/
//   - questions.ts
//   - responses.ts
//   - summary.ts

apps/web/app/(app)/(survey-editor)/environments/[environmentId]/surveys/[surveyId]/edit/lib/utils.tsx (3)

1063-1065: Consider improving readability with destructuring

The condition check could be more readable by destructuring the question properties.

-    (question) =>
-      question.logicFallback === questionId ||
-      (question.id !== questionId && question.logic?.some(isUsedInLogicRule))
+    ({ id, logicFallback, logic }) =>
+      logicFallback === questionId ||
+      (id !== questionId && logic?.some(isUsedInLogicRule))

---

1151-1163: Add JSDoc documentation for consistency

The new function follows the pattern of other utility functions but lacks documentation. Consider adding JSDoc for consistency with other utility functions.

+/**
+ * Finds the index of the first question that uses the specified ending card in its logic
+ * @param survey - The survey to search in
+ * @param endingCardId - The ID of the ending card to search for
+ * @returns The index of the first question using the ending card, or -1 if not found
+ */
 export const findEndingCardUsedInLogic = (survey: TSurvey, endingCardId: string): number => {

---

1160-1162: Consider improving readability with destructuring (similar to previous comment)

For consistency with the suggested improvement above, consider destructuring here as well.

-    (question) => question.logicFallback === endingCardId || question.logic?.some(isUsedInLogicRule)
+    ({ logicFallback, logic }) => logicFallback === endingCardId || logic?.some(isUsedInLogicRule)

packages/lib/messages/de-DE.json (1)

Line range hint 1-2000: Inconsistent use of formal/informal address throughout the file

There are inconsistencies in the use of formal ("Sie") and informal ("Du") address forms throughout the translations. For example, some sections use the informal "Du" while others use the formal "Sie", which could create an inconsistent tone of voice.

Consider standardizing the address form throughout the translations to maintain consistency. The informal "Du" form seems to be more prevalent and aligns better with modern software UI conventions.

packages/types/surveys/types.ts (2)

2032-2076: Consider returning an empty array from validateLogicFallback for consistency.

To simplify the handling of validation issues, modify validateLogicFallback to always return a z.ZodIssue[], returning an empty array when there are no issues. This avoids the need to check for undefined when combining validation results.

Apply this diff to implement the change:

-const validateLogicFallback = (survey: TSurvey, questionIdx: number): z.ZodIssue[] | undefined => {
+const validateLogicFallback = (survey: TSurvey, questionIdx: number): z.ZodIssue[] => {
  const question = survey.questions[questionIdx];

-  if (!question.logicFallback) return;
+  if (!question.logicFallback) return [];

  // ... rest of the code ...
}

Update the validateLogic function accordingly:

 const validateLogic = (survey: TSurvey, questionIndex: number, logic: TSurveyLogic[]): z.ZodIssue[] => {
   const logicFallbackIssue = validateLogicFallback(survey, questionIndex);

-  return [...logicIssues.flat(), ...(logicFallbackIssue ? logicFallbackIssue : [])];
+  return [...logicIssues.flat(), ...logicFallbackIssue];
 };

---

2055-2066: Simplify collection of possibleFallbackIds.

You can improve readability by using array methods like filter and map to collect possibleFallbackIds.

Apply this diff to refactor the code:

-  const possibleFallbackIds: string[] = [];
-
-  survey.questions.forEach((q, idx) => {
-    if (idx !== questionIdx) {
-      possibleFallbackIds.push(q.id);
-    }
-  });
-
-  survey.endings.forEach((e) => {
-    possibleFallbackIds.push(e.id);
-  });
+  const possibleFallbackIds = survey.questions
+    .filter((_, idx) => idx !== questionIdx)
+    .map((q) => q.id)
+    .concat(survey.endings.map((e) => e.id));

Configuration used: CodeRabbit UI
Review profile: CHILL

- const isLast = logicCopy.length === 1;
+ const isLast = Array.isArray(logicCopy) && logicCopy.length === 1;

#!/bin/bash
# Description: Check for questions with both logic jumps and fallback logic
# This helps us understand the complexity of logic chains

# Search for questions with both logic and logicFallback
ast-grep --pattern 'const question = {
  $$$
  logic: $_,
  $$$
  logicFallback: $_,
  $$$
}'

#!/bin/bash
# Let's try a different approach to analyze the cyclic logic detection implementation

# First, let's see the full implementation of the cyclic detection function
rg -A 30 "function.*findQuestionsWithCyclicLogic" 

# Then, let's look for any test cases that verify this behavior
rg -l "logicFallback.*test|test.*logicFallback"

# Also search for any interfaces or types defining the question structure
ast-grep --pattern 'interface $_ {
  $$$
  logicFallback?: $_
  $$$
}'

#!/bin/bash
# Let's try a simpler approach focusing on the validation.ts file

# View the complete validation.ts file to understand the full context
cat packages/types/surveys/validation.ts

# Also check for any test files in the same directory
fd "validation.test" packages/types/surveys/

    // Use logicFallback if no jump target was set
+   // This provides a default navigation path when no logic conditions are met
    if (!firstJumpTarget && currQuesTemp.logicFallback) {
      firstJumpTarget = currQuesTemp.logicFallback;
    }

#!/bin/bash
# Description: Verify logicFallback implementation and usage

# Check for logicFallback property definition in survey types
ast-grep --pattern 'interface TSurveyQuestion {
  $$$
  logicFallback?: $_
  $$$
}'

# Check for logicFallback assignments in survey configuration
rg -A 3 'logicFallback.*=' 

# Check for potential edge cases in logic evaluation
rg -A 5 'evaluateLogic.*\(' 

#!/bin/bash
# Description: Find potential missing translations by looking for English text in the German translation file
# Look for common English words that shouldn't appear in German translations
rg -i '\b(the|and|or|with|in|on|at|to|for|is|are|was|were)\b' packages/lib/messages/de-DE.json

# Look for potential untranslated UI elements (words that are typically translated)
rg -i '\b(button|click|select|choose|settings|options|menu|file|edit|view|help)\b' packages/lib/messages/de-DE.json

apps/web/app/(app)/(survey-editor)/environments/[environmentId]/surveys/[surveyId]/edit/components/LogicEditor.tsx (2)

38-65: Enhance the fallback options implementation.

A few suggestions to improve the code:

+ interface FallbackOption {
+   icon?: ReactElement;
+   label: string;
+   value: string;
+ }

  const fallbackOptions = useMemo(() => {
-   let options: {
-     icon?: ReactElement;
-     label: string;
-     value: string;
-   }[] = [];
+   const availableOptions: FallbackOption[] = [];

    localSurvey.questions.forEach((ques) => {
-     if (ques.id === question.id) return null;
+     if (ques.id === question.id) return;
      options.push({
        icon: QUESTIONS_ICON_MAP[ques.type],
        label: getLocalizedValue(ques.headline, "default"),
        value: ques.id,
      });
    });

---

91-115: Extract string literals into constants.

Consider extracting the repeated string literal into a constant for better maintainability:

+ const DEFAULT_SELECTION_VALUE = "defaultSelection";

  <Select
    autoComplete={true}
-   defaultValue={question.logicFallback || "defaultSelection"}
+   defaultValue={question.logicFallback || DEFAULT_SELECTION_VALUE}
    onValueChange={(val) => {
      updateQuestion(questionIdx, {
-       logicFallback: val === "defaultSelection" ? undefined : val,
+       logicFallback: val === DEFAULT_SELECTION_VALUE ? undefined : val,
      });
    }}>
    ...
    <SelectContent>
      <SelectItem 
        key="fallback_default_selection" 
-       value={"defaultSelection"}
+       value={DEFAULT_SELECTION_VALUE}
      >

apps/web/app/(app)/(survey-editor)/environments/[environmentId]/surveys/[surveyId]/edit/components/EditEndingCard.tsx (1)

104-104: Consider enhancing the error message for better clarity

The current error message uses array index + 1 to show the question number. This might be confusing if questions can be reordered or if some questions are hidden/inactive.

Consider modifying the error message to include more context:

-toast.error(t("environments.surveys.edit.ending_card_used_in_logic", { questionIndex: quesIdx + 1 }));
+toast.error(t("environments.surveys.edit.ending_card_used_in_logic", {
+  questionIndex: quesIdx + 1,
+  questionTitle: localSurvey.questions[quesIdx].headline[selectedLanguageCode] || `Question ${quesIdx + 1}`
+}));

You'll also need to update the translation key to include the question title in the message.

packages/types/surveys/types.ts (1)

2037-2045: Optimize the condition check

The condition can be simplified using short-circuit evaluation.

-  if (!question.logic?.length && question.logicFallback) {
+  if (!question.logic?.length) {

This works because we already check for logicFallback existence at line 2035.

Configuration used: CodeRabbit UI
Review profile: CHILL

#!/bin/bash
# Description: Verify the integration of logicFallback with survey logic evaluation

# Check for logicFallback usage in survey logic evaluation
echo "Checking for logicFallback usage in survey logic evaluation..."
rg -l "logicFallback" --type ts

# Check for potential validation implementations
echo "Checking for validation implementations..."
ast-grep --pattern 'function $_(logicFallback: $_) {
  $$$
}'

#!/bin/bash
# Let's examine the implementation details in key files

# Check the types definition
echo "=== Checking types definition ==="
cat packages/types/surveys/types.ts | grep -A 10 "logicFallback"

# Check validation implementation
echo -e "\n=== Checking validation implementation ==="
cat packages/types/surveys/validation.ts | grep -A 10 "logicFallback"

# Check survey logic evaluation
echo -e "\n=== Checking survey logic evaluation ==="
cat packages/surveys/src/components/general/Survey.tsx | grep -A 10 "logicFallback"

# Check utils implementation
echo -e "\n=== Checking utils implementation ==="
cat "apps/web/app/(app)/(survey-editor)/environments/[environmentId]/surveys/[surveyId]/edit/lib/utils.tsx" | grep -A 10 "logicFallback"

# Check conditional logic implementation
echo -e "\n=== Checking conditional logic implementation ==="
cat "apps/web/app/(app)/(survey-editor)/environments/[environmentId]/surveys/[surveyId]/edit/components/ConditionalLogic.tsx" | grep -A 10 "logicFallback"

#!/bin/bash
# Description: Check for potential edge cases in the codebase
# Test: Search for existing logicFallback usage patterns

# Look for any existing logicFallback implementations
rg -l "logicFallback"

# Look for any logic-related test files
fd -e test.ts -e spec.ts | rg -i "logic"

#!/bin/bash
# Let's analyze the implementation and test coverage

# Check the validation implementation
rg -A 10 "logicFallback" packages/types/surveys/validation.ts

# Look for test cases in the codebase
rg -A 10 "validateLogicFallback" --type ts

# Check the actual usage in components to understand edge cases
rg -A 5 "logicFallback" packages/surveys/src/components/general/Survey.tsx

# Check logic editor implementation for potential edge cases
rg -A 5 "logicFallback" 'apps/web/app/(app)/(survey-editor)/environments/[environmentId]/surveys/[surveyId]/edit/components/LogicEditor.tsx'

#!/bin/bash
# Let's check for cyclic logic validation and other edge cases

# Check cyclic logic validation implementation
ast-grep --pattern 'function checkForCyclicLogic($_) {
  $$$
}'

# Look for any test cases involving cyclic logic
rg -B 5 -A 10 "cyclic.*logic" --type ts -i

# Check the schema definition to understand the complete logic structure
rg -A 10 "logicFallback.*z\." --type ts

apps/web/app/(app)/(survey-editor)/environments/[environmentId]/surveys/[surveyId]/edit/components/SurveyMenuBar.tsx (1)

347-347: Consider accessibility improvements for the tooltip text

The tooltip text is correctly using the pre-translated cautionText variable. However, to improve accessibility and user experience:

1. Consider adding an aria-label to the tooltip trigger for screen readers
2. The text size (text-xs) might be too small for comfortable reading

-                  <p className="py-2 text-center text-xs text-slate-500 dark:text-slate-400">{cautionText}</p>
+                  <p className="py-2 text-center text-sm text-slate-500 dark:text-slate-400" role="tooltip">{cautionText}</p>

Additionally, consider adding an aria-label to the trigger:

-                  <AlertTriangleIcon className="h-5 w-5 text-amber-400" />
+                  <AlertTriangleIcon className="h-5 w-5 text-amber-400" aria-label="Warning about survey changes" />

Configuration used: CodeRabbit UI
Review profile: CHILL