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 9, 2024
1 parent 56759df commit a3a0715
Show file tree
Hide file tree
Showing 11 changed files with 342 additions and 4 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)

# TBD: Clear 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
1 change: 1 addition & 0 deletions app/javascript/src/usage/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ $(() => {
const plansData = JSON.parse($('#plans_created').val());
if (isObject(plansData)) {
const chart = createChart('#yearly_plans', plansData, '', (event) => {
console.log(event);
const segment = chart.getElementAtEvent(event)[0];
if (!isUndefined(segment)) {
const target = $('#plans_click_target').val();
Expand Down
27 changes: 27 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,28 @@ 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('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('');
}

});
};
4 changes: 4 additions & 0 deletions app/models/condition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ class Condition < ApplicationRecord
def deep_copy(**options)
copy = dup
copy.question_id = options.fetch(:question_id, nil)
copy.option_list = options.fetch(:option_list, option_list)
copy.remove_data = options.fetch(:remove_data, remove_data)
copy.action_type = options.fetch(:action_type, action_type)

# TODO: why call validate false here
copy.save!(validate: false) if options.fetch(:save, false)
copy
Expand Down
95 changes: 95 additions & 0 deletions spec/controllers/answers_controller_conditional_questions_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
# frozen_string_literal: true

require 'rails_helper'

RSpec.describe AnswersController, type: :controller do
include RolesHelper

before(:each) do
template = create(:template, phases: 1, sections: 1)
@section = template.sections.first

question1 = create(:question, :checkbox, section: @section, options: 5)
question2 = create(:question, :dropdown, section: @section, options: 3)
question3 = create(:question, :radiobuttons, section: @section, options: 3)
question4 = create(:question, :textarea, section: @section)
question5 = create(:question, :textfield, section: @section)

q1_options = [create(:question_option), create(:question_option), create(:question_option),
create(:question_option), create(:question_option)]
q2_options = [create(:question_option), create(:question_option), create(:question_option)]
q3_options = [create(:question_option), create(:question_option), create(:question_option)]

# Add our created question options to the questions
question1.question_options = q1_options
question2.question_options = q2_options
question3.question_options = q3_options

create(:condition, question: question1,
option_list: [q1_options[0].id],
action_type: 'remove',
remove_data: [question2.id, question4.id])
create(:condition, question: question2, option_list: [q2_options[1].id], remove_data: [question5.id])
create(:condition, question: question3, option_list: [q2_options[1].id], remove_data: [question4.id])

@questions = [question1, question2, question3, question4, question5]
@plan = create(:plan, :creator, template: template)
@user = @plan.owner

ans1 = create(:answer, plan: @plan, question: question1,
question_options: [q1_options[0]], user: @user)
ans2 = create(:answer, plan: @plan, question: question2,
question_options: [q2_options[1]], user: @user)
ans3 = create(:answer, plan: @plan, question: question3,
question_options: [q3_options[2]], user: @user)
ans4 = create(:answer, plan: @plan, question: question4, text: Faker::Lorem.paragraph, user: @user)
ans5 = create(:answer, plan: @plan, question: question5, text: Faker::Lorem.paragraph, user: @user)

@answers = [ans1, ans2, ans3, ans4, ans5]

ActionMailer::Base.deliveries = []
@controller = described_class.new
sign_in(@user)
end

after(:each) do
ActionMailer::Base.deliveries = []
end

describe 'POST /answers/create_or_update', js: true do
context 'TBD' do
before(:each) do
@question = @questions[0]
@question_options = @question.question_options
@ans_text = Faker::Lorem.paragraph
@args = { text: @ans_text, question_option_ids: [@question_options[0]],
user_id: @user.id,
question_id: @question.id, plan_id: @plan.id,
lock_version: 0 }
end
it 'succeeds in updating' do
post :create_or_update, params: { answer: @args }
answer = Answer.where(question: @question).last
expect(answer.present?).to eql(true)
expect(answer.question).to eql(@question)
expect(answer.plan).to eql(@plan)
expect(answer.user).to eql(@user)

json = JSON.parse(response.body).with_indifferent_access
expect(json[:plan].present?).to eql(true)
expect(json[:plan][:progress]).to eql('')
expect(json[:plan][:id]).to eql(@plan.id)
expect(json[:question].present?).to eql(true)
expect(json[:question][:answer_lock_version]).to eql(answer.lock_version)
expect(json[:question][:answer_status]).to eql('')
expect(json[:question][:form]).to eql('')
expect(json[:question][:id]).to eql(@question.id)
expect(json[:question][:locking]).to eql(nil)
expect(json[:section_data].present?).to eql(true)
expect(json[:qn_data].present?).to eql(true)
expect(json[:qn_data][:to_show]).to contain_exactly(@questions[0].id, @questions[2].id, @questions[4].id)
expect(json[:qn_data][:to_hide]).to contain_exactly(@questions[1].id, @questions[3].id)
end
end
end
end
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), create(:question_option)] }
end
trait :lock_version do
lock_version { 0 }
end
end
end
12 changes: 12 additions & 0 deletions spec/factories/conditions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,17 @@
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."}'
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
127 changes: 127 additions & 0 deletions spec/features/questions/conditions_questions_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
# frozen_string_literal: true

