Fix Broken Conditionals When Cloning Survey Question Groups

[Abishekcs]

closes #2418

What this PR does

Fixes an issue where conditionals in cloned question groups pointed to non-existent questions from the original group by updating them to reference corresponding cloned questions and adding validations to ensure conditionals are valid.

Cause of the Bug:

The root cause of the issue lies in the cloning process. When a question group is cloned:

• All questions within the group are duplicated, generating new records with new IDs.
• Conditionals are copied directly from the original questions without updating the referenced question IDs to match the new cloned group.
• As a result, conditionals in the cloned group refer to question IDs from the original group, causing broken or invalid behavior in initConditionals() in SurveyAdmin.js the line of code which lead to the error is mentioned below.

 return $row.find('select').val(`${question_id}`).trigger('change'); 

Changes Implemented

1. Conditional Mapping During Cloning:

• When a question group is cloned, a mapping is created between the original questions and their cloned counterparts.
• Conditionals in the cloned questions are updated to refer to the corresponding cloned question IDs instead of the original IDs.

2. Validation for Conditionals:

• Added validation to ensure all conditionals within a question group point to questions that exist within the same group.
• This prevents broken or invalid conditionals from being saved.

3. Fallback Handling:

• If an original question referenced in a conditional cannot be matched to a cloned question, the conditional is cleared to prevent invalid references.

Test Description

This test verifies that conditional question groups are correctly cloned, including their conditional logic.

Setup:

• Creates a new question group with a conditional follow-up question.
• Configures conditional logic to show the follow-up question only when a specific answer is selected for the first question.

Clone Operation:

• Clones the created question group using the "Clone" action.

Assertions:

• Ensures the cloned question group exists with the correct number of questions.
• Confirms the conditional logic in the cloned group correctly points to the corresponding cloned questions and values.

This ensures that the cloning functionality preserves all related conditional configurations.

Screenshots

Before:

Before.mp4

After:

After.mp4

Open questions and concerns

• When a question group is deleted, the questions belonging to that group still exist in the database. Shouldn't these questions also be deleted ?

a.1.mp4

• When a question used by a conditional question is deleted, the conditionals column of the conditional question is not updated. It still references the deleted question instead of being set to NULL.

b.1.mp4

[ragesoss]

Nice!

I think it would be good to extend the survey_admin_spec so that it includes this behavior (in a way that would break the test if we undid the controller changes).

I think both of the additional concerns you identify are valid, although I don't think they've caused any problems in practice.

[Abishekcs]

I think it would be good to extend the survey_admin_spec so that it includes this behavior (in a way that would break the test if we undid the controller changes).

Great! I will write the test code for it too.😄

I think both of the additional concerns you identify are valid, although I don't think they've caused any problems in practice.

Okay.

[Abishekcs]

@ragesoss,

I’ve extended the test for survey_admin_spec.

While writing the test, I noticed three "FIXME" tests that were commented out because they were failing. Interestingly, the test I was working on for the current PR was also failing for the same reason as at least one of those "FIXME" tests.

The issue was due to not using optional chaining (?.) when accessing the content property of a potentially non-existent CSRF token object. This caused a JavaScript error:
Cannot read property 'content' of undefined.

The error would occur when the token wasn’t immediately available during test execution, leading to failures.

The problematic code is in SurveyAdmin.js (line 185):

getQuestion(id) {
  const that = this; // Storing the correct "this" reference for use inside the promise
  return fetch(`/surveys/question_group_question/${id}`, {
    method: 'GET',
    credentials: 'include',
    headers: {
      'Content-Type': 'application/json',
      'X-CSRF-Token': SurveyAdmin.$csrf_token.content // Issue: No optional chaining used here
    },
  })
  .then(response => response.json())
  .then((data) => {
    SurveyAdmin.handleConditionalQuestionSelect.call(that, data);
  });
}

The failure occurred because optional chaining (?.) was not used in SurveyAdmin.$csrf_token.content.

Screen recording (For test case):

2024-12-19-14-43-13_QTFQkBHD.mp4

Capybara Screenshots (Just adding for reference in case required):

[Abishekcs]

Also, I have tested one of the three FIXME tests, and it is now passing. I have not tried the other two FIXME tests yet. If those also pass, I will open a separate PR for those three FIXME test.

Update: Below Screenshot is the one FIXME which was passing (rest yet to check) 😅: