Skip to content

Commit

Permalink
Fix for bugs in the Conditional Questions functionality:
Browse files Browse the repository at this point in the history
     - In case of conditional question with checkbox answers the removed
       questions were not being removed from view, nor was the answer to
    these questions (which persisted in the db.

    Changes:
    - Fixed the broken functionality in the method remove_answers_list in
      app/helpers/conditions_helper.rb.
    - Removed and destroyed the answers of the removed questions.
    - Added more tests for Conditional Questions functionality.
  • Loading branch information
John Pinto committed Sep 20, 2024
1 parent 56759df commit 3c5f5bd
Show file tree
Hide file tree
Showing 11 changed files with 1,005 additions and 5 deletions.
9 changes: 8 additions & 1 deletion app/controllers/answers_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,15 @@ def create_or_update
template = @section.phase.template

remove_list_after = remove_list(@plan)

all_question_ids = @plan.questions.pluck(:id)

# Destroy all answers for removed questions
remove_list_after.each do |id|
Answer.where(question_id: id, plan: @plan).each do |a|
Answer.destroy(a.id)
end
end

# rubocop pointed out that these variable is not used
# all_answers = @plan.answers
qn_data = {
Expand Down
2 changes: 1 addition & 1 deletion app/helpers/conditions_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def answer_remove_list(answer, user = nil)
opts = cond.option_list.map(&:to_i).sort
action = cond.action_type
chosen = answer.question_option_ids.sort
if chosen == opts
if !opts.empty? && !chosen.empty? && !(chosen & opts).empty?
if action == 'remove'
rems = cond.remove_data.map(&:to_i)
id_list += rems
Expand Down
4 changes: 3 additions & 1 deletion app/javascript/src/answers/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
} from '../utils/isType';
import { Tinymce } from '../utils/tinymce.js';
import debounce from '../utils/debounce';
import { updateSectionProgress, getQuestionDiv } from '../utils/sectionUpdate';
import { updateSectionProgress, getQuestionDiv , deleteAllAnswersForQuestion } from '../utils/sectionUpdate';
import datePicker from '../utils/datePicker';
import TimeagoFactory from '../utils/timeagoFactory.js.erb';

Expand All @@ -23,7 +23,9 @@ $(() => {
updateSectionProgress(section.sec_id, section.no_ans, section.no_qns);
});
data.qn_data.to_hide.forEach((questionid) => {
deleteAllAnswersForQuestion(questionid);
getQuestionDiv(questionid).slideUp();

});
data.qn_data.to_show.forEach((questionid) => {
getQuestionDiv(questionid).slideDown();
Expand Down
29 changes: 29 additions & 0 deletions app/javascript/src/utils/sectionUpdate.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { Tinymce } from '../utils/tinymce.js';

// update details in section progress panel
export const updateSectionProgress = (id, numSecAnswers, numSecQuestions) => {
const progressDiv = $(`#section-panel-${id}`).find('.section-status');
Expand Down Expand Up @@ -25,3 +27,30 @@ export const updateSectionProgress = (id, numSecAnswers, numSecQuestions) => {
// given a question id find the containing div
// used inconditional questions
export const getQuestionDiv = (id) => $(`#answer-form-${id}`).closest('.question-body');

// Clear an answers for a given question id.
export const deleteAllAnswersForQuestion = (questionid) => {
const answerFormDiv = $(`#answer-form-${questionid}`);
const editAnswerForm = $(`#answer-form-${questionid}`).find('.form-answer');

editAnswerForm.find('input:checkbox').prop('checked', false);
editAnswerForm.find('input:radio').prop('checked', false);
editAnswerForm.find('option').prop('selected', false);
editAnswerForm.find('input:text').text('');

// Get the TinyMce editor textarea and rest content to ''
const editorAnswerTextAreaId = `answer-text-${questionid}`;
const tinyMceAnswerEditor = Tinymce.findEditorById(editorAnswerTextAreaId);
if (tinyMceAnswerEditor) {
tinyMceAnswerEditor.setContent('');
}
// Date fields in form are input of type="date"
// The editAnswerForm.find('input:date') throws error, so
// we need an alternate way to reset date.
editAnswerForm.find('#answer_text').each ( (el) => {
if($(el).attr('type') === 'date') {
$(el).val('');
}

});
};
8 changes: 7 additions & 1 deletion app/models/condition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,18 @@ class Condition < ApplicationRecord

# Sort order: Number ASC
default_scope { order(number: :asc) }

# rubocop:disable Metrics/AbcSize
def deep_copy(**options)
copy = dup
copy.question_id = options.fetch(:question_id, nil)
# Added to allow options to be passed in for all fields
copy.option_list = options.fetch(:option_list, option_list) if options.key?(:option_list)
copy.remove_data = options.fetch(:remove_data, remove_data) if options.key?(:remove_data)
copy.action_type = options.fetch(:action_type, action_type) if options.key?(:action_type)
copy.webhook_data = options.fetch(:webhook_data, webhook_data) if options.key?(:webhook_data)
# TODO: why call validate false here
copy.save!(validate: false) if options.fetch(:save, false)
copy
end
# rubocop:enable Metrics/AbcSize
end
513 changes: 513 additions & 0 deletions spec/controllers/answers_controller_with_conditional_questions_spec.rb

Large diffs are not rendered by default.

6 changes: 6 additions & 0 deletions spec/factories/answers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,11 @@
plan
user
question
trait :question_options do
question_options { [create(:question_option)] }
end
trait :lock_version do
lock_version { 0 }
end
end
end
15 changes: 15 additions & 0 deletions spec/factories/conditions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,20 @@
factory :condition do
option_list { nil }
remove_data { nil }
action_type { nil }
# the webhook_data is a Json string of form:
# '{"name":"Joe Bloggs","email":"[email protected]","subject":"Large data volume","message":"A message."}'
trait :webhook do
action_type { 'add_webhook' }
webhook_data do
# Generates string from hash
JSON.generate({
name: Faker::Name.name,
email: Faker::Internet.email,
subject: Faker::Lorem.sentence(word_count: 4),
message: Faker::Lorem.paragraph(sentence_count: 2)
})
end
end
end
end
Loading

0 comments on commit 3c5f5bd

Please sign in to comment.