Skip to content

Commit ec8ef70

Browse files
author
John Pinto
committed
Fix for bugs in the Conditional Questions functionality:
In the case of a conditional question with answers that removed questions from different sections of a phase, any answers of removed questions were not removed, just hidden. Nor were the removed answers deleted in the database. Changes: - Added code to delete answers in AnswersController and ConditionsHelper in the database. - Added javascript code in answers/edit.js and utils/sectionUpdate.js to delete answers for removed questions. Rspec tests updated in next commit.
1 parent 4bbaa34 commit ec8ef70

File tree

5 files changed

+70
-32
lines changed

5 files changed

+70
-32
lines changed

app/controllers/answers_controller.rb

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -98,14 +98,32 @@ def create_or_update
9898
@section = @plan.sections.find_by(id: @question.section_id)
9999
template = @section.phase.template
100100

101-
remove_list_after = remove_list(@plan)
102-
101+
# Get list of questions to be removed from the plan based on any conditional questions.
102+
questions_remove_list_before_destroying_answers = remove_list(@plan)
103103
all_question_ids = @plan.questions.pluck(:id)
104-
# rubocop pointed out that these variable is not used
105-
# all_answers = @plan.answers
104+
105+
# Destroy all answers for removed questions
106+
questions_remove_list_before_destroying_answers.each do |id|
107+
Answer.where(question_id: id, plan: @plan).each do |a|
108+
Answer.destroy(a.id)
109+
end
110+
end
111+
# Now update @plan after removing answers of questions removed from the plan.
112+
@plan = Plan.includes(
113+
sections: {
114+
questions: %i[
115+
answers
116+
question_format
117+
]
118+
}
119+
).find(p_params[:plan_id])
120+
121+
# Now get list of question ids to remove based on remaining answers.
122+
remove_list_question_ids = remove_list(@plan)
123+
106124
qn_data = {
107-
to_show: all_question_ids - remove_list_after,
108-
to_hide: remove_list_after
125+
to_show: all_question_ids - remove_list_question_ids,
126+
to_hide: remove_list_question_ids
109127
}
110128

111129
section_data = []

app/helpers/conditions_helper.rb

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,14 @@ def answer_remove_list(answer, user = nil)
2727
opts = cond.option_list.map(&:to_i).sort
2828
action = cond.action_type
2929
chosen = answer.question_option_ids.sort
30-
if chosen == opts
30+
31+
# If the chosen (options) include the opts (options list) in the condition,
32+
# then we apply the action.
33+
# Currently, the Template edit through the UI only allows an action to be
34+
# added to a single option at a time,
35+
# so the opts array is always length 0 or 1.
36+
# This if checks that all elements in opts are also in chosen.
37+
if !opts.empty? && !chosen.empty? && opts.intersection(chosen) == opts
3138
if action == 'remove'
3239
rems = cond.remove_data.map(&:to_i)
3340
id_list += rems

app/javascript/src/answers/edit.js

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {
55
} from '../utils/isType';
66
import { Tinymce } from '../utils/tinymce.js';
77
import debounce from '../utils/debounce';
8-
import { updateSectionProgress, getQuestionDiv } from '../utils/sectionUpdate';
8+
import { updateSectionProgress, getQuestionDiv , deleteAllAnswersForQuestion } from '../utils/sectionUpdate';
99
import datePicker from '../utils/datePicker';
1010
import TimeagoFactory from '../utils/timeagoFactory.js.erb';
1111