require 'rails_helper'

RSpec.describe 'Question::Conditions questions', type: :feature do
before do
@user = create(:user)
@template = create(:template, :default, :published)
@plan = create(:plan, :creator, template: @template)
@phase = create(:phase, template: @template)
@section = create(:section, phase: @phase)

create(:role, :creator, :editor, :commenter, user: @user, plan: @plan)
sign_in(@user)
end

scenario 'User answers multiple questions with conditions', :js do
# Create several different type of questions
question1 = create(:question, :checkbox, section: @section, options: 4)
question2 = create(:question, :dropdown, section: @section, options: 2)
question3 = create(:question, :radiobuttons, section: @section, options: 3)
question4 = create(:question, :textarea, section: @section)
question5 = create(:question, :textfield, section: @section)
question6 = create(:question, :textarea, section: @section)
question7 = create(:question, :textfield, section: @section)

q1_options = question1.question_options
q2_options = question2.question_options
q3_options = question3.question_options

puts "q1_options.length: #{q1_options.length}"

puts "q1_options: #{q1_options.inspect}"

create(:condition, question: question1,
option_list: [q1_options.first.id],
action_type: 'remove',
remove_data: [question4.id])

create(:condition, question: question2,
option_list: [q2_options.last.id],
action_type: 'remove',
remove_data: [question5.id])

visit overview_plan_path(@plan)

click_link 'Write plan'

find("#section-panel-#{@section.id}").click

expect(page).to have_text('(0 / 7)')

within("#answer-form-#{question1.id}") do
check q1_options.first.text
click_button 'Save'
end
expect(page).to have_text('Answered just now')
expect(page).not_to have_selector("#answer-form-#{question4.id}")
expect(page).to have_text('(1 / 6)')

within("#answer-form-#{question2.id}") do
select q2_options.last.text
click_button 'Save'
end

expect(page).to have_text('Answered just now')
expect(page).not_to have_selector("#answer-form-#{question5.id}")
# As one question has been removed, the total of questions is 3
expect(page).to have_text('(2 / 5)')
end

# scenario 'Conditions with multiple options selected', :js do
# question = create(:question, :checkbox, section: @section, options: 4)
# dependent_question = create(:question, :textfield, section: @section)

# options = question.question_options

# create(:condition, question: question,
# option_list: [options[0].id, options[1].id],
# action_type: 'remove',
# remove_data: [dependent_question.id])

# visit edit_plan_path(@plan)

# find("#section-panel-#{@section.id}").click

# within("#answer-form-#{question.id}") do
# check options[0].text
# check options[2].text
# click_button 'Save'
# end

# expect(page).to have_selector("#answer-form-#{dependent_question.id}")

# within("#answer-form-#{question.id}") do
# check options[1].text
# click_button 'Save'
# end

# expect(page).not_to have_selector("#answer-form-#{dependent_question.id}")
# end

# scenario 'User answers question', :js do
# # Setup
# visit overview_plan_path(@plan)

# # Action
# click_link 'Write plan'

# # Expectations
# expect(current_path).to eql(edit_plan_path(@plan))
# expect(page).to have_text('(4 / 5)')

# # # Action
# find("#section-panel-#{@section.id}").click
# # # Fill in the answer form...
# within("#answer-form-#{@questions[0].id}") do
# check @questions[0].question_options.first.text
# click_button 'Save'
# end

# # # Expectations
# # expect(page).to have_text 'Answered just now'
# # expect(page).to have_text '(1 / 1)'
# # expect(Answer.where(question_id: @question.id)).to be_any
# end
end
Loading

0 comments on commit a3a0715

Please sign in to comment.