@@ -23,6 +23,7 @@ $(() => {
2323
updateSectionProgress(section.sec_id, section.no_ans, section.no_qns);
2424
});
2525
data.qn_data.to_hide.forEach((questionid) => {
26+
deleteAllAnswersForQuestion(questionid);
2627
getQuestionDiv(questionid).slideUp();
2728
});
2829
data.qn_data.to_show.forEach((questionid) => {
@@ -100,11 +101,6 @@ $(() => {
100101
const form = target.closest('form');
101102
const id = questionId(target);
102103

103-
console.log('SUBMITTING');
104-
console.log(target);
105-
console.log(form);
106-
console.log(id);
107-
108104
if (debounceMap[id]) {
109105
// Cancels the delated execution of autoSaving
110106
// (e.g. user clicks the button before the delay is met)
Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,38 @@
1+
import { Tinymce } from './tinymce.js';
2+
13
// update details in section progress panel
24
export const updateSectionProgress = (id, numSecAnswers, numSecQuestions) => {
35
const progressDiv = $(`#section-panel-${id}`).find('.section-status');
46
progressDiv.html(`(${numSecAnswers} / ${numSecQuestions})`);
5-
6-
/**
7-
// THIS CODE MAY BE OBSOLETE.
8-
// RETAINING IT TILL SURE.
9-
const heading = progressDiv.closest('.card-heading');
10-
if (numSecQuestions === 0) { // disable section if empty
11-
if (heading.parent().attr('aria-expanded') === 'true') {
12-
heading.parent().trigger('click');
13-
}
14-
heading.addClass('empty-section');
15-
heading.closest('.card').find(`#collapse-${id}`).hide();
16-
heading.closest('.card').find('i.fa-plus, i.fa-minus').removeClass('fa-plus').removeClass('fa-minus');
17-
} else if (heading.hasClass('empty-section')) { // enable section if questions re-added
18-
heading.removeClass('empty-section');
19-
heading.closest('.card').find('i[aria-hidden="true"]').addClass('fa-plus');
20-
heading.closest('.card').find(`#collapse-${id}`).css('display', '');
21-
}
22-
*/
237
};
248

259
// given a question id find the containing div
2610
// used inconditional questions
2711
export const getQuestionDiv = (id) => $(`#answer-form-${id}`).closest('.question-body');
12+
13+
// Clear an answers for a given question id.
14+
export const deleteAllAnswersForQuestion = (questionid) => {
15+
const answerFormDiv = $(`#answer-form-${questionid}`);
16+
const editAnswerForm = $(`#answer-form-${questionid}`).find('.form-answer');
17+
18+
editAnswerForm.find('input:checkbox').prop('checked', false);
19+
editAnswerForm.find('input:radio').prop('checked', false);
20+
editAnswerForm.find('option').prop('selected', false);
21+
editAnswerForm.find('input:text').val('');
22+
23+
// Get the TinyMce editor textarea and rest content to ''
24+
const editorAnswerTextAreaId = `answer-text-${questionid}`;
25+
const tinyMceAnswerEditor = Tinymce.findEditorById(editorAnswerTextAreaId);
26+
if (tinyMceAnswerEditor) {
27+
tinyMceAnswerEditor.setContent('');
28+
}
29+
// Date fields in form are input of type="date"
30+
// The editAnswerForm.find('input:date') throws error, so
31+
// we need an alternate way to reset date.
32+
editAnswerForm.find('#answer_text').each ( (el) => {
33+
if($(el).attr('type') === 'date') {
34+
$(el).val('');
35+
}
36+
37+
});
38+
};

app/models/condition.rb

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,18 @@ class Condition < ApplicationRecord
3434

3535
# Sort order: Number ASC
3636
default_scope { order(number: :asc) }
37-
37+
# rubocop:disable Metrics/AbcSize
3838
def deep_copy(**options)
3939
copy = dup
4040
copy.question_id = options.fetch(:question_id, nil)
41+
# Added to allow options to be passed in for all fields
42+
copy.option_list = options.fetch(:option_list, option_list) if options.key?(:option_list)
43+
copy.remove_data = options.fetch(:remove_data, remove_data) if options.key?(:remove_data)
44+
copy.action_type = options.fetch(:action_type, action_type) if options.key?(:action_type)
45+
copy.webhook_data = options.fetch(:webhook_data, webhook_data) if options.key?(:webhook_data)
4146
# TODO: why call validate false here
4247
copy.save!(validate: false) if options.fetch(:save, false)
4348
copy
4449
end
50+
# rubocop:enable Metrics/AbcSize
4551
end

0 commit comments

Comments
 (0)