From 32c8203d2143c6b6e52518e15da2cf3cba44bbf2 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Mon, 5 May 2025 14:46:46 -0600 Subject: [PATCH 01/17] Refactor assigning of sections --- .../answers_controller_with_conditional_questions_spec.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/spec/controllers/answers_controller_with_conditional_questions_spec.rb b/spec/controllers/answers_controller_with_conditional_questions_spec.rb index 589de45b36..61d1ee5ff8 100644 --- a/spec/controllers/answers_controller_with_conditional_questions_spec.rb +++ b/spec/controllers/answers_controller_with_conditional_questions_spec.rb @@ -8,9 +8,7 @@ before(:each) do template = create(:template, phases: 1, sections: 3) # 3 sections for ensuring that conditions involve questions in different sections. - @section1 = template.sections[0] - @section2 = template.sections[1] - @section3 = template.sections[2] + @section1, @section2, @section3 = template.sections # Different types of questions (than can have conditional options) @checkbox_conditional_question = create(:question, :checkbox, section: @section1, options: 5) From a756661828c1c1d0db773e4f0ed070c3fa617ea3 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Mon, 5 May 2025 15:03:24 -0600 Subject: [PATCH 02/17] Refactor assigning of questions --- ...troller_with_conditional_questions_spec.rb | 280 +++++++++--------- .../questions/conditions_questions_spec.rb | 258 ++++++++-------- 2 files changed, 267 insertions(+), 271 deletions(-) diff --git a/spec/controllers/answers_controller_with_conditional_questions_spec.rb b/spec/controllers/answers_controller_with_conditional_questions_spec.rb index 61d1ee5ff8..3f37ddf307 100644 --- a/spec/controllers/answers_controller_with_conditional_questions_spec.rb +++ b/spec/controllers/answers_controller_with_conditional_questions_spec.rb @@ -11,63 +11,61 @@ @section1, @section2, @section3 = template.sections # Different types of questions (than can have conditional options) - @checkbox_conditional_question = create(:question, :checkbox, section: @section1, options: 5) - @radiobutton_conditional_question = create(:question, :radiobuttons, section: @section2, options: 5) - @dropdown_conditional_question = create(:question, :dropdown, section: @section3, options: 5) - - @conditional_questions = [@checkbox_conditional_question, @radiobutton_conditional_question, - @dropdown_conditional_question] + @conditional_questions = { + checkbox: create(:question, :checkbox, section: @section1, options: 5), + radiobutton: create(:question, :radiobuttons, section: @section2, options: 5), + dropdown: create(:question, :dropdown, section: @section3, options: 5) + } # Questions that do not have conditional options for adding or removing - @textarea_questions = create_list(:question, 7, :textarea, section: @section1) - @textfield_questions = create_list(:question, 7, :textfield, section: @section2) - @date_questions = create_list(:question, 7, :date, section: @section3) - @rda_metadata_questions = create_list(:question, 7, :rda_metadata, section: @section1, options: 3) - @checkbox_questions = create_list(:question, 7, :checkbox, section: @section2, options: 3) - @radiobuttons_questions = create_list(:question, 7, :radiobuttons, section: @section3, options: 3) - @dropdown_questions = create_list(:question, 7, :dropdown, section: @section1, options: 3) - @multiselectbox_questions = create_list(:question, 7, :multiselectbox, section: @section2, options: 3) + @non_conditional_questions = { + textarea: create_list(:question, 7, :textarea, section: @section1), + textfield: create_list(:question, 7, :textfield, section: @section2), + date: create_list(:question, 7, :date, section: @section3), + rda_metadata: create_list(:question, 7, :rda_metadata, section: @section1, options: 3), + checkbox: create_list(:question, 7, :checkbox, section: @section2, options: 3), + radiobutton: create_list(:question, 7, :radiobuttons, section: @section3, options: 3), + dropdown: create_list(:question, 7, :dropdown, section: @section1, options: 3), + multiselectbox: create_list(:question, 7, :multiselectbox, section: @section2, options: 3) + } @plan = create(:plan, :creator, template: template) @user = @plan.owner # Answer the questions in List2 - @textarea_answers = @textarea_questions.each.map do |question| + @textarea_answers = @non_conditional_questions[:textarea].each.map do |question| create(:answer, plan: @plan, question: question, user: @user) end - @textfield_answers = @textfield_questions.each.map do |question| + @textfield_answers = @non_conditional_questions[:textfield].each.map do |question| create(:answer, plan: @plan, question: question, user: @user) end - @date_answers = @date_questions.each.map do |question| + @date_answers = @non_conditional_questions[:date].each.map do |question| create(:answer, plan: @plan, question: question, user: @user) end - @rda_metadata_answers = @rda_metadata_questions.each.map do |question| + @rda_metadata_answers = @non_conditional_questions[:rda_metadata].each.map do |question| create(:answer, plan: @plan, question: question, user: @user) end - @checkbox_answers = @checkbox_questions.each.map do |question| + @checkbox_answers = @non_conditional_questions[:checkbox].each.map do |question| create(:answer, plan: @plan, question: question, question_options: [question.question_options[2]], user: @user) end - @radiobuttons_answers = @radiobuttons_questions.each.map do |question| + @radiobuttons_answers = @non_conditional_questions[:radiobutton].each.map do |question| create(:answer, plan: @plan, question: question, question_options: [question.question_options[2]], user: @user) end - @dropdown_answers = @dropdown_questions.each.map do |question| + @dropdown_answers = @non_conditional_questions[:dropdown].each.map do |question| create(:answer, plan: @plan, question: question, question_options: [question.question_options[2]], user: @user) end - @multiselectbox_answers = @multiselectbox_questions.each.map do |question| + @multiselectbox_answers = @non_conditional_questions[:multiselectbox].each.map do |question| create(:answer, plan: @plan, question: question, question_options: [question.question_options[2]], user: @user) end - @all_questions_ids = (@conditional_questions + @textarea_questions + @textfield_questions + - @date_questions + @rda_metadata_questions + - @checkbox_questions + @radiobuttons_questions + - @dropdown_questions + @multiselectbox_questions).map(&:id) + @all_questions_ids = (@conditional_questions.values + @non_conditional_questions.values.flatten).map(&:id) @all_answers_ids = (@textarea_answers + @textfield_answers + @date_answers + @rda_metadata_answers + @@ -89,21 +87,21 @@ # NOTE: Checkboxes allow for multiple options to be selected. context 'with conditional checkbox question' do it 'handles single option (with condition) in option_list ' do - condition = create(:condition, question: @checkbox_conditional_question, - option_list: [@checkbox_conditional_question.question_options[2].id], + condition = create(:condition, question: @conditional_questions[:checkbox], + option_list: [@conditional_questions[:checkbox].question_options[2].id], action_type: 'remove', - remove_data: [@textarea_questions[5].id, @textfield_questions[5].id, - @date_questions[5].id, @rda_metadata_questions[5].id, - @checkbox_questions[5].id, @radiobuttons_questions[5].id, - @dropdown_questions[5].id, @multiselectbox_questions[5].id]) + remove_data: [@non_conditional_questions[:textarea][5].id, @non_conditional_questions[:textfield][5].id, + @non_conditional_questions[:date][5].id, @non_conditional_questions[:rda_metadata][5].id, + @non_conditional_questions[:checkbox][5].id, @non_conditional_questions[:radiobutton][5].id, + @non_conditional_questions[:dropdown][5].id, @non_conditional_questions[:multiselectbox][5].id]) # We chose an option that is in the option_list of the condition defined above. Note that # the text sent by UI is an empty string. args = { text: '', - question_option_ids: [@checkbox_conditional_question.question_options[2].id], + question_option_ids: [@conditional_questions[:checkbox].question_options[2].id], user_id: @user.id, - question_id: @checkbox_conditional_question.id, + question_id: @conditional_questions[:checkbox].id, plan_id: @plan.id, lock_version: 0 } @@ -131,28 +129,28 @@ ) end it 'handles single option (without condition) in option_list' do - create(:condition, question: @checkbox_conditional_question, - option_list: [@checkbox_conditional_question.question_options[1].id], + create(:condition, question: @conditional_questions[:checkbox], + option_list: [@conditional_questions[:checkbox].question_options[1].id], action_type: 'remove', - remove_data: [@textarea_questions[3].id, @textfield_questions[3].id, - @date_questions[3].id, @rda_metadata_questions[3].id, - @checkbox_questions[3].id, @dropdown_questions[3].id, - @multiselectbox_questions[3].id]) + remove_data: [@non_conditional_questions[:textarea][3].id, @non_conditional_questions[:textfield][3].id, + @non_conditional_questions[:date][3].id, @non_conditional_questions[:rda_metadata][3].id, + @non_conditional_questions[:checkbox][3].id, @non_conditional_questions[:dropdown][3].id, + @non_conditional_questions[:multiselectbox][3].id]) - create(:condition, question: @checkbox_conditional_question, - option_list: [@checkbox_conditional_question.question_options[4].id], + create(:condition, question: @conditional_questions[:checkbox], + option_list: [@conditional_questions[:checkbox].question_options[4].id], action_type: 'remove', - remove_data: [@textarea_questions[0].id, @textfield_questions[0].id, - @date_questions[0].id, @rda_metadata_questions[0].id, - @checkbox_questions[0].id, @dropdown_questions[0].id, - @multiselectbox_questions[0].id]) + remove_data: [@non_conditional_questions[:textarea][0].id, @non_conditional_questions[:textfield][0].id, + @non_conditional_questions[:date][0].id, @non_conditional_questions[:rda_metadata][0].id, + @non_conditional_questions[:checkbox][0].id, @non_conditional_questions[:dropdown][0].id, + @non_conditional_questions[:multiselectbox][0].id]) # We choose an option that is not in the option_list of the conditions defined above. args = { text: '', - question_option_ids: [@checkbox_conditional_question.question_options[0].id], + question_option_ids: [@conditional_questions[:checkbox].question_options[0].id], user_id: @user.id, - question_id: @checkbox_conditional_question.id, + question_id: @conditional_questions[:checkbox].id, plan_id: @plan.id, lock_version: 0 } @@ -165,30 +163,30 @@ end it 'handles multiple options (some with conditions) in option_list' do - condition1 = create(:condition, question: @checkbox_conditional_question, - option_list: [@checkbox_conditional_question.question_options[2].id], + condition1 = create(:condition, question: @conditional_questions[:checkbox], + option_list: [@conditional_questions[:checkbox].question_options[2].id], action_type: 'remove', - remove_data: [@textarea_questions[0].id, @textfield_questions[0].id, - @date_questions[0].id, @rda_metadata_questions[0].id, - @checkbox_questions[0].id, @dropdown_questions[0].id, - @multiselectbox_questions[0].id]) + remove_data: [@non_conditional_questions[:textarea][0].id, @non_conditional_questions[:textfield][0].id, + @non_conditional_questions[:date][0].id, @non_conditional_questions[:rda_metadata][0].id, + @non_conditional_questions[:checkbox][0].id, @non_conditional_questions[:dropdown][0].id, + @non_conditional_questions[:multiselectbox][0].id]) - condition2 = create(:condition, question: @checkbox_conditional_question, - option_list: [@checkbox_conditional_question.question_options[4].id], + condition2 = create(:condition, question: @conditional_questions[:checkbox], + option_list: [@conditional_questions[:checkbox].question_options[4].id], action_type: 'remove', - remove_data: [@textarea_questions[3].id, @textfield_questions[3].id, - @date_questions[3].id, @rda_metadata_questions[3].id, - @checkbox_questions[3].id, @dropdown_questions[3].id, - @multiselectbox_questions[3].id]) + remove_data: [@non_conditional_questions[:textarea][3].id, @non_conditional_questions[:textfield][3].id, + @non_conditional_questions[:date][3].id, @non_conditional_questions[:rda_metadata][3].id, + @non_conditional_questions[:checkbox][3].id, @non_conditional_questions[:dropdown][3].id, + @non_conditional_questions[:multiselectbox][3].id]) # We choose options that is in the option_list of the conditions defined above as well as an option # with no condition defined. args = { - question_option_ids: [@checkbox_conditional_question.question_options[1].id, - @checkbox_conditional_question.question_options[2].id, - @checkbox_conditional_question.question_options[4].id], + question_option_ids: [@conditional_questions[:checkbox].question_options[1].id, + @conditional_questions[:checkbox].question_options[2].id, + @conditional_questions[:checkbox].question_options[4].id], user_id: @user.id, - question_id: @checkbox_conditional_question.id, + question_id: @conditional_questions[:checkbox].id, plan_id: @plan.id, lock_version: 0 } @@ -206,20 +204,20 @@ # Note: radiobuttons only allow single selection. context 'with conditional radiobuttons question' do it 'handles single option (with condition) in option_list ' do - condition = create(:condition, question: @radiobutton_conditional_question, - option_list: [@radiobutton_conditional_question.question_options[2].id], + condition = create(:condition, question: @conditional_questions[:radiobutton], + option_list: [@conditional_questions[:radiobutton].question_options[2].id], action_type: 'remove', - remove_data: [@textarea_questions[5].id, @textfield_questions[5].id, - @date_questions[5].id, @rda_metadata_questions[5].id, - @checkbox_questions[5].id, @radiobuttons_questions[5].id, - @dropdown_questions[5].id, @multiselectbox_questions[5].id]) + remove_data: [@non_conditional_questions[:textarea][5].id, @non_conditional_questions[:textfield][5].id, + @non_conditional_questions[:date][5].id, @non_conditional_questions[:rda_metadata][5].id, + @non_conditional_questions[:checkbox][5].id, @non_conditional_questions[:radiobutton][5].id, + @non_conditional_questions[:dropdown][5].id, @non_conditional_questions[:multiselectbox][5].id]) # We choose an option that is in the option_list of the condition defined above. args = { text: '', - question_option_ids: [@radiobutton_conditional_question.question_options[2].id], + question_option_ids: [@conditional_questions[:radiobutton].question_options[2].id], user_id: @user.id, - question_id: @radiobutton_conditional_question.id, + question_id: @conditional_questions[:radiobutton].id, plan_id: @plan.id, lock_version: 0 } @@ -233,28 +231,28 @@ expect(json[:qn_data][:to_hide]).to match_array(expected_to_hide_question_ids) end it 'handles single option (without condition) in option_list' do - create(:condition, question: @radiobutton_conditional_question, - option_list: [@radiobutton_conditional_question.question_options[1].id], + create(:condition, question: @conditional_questions[:radiobutton], + option_list: [@conditional_questions[:radiobutton].question_options[1].id], action_type: 'remove', - remove_data: [@textarea_questions[3].id, @textfield_questions[3].id, - @date_questions[3].id, @rda_metadata_questions[3].id, - @checkbox_questions[3].id, @dropdown_questions[3].id, - @multiselectbox_questions[3].id]) + remove_data: [@non_conditional_questions[:textarea][3].id, @non_conditional_questions[:textfield][3].id, + @non_conditional_questions[:date][3].id, @non_conditional_questions[:rda_metadata][3].id, + @non_conditional_questions[:checkbox][3].id, @non_conditional_questions[:dropdown][3].id, + @non_conditional_questions[:multiselectbox][3].id]) - create(:condition, question: @radiobutton_conditional_question, - option_list: [@radiobutton_conditional_question.question_options[4].id], + create(:condition, question: @conditional_questions[:radiobutton], + option_list: [@conditional_questions[:radiobutton].question_options[4].id], action_type: 'remove', - remove_data: [@textarea_questions[0].id, @textfield_questions[0].id, - @date_questions[0].id, @rda_metadata_questions[0].id, - @checkbox_questions[0].id, @dropdown_questions[0].id, - @multiselectbox_questions[0].id]) + remove_data: [@non_conditional_questions[:textarea][0].id, @non_conditional_questions[:textfield][0].id, + @non_conditional_questions[:date][0].id, @non_conditional_questions[:rda_metadata][0].id, + @non_conditional_questions[:checkbox][0].id, @non_conditional_questions[:dropdown][0].id, + @non_conditional_questions[:multiselectbox][0].id]) # We choose an option that is not in the option_list of the conditions defined above. args = { text: '', - question_option_ids: [@radiobutton_conditional_question.question_options[0].id], + question_option_ids: [@conditional_questions[:radiobutton].question_options[0].id], user_id: @user.id, - question_id: @radiobutton_conditional_question.id, + question_id: @conditional_questions[:radiobutton].id, plan_id: @plan.id, lock_version: 0 } @@ -270,20 +268,20 @@ # NOTE: dropdowns only allow single selection. context 'with conditional dropdown question' do it 'handles single option (with condition) in option_list ' do - condition = create(:condition, question: @dropdown_conditional_question, - option_list: [@dropdown_conditional_question.question_options[2].id], + condition = create(:condition, question: @conditional_questions[:dropdown], + option_list: [@conditional_questions[:dropdown].question_options[2].id], action_type: 'remove', - remove_data: [@textarea_questions[5].id, @textfield_questions[5].id, - @date_questions[5].id, @rda_metadata_questions[5].id, - @checkbox_questions[5].id, @radiobuttons_questions[5].id, - @dropdown_questions[5].id, @multiselectbox_questions[5].id]) + remove_data: [@non_conditional_questions[:textarea][5].id, @non_conditional_questions[:textfield][5].id, + @non_conditional_questions[:date][5].id, @non_conditional_questions[:rda_metadata][5].id, + @non_conditional_questions[:checkbox][5].id, @non_conditional_questions[:radiobutton][5].id, + @non_conditional_questions[:dropdown][5].id, @non_conditional_questions[:multiselectbox][5].id]) # We chose an option that is in the option_list of the condition defined above. args = { - text: @dropdown_conditional_question.question_options[2].text, - question_option_ids: [@dropdown_conditional_question.question_options[2].id], + text: @conditional_questions[:dropdown].question_options[2].text, + question_option_ids: [@conditional_questions[:dropdown].question_options[2].id], user_id: @user.id, - question_id: @dropdown_conditional_question.id, + question_id: @conditional_questions[:dropdown].id, plan_id: @plan.id, lock_version: 0 } @@ -297,28 +295,28 @@ expect(json[:qn_data][:to_hide]).to match_array(expected_to_hide_question_ids) end it 'handles single option (without condition) in option_list' do - create(:condition, question: @dropdown_conditional_question, - option_list: [@dropdown_conditional_question.question_options[1].id], + create(:condition, question: @conditional_questions[:dropdown], + option_list: [@conditional_questions[:dropdown].question_options[1].id], action_type: 'remove', - remove_data: [@textarea_questions[3].id, @textfield_questions[3].id, - @date_questions[3].id, @rda_metadata_questions[3].id, - @checkbox_questions[3].id, @dropdown_questions[3].id, - @multiselectbox_questions[3].id]) + remove_data: [@non_conditional_questions[:textarea][3].id, @non_conditional_questions[:textfield][3].id, + @non_conditional_questions[:date][3].id, @non_conditional_questions[:rda_metadata][3].id, + @non_conditional_questions[:checkbox][3].id, @non_conditional_questions[:dropdown][3].id, + @non_conditional_questions[:multiselectbox][3].id]) - create(:condition, question: @dropdown_conditional_question, - option_list: [@dropdown_conditional_question.question_options[4].id], + create(:condition, question: @conditional_questions[:dropdown], + option_list: [@conditional_questions[:dropdown].question_options[4].id], action_type: 'remove', - remove_data: [@textarea_questions[0].id, @textfield_questions[0].id, - @date_questions[0].id, @rda_metadata_questions[0].id, - @checkbox_questions[0].id, @dropdown_questions[0].id, - @multiselectbox_questions[0].id]) + remove_data: [@non_conditional_questions[:textarea][0].id, @non_conditional_questions[:textfield][0].id, + @non_conditional_questions[:date][0].id, @non_conditional_questions[:rda_metadata][0].id, + @non_conditional_questions[:checkbox][0].id, @non_conditional_questions[:dropdown][0].id, + @non_conditional_questions[:multiselectbox][0].id]) # We choose an option that is not in the option_list of the conditions defined above. args = { text: '', - question_option_ids: [@dropdown_conditional_question.question_options[0].id], + question_option_ids: [@conditional_questions[:dropdown].question_options[0].id], user_id: @user.id, - question_id: @dropdown_conditional_question.id, + question_id: @conditional_questions[:dropdown].id, plan_id: @plan.id, lock_version: 0 } @@ -343,16 +341,16 @@ it 'handles a checkbox option (with add_webhook condition)' do add_webhook_condition = create( :condition, :webhook, - question: @checkbox_conditional_question, - option_list: [@checkbox_conditional_question.question_options[2].id] + question: @conditional_questions[:checkbox], + option_list: [@conditional_questions[:checkbox].question_options[2].id] ) # We chose an option that is in the option_list of the condition defined above. Note that # the text sent by UI is an empty string. args = { text: '', - question_option_ids: [@checkbox_conditional_question.question_options[2].id], + question_option_ids: [@conditional_questions[:checkbox].question_options[2].id], user_id: @user.id, - question_id: @checkbox_conditional_question.id, + question_id: @conditional_questions[:checkbox].id, plan_id: @plan.id, lock_version: 0 } @@ -381,33 +379,33 @@ # Message should have @user.name, chosen option text and question text. expect(mail.body.encoded).to include(@user.name) - expect(mail.body.encoded).to include(@checkbox_conditional_question.question_options[2].text) - expect(mail.body.encoded).to include(@checkbox_conditional_question.text) + expect(mail.body.encoded).to include(@conditional_questions[:checkbox].question_options[2].text) + expect(mail.body.encoded).to include(@conditional_questions[:checkbox].text) end end it 'handles multiple checkbox options (one of which is add_webhook condition)' do add_webhook_condition = create(:condition, :webhook, - question: @checkbox_conditional_question, - option_list: [@checkbox_conditional_question.question_options[2].id]) + question: @conditional_questions[:checkbox], + option_list: [@conditional_questions[:checkbox].question_options[2].id]) - condition2 = create(:condition, question: @checkbox_conditional_question, - option_list: [@checkbox_conditional_question.question_options[4].id], + condition2 = create(:condition, question: @conditional_questions[:checkbox], + option_list: [@conditional_questions[:checkbox].question_options[4].id], action_type: 'remove', - remove_data: [@textarea_questions[3].id, @textfield_questions[3].id, - @date_questions[3].id, @rda_metadata_questions[3].id, - @checkbox_questions[3].id, @dropdown_questions[3].id, - @multiselectbox_questions[3].id]) + remove_data: [@non_conditional_questions[:textarea][3].id, @non_conditional_questions[:textfield][3].id, + @non_conditional_questions[:date][3].id, @non_conditional_questions[:rda_metadata][3].id, + @non_conditional_questions[:checkbox][3].id, @non_conditional_questions[:dropdown][3].id, + @non_conditional_questions[:multiselectbox][3].id]) # We chose an option that is in the option_list of the condition defined above. Note that # the text sent by UI is an empty string. args = { text: '', - question_option_ids: [@checkbox_conditional_question.question_options[2].id, - @checkbox_conditional_question.question_options[4].id, - @checkbox_conditional_question.question_options[1].id], + question_option_ids: [@conditional_questions[:checkbox].question_options[2].id, + @conditional_questions[:checkbox].question_options[4].id, + @conditional_questions[:checkbox].question_options[1].id], user_id: @user.id, - question_id: @checkbox_conditional_question.id, + question_id: @conditional_questions[:checkbox].id, plan_id: @plan.id, lock_version: 0 } @@ -437,24 +435,24 @@ # Message should have @user.name, chosen option text and question text. expect(mail.body.encoded).to include(@user.name) - expect(mail.body.encoded).to include(@checkbox_conditional_question.question_options[2].text) - expect(mail.body.encoded).to include(@checkbox_conditional_question.text) + expect(mail.body.encoded).to include(@conditional_questions[:checkbox].question_options[2].text) + expect(mail.body.encoded).to include(@conditional_questions[:checkbox].text) end end it 'handles selection of a dropdown option (with add_webhook condition)' do add_webhook_condition = create(:condition, :webhook, - question: @dropdown_conditional_question, - option_list: [@dropdown_conditional_question.question_options[2].id]) + question: @conditional_questions[:dropdown], + option_list: [@conditional_questions[:dropdown].question_options[2].id]) # We chose an option that is in the option_list of the condition defined above. Note that # the text sent by UI is an empty string. args = { text: '', - question_option_ids: [@dropdown_conditional_question.question_options[2].id], + question_option_ids: [@conditional_questions[:dropdown].question_options[2].id], user_id: @user.id, - question_id: @dropdown_conditional_question.id, + question_id: @conditional_questions[:dropdown].id, plan_id: @plan.id, lock_version: 0 } @@ -483,24 +481,24 @@ # Message should have @user.name, chosen option text and question text. expect(mail.body.encoded).to include(@user.name) - expect(mail.body.encoded).to include(@dropdown_conditional_question.question_options[2].text) - expect(mail.body.encoded).to include(@dropdown_conditional_question.text) + expect(mail.body.encoded).to include(@conditional_questions[:dropdown].question_options[2].text) + expect(mail.body.encoded).to include(@conditional_questions[:dropdown].text) end end it 'handles selection of a radiobutton option (with add_webhook condition)' do add_webhook_condition = create(:condition, :webhook, - question: @radiobutton_conditional_question, - option_list: [@radiobutton_conditional_question.question_options[2].id]) + question: @conditional_questions[:radiobutton], + option_list: [@conditional_questions[:radiobutton].question_options[2].id]) # We chose an option that is in the option_list of the condition defined above. Note that # the text sent by UI is an empty string. args = { text: '', - question_option_ids: [@radiobutton_conditional_question.question_options[2].id], + question_option_ids: [@conditional_questions[:radiobutton].question_options[2].id], user_id: @user.id, - question_id: @radiobutton_conditional_question.id, + question_id: @conditional_questions[:radiobutton].id, plan_id: @plan.id, lock_version: 0 } @@ -529,8 +527,8 @@ # Message should have @user.name, chosen option text and question text. expect(mail.body.encoded).to include(@user.name) - expect(mail.body.encoded).to include(@radiobutton_conditional_question.question_options[2].text) - expect(mail.body.encoded).to include(@radiobutton_conditional_question.text) + expect(mail.body.encoded).to include(@conditional_questions[:radiobutton].question_options[2].text) + expect(mail.body.encoded).to include(@conditional_questions[:radiobutton].text) end end end diff --git a/spec/features/questions/conditions_questions_spec.rb b/spec/features/questions/conditions_questions_spec.rb index fc0fe340a2..26aa9e775b 100644 --- a/spec/features/questions/conditions_questions_spec.rb +++ b/spec/features/questions/conditions_questions_spec.rb @@ -14,68 +14,66 @@ @section3 = create(:section, phase: @phase) # Different types of questions (than can have conditional options) - @checkbox_conditional_question = create(:question, :checkbox, section: @section1, options: 5) - @radiobutton_conditional_question = create(:question, :radiobuttons, section: @section2, options: 5) - @dropdown_conditional_question = create(:question, :dropdown, section: @section3, options: 5) - - @conditional_questions = [@checkbox_conditional_question, @radiobutton_conditional_question, - @dropdown_conditional_question] + @conditional_questions = { + checkbox: create(:question, :checkbox, section: @section1, options: 5), + radiobutton: create(:question, :radiobuttons, section: @section2, options: 5), + dropdown: create(:question, :dropdown, section: @section3, options: 5) + } # Questions that do not have conditional options for adding or removing - @textarea_questions = create_list(:question, 3, :textarea, section: @section1) - @textfield_questions = create_list(:question, 3, :textfield, section: @section2) - @date_questions = create_list(:question, 3, :date, section: @section3) - @rda_metadata_questions = create_list(:question, 3, :rda_metadata, section: @section1, options: 5) - @checkbox_questions = create_list(:question, 3, :checkbox, section: @section2, options: 5) - @radiobuttons_questions = create_list(:question, 3, :radiobuttons, section: @section3, options: 5) - @dropdown_questions = create_list(:question, 3, :dropdown, section: @section1, options: 5) - @multiselectbox_questions = create_list(:question, 3, :multiselectbox, section: @section2, options: 5) + @non_conditional_questions = { + textarea: create_list(:question, 3, :textarea, section: @section1), + textfield: create_list(:question, 3, :textfield, section: @section2), + date: create_list(:question, 3, :date, section: @section3), + rda_metadata: create_list(:question, 3, :rda_metadata, section: @section1, options: 5), + checkbox: create_list(:question, 3, :checkbox, section: @section2, options: 5), + radiobutton: create_list(:question, 3, :radiobuttons, section: @section3, options: 5), + dropdown: create_list(:question, 3, :dropdown, section: @section1, options: 5), + multiselectbox: create_list(:question, 3, :multiselectbox, section: @section2, options: 5) + } create(:role, :creator, :editor, :commenter, user: @user, plan: @plan) - @all_questions_ids = (@conditional_questions + @textarea_questions + @textfield_questions + - @date_questions + @rda_metadata_questions + - @checkbox_questions + @radiobuttons_questions + - @dropdown_questions + @multiselectbox_questions).map(&:id) + @all_questions_ids = (@conditional_questions.values + @non_conditional_questions.values.flatten).map(&:id) # Answer the non-conditional questions - @textarea_answers = @textarea_questions.each.map do |question| + @textarea_answers = @non_conditional_questions[:textarea].each.map do |question| create(:answer, plan: @plan, question: question, user: @user) end @all_non_conditional_question_answers_ids = @textarea_answers.map(&:id) - @textfield_answers = @textfield_questions.each.map do |question| + @textfield_answers = @non_conditional_questions[:textfield].each.map do |question| create(:answer, plan: @plan, question: question, user: @user) end @all_non_conditional_question_answers_ids += @textfield_answers.map(&:id) - @date_answers = @date_questions.each.map do |question| + @date_answers = @non_conditional_questions[:date].each.map do |question| create(:answer, plan: @plan, question: question, user: @user) end @all_non_conditional_question_answers_ids += @date_answers.map(&:id) - @rda_metadata_answers = @rda_metadata_questions.each.map do |question| + @rda_metadata_answers = @non_conditional_questions[:rda_metadata].each.map do |question| create(:answer, plan: @plan, question: question, question_options: [question.question_options[2]], user: @user) end @all_non_conditional_question_answers_ids += @rda_metadata_answers.map(&:id) - @checkbox_answers = @checkbox_questions.each.map do |question| + @checkbox_answers = @non_conditional_questions[:checkbox].each.map do |question| create(:answer, plan: @plan, question: question, question_options: [question.question_options[2]], user: @user) end @all_non_conditional_question_answers_ids += @checkbox_answers.map(&:id) - @radiobuttons_answers = @radiobuttons_questions.each.map do |question| + @radiobuttons_answers = @non_conditional_questions[:radiobutton].each.map do |question| create(:answer, plan: @plan, question: question, question_options: [question.question_options[2]], user: @user) end @all_non_conditional_question_answers_ids += @radiobuttons_answers.map(&:id) - @dropdown_answers = @dropdown_questions.each.map do |question| + @dropdown_answers = @non_conditional_questions[:dropdown].each.map do |question| create(:answer, plan: @plan, question: question, question_options: [question.question_options[2]], user: @user) end @all_non_conditional_question_answers_ids += @dropdown_answers.map(&:id) - @multiselectbox_answers = @multiselectbox_questions.each.map do |question| + @multiselectbox_answers = @non_conditional_questions[:multiselectbox].each.map do |question| create(:answer, plan: @plan, question: question, question_options: [question.question_options[2]], user: @user) end @all_non_conditional_question_answers_ids += @multiselectbox_answers.map(&:id) @@ -94,17 +92,17 @@ describe 'conditions with action_type remove' do feature 'User answers a checkboxes question with a condition' do scenario 'User answers chooses checkbox option with a condition', :js do - condition = create(:condition, question: @checkbox_conditional_question, - option_list: [@checkbox_conditional_question.question_options[2].id], + condition = create(:condition, question: @conditional_questions[:checkbox], + option_list: [@conditional_questions[:checkbox].question_options[2].id], action_type: 'remove', - remove_data: [@textarea_questions[0].id, - @textfield_questions[1].id, - @date_questions[2].id, - @rda_metadata_questions[0].id, - @checkbox_questions[1].id, - @radiobuttons_questions[2].id, - @dropdown_questions[0].id, - @multiselectbox_questions[1].id]) + remove_data: [@non_conditional_questions[:textarea][0].id, + @non_conditional_questions[:textfield][1].id, + @non_conditional_questions[:date][2].id, + @non_conditional_questions[:rda_metadata][0].id, + @non_conditional_questions[:checkbox][1].id, + @non_conditional_questions[:radiobutton][2].id, + @non_conditional_questions[:dropdown][0].id, + @non_conditional_questions[:multiselectbox][1].id]) visit overview_plan_path(@plan) @@ -118,8 +116,8 @@ expect(page).to have_text('24/27 answered') # Answer the checkbox_conditional_question. - within("#answer-form-#{@checkbox_conditional_question.id}") do - check @checkbox_conditional_question.question_options[2].text + within("#answer-form-#{@conditional_questions[:checkbox].id}") do + check @conditional_questions[:checkbox].question_options[2].text click_button 'Save' end @@ -140,33 +138,33 @@ end # Now uncheck checkbox_conditional_question answer. - within("#answer-form-#{@checkbox_conditional_question.id}") do - uncheck @checkbox_conditional_question.question_options[2].text + within("#answer-form-#{@conditional_questions[:checkbox].id}") do + uncheck @conditional_questions[:checkbox].question_options[2].text click_button 'Save' end # Expect 27 questions to appear again, but the 8 answers that were removed should not be there. - # Also 1 answer should be removed as we unchecked @checkbox_conditional_question.question_options[2].text + # Also 1 answer should be removed as we unchecked @conditional_questions[:checkbox].question_options[2].text # 17 (from previous check) - 1 = 16 expect(page).to have_text('16/27 answered') end scenario 'User answers chooses checkbox option without a condition', :js do - create(:condition, question: @checkbox_conditional_question, - option_list: [@checkbox_conditional_question.question_options[1].id], + create(:condition, question: @conditional_questions[:checkbox], + option_list: [@conditional_questions[:checkbox].question_options[1].id], action_type: 'remove', - remove_data: [@textarea_questions[2].id, @textfield_questions[2].id, - @date_questions[2].id, @rda_metadata_questions[2].id, - @checkbox_questions[2].id, @dropdown_questions[2].id, - @multiselectbox_questions[2].id]) + remove_data: [@non_conditional_questions[:textarea][2].id, @non_conditional_questions[:textfield][2].id, + @non_conditional_questions[:date][2].id, @non_conditional_questions[:rda_metadata][2].id, + @non_conditional_questions[:checkbox][2].id, @non_conditional_questions[:dropdown][2].id, + @non_conditional_questions[:multiselectbox][2].id]) - create(:condition, question: @checkbox_conditional_question, - option_list: [@checkbox_conditional_question.question_options[4].id], + create(:condition, question: @conditional_questions[:checkbox], + option_list: [@conditional_questions[:checkbox].question_options[4].id], action_type: 'remove', - remove_data: [@textarea_questions[0].id, @textfield_questions[0].id, - @date_questions[0].id, @rda_metadata_questions[0].id, - @checkbox_questions[0].id, @dropdown_questions[0].id, - @multiselectbox_questions[0].id]) + remove_data: [@non_conditional_questions[:textarea][0].id, @non_conditional_questions[:textfield][0].id, + @non_conditional_questions[:date][0].id, @non_conditional_questions[:rda_metadata][0].id, + @non_conditional_questions[:checkbox][0].id, @non_conditional_questions[:dropdown][0].id, + @non_conditional_questions[:multiselectbox][0].id]) # We choose an option that is not in the option_list of the conditions defined above. visit overview_plan_path(@plan) @@ -181,8 +179,8 @@ expect(page).to have_text('24/27 answered') # Answer the checkbox_conditional_question - within("#answer-form-#{@checkbox_conditional_question.id}") do - check @checkbox_conditional_question.question_options[0].text + within("#answer-form-#{@conditional_questions[:checkbox].id}") do + check @conditional_questions[:checkbox].question_options[0].text click_button 'Save' end @@ -195,17 +193,17 @@ feature 'User answers a radiobutton question with a condition' do scenario 'User answers selects radiobutton option with a condition', :js do - condition = create(:condition, question: @radiobutton_conditional_question, - option_list: [@radiobutton_conditional_question.question_options[2].id], + condition = create(:condition, question: @conditional_questions[:radiobutton], + option_list: [@conditional_questions[:radiobutton].question_options[2].id], action_type: 'remove', - remove_data: [@textarea_questions[0].id, - @textfield_questions[1].id, - @date_questions[2].id, - @rda_metadata_questions[0].id, - @checkbox_questions[1].id, - @radiobuttons_questions[2].id, - @dropdown_questions[0].id, - @multiselectbox_questions[1].id]) + remove_data: [@non_conditional_questions[:textarea][0].id, + @non_conditional_questions[:textfield][1].id, + @non_conditional_questions[:date][2].id, + @non_conditional_questions[:rda_metadata][0].id, + @non_conditional_questions[:checkbox][1].id, + @non_conditional_questions[:radiobutton][2].id, + @non_conditional_questions[:dropdown][0].id, + @non_conditional_questions[:multiselectbox][1].id]) visit overview_plan_path(@plan) @@ -219,8 +217,8 @@ expect(page).to have_text('24/27 answered') # Answer the radiobutton_conditional_question. - within("#answer-form-#{@radiobutton_conditional_question.id}") do - choose @radiobutton_conditional_question.question_options[2].text + within("#answer-form-#{@conditional_questions[:radiobutton].id}") do + choose @conditional_questions[:radiobutton].question_options[2].text click_button 'Save' end @@ -243,34 +241,34 @@ # Now for radiobutton_conditional_question answer, there in no unchoose option, # so we switch options to a different option without any conditions. - within("#answer-form-#{@radiobutton_conditional_question.id}") do - choose @radiobutton_conditional_question.question_options[0].text + within("#answer-form-#{@conditional_questions[:radiobutton].id}") do + choose @conditional_questions[:radiobutton].question_options[0].text click_button 'Save' end # Check questions answered in progress bar. # Expect 27 questions to appear again, but the 8 answers that were removed should not be there. - # Also 1 answer should be removed as we unchecked @radiobutton_conditional_question.question_options[2].text + # Also 1 answer should be removed as we unchecked @conditional_questions[:radiobutton].question_options[2].text # 17 (from previous check) - 1 = 16 expect(page).to have_text('17/27 answered') end scenario 'User answers selects radiobutton option without a condition', :js do - create(:condition, question: @radiobutton_conditional_question, - option_list: [@radiobutton_conditional_question.question_options[1].id], + create(:condition, question: @conditional_questions[:radiobutton], + option_list: [@conditional_questions[:radiobutton].question_options[1].id], action_type: 'remove', - remove_data: [@textarea_questions[2].id, @textfield_questions[2].id, - @date_questions[2].id, @rda_metadata_questions[2].id, - @checkbox_questions[2].id, @dropdown_questions[2].id, - @multiselectbox_questions[2].id]) + remove_data: [@non_conditional_questions[:textarea][2].id, @non_conditional_questions[:textfield][2].id, + @non_conditional_questions[:date][2].id, @non_conditional_questions[:rda_metadata][2].id, + @non_conditional_questions[:checkbox][2].id, @non_conditional_questions[:dropdown][2].id, + @non_conditional_questions[:multiselectbox][2].id]) - create(:condition, question: @radiobutton_conditional_question, - option_list: [@radiobutton_conditional_question.question_options[4].id], + create(:condition, question: @conditional_questions[:radiobutton], + option_list: [@conditional_questions[:radiobutton].question_options[4].id], action_type: 'remove', - remove_data: [@textarea_questions[0].id, @textfield_questions[0].id, - @date_questions[0].id, @rda_metadata_questions[0].id, - @checkbox_questions[0].id, @dropdown_questions[0].id, - @multiselectbox_questions[0].id]) + remove_data: [@non_conditional_questions[:textarea][0].id, @non_conditional_questions[:textfield][0].id, + @non_conditional_questions[:date][0].id, @non_conditional_questions[:rda_metadata][0].id, + @non_conditional_questions[:checkbox][0].id, @non_conditional_questions[:dropdown][0].id, + @non_conditional_questions[:multiselectbox][0].id]) # We choose an option that is not in the option_list of the conditions defined above. visit overview_plan_path(@plan) @@ -285,8 +283,8 @@ expect(page).to have_text('24/27 answered') # Answer the radiobutton_conditional_question. - within("#answer-form-#{@radiobutton_conditional_question.id}") do - choose @radiobutton_conditional_question.question_options[0].text + within("#answer-form-#{@conditional_questions[:radiobutton].id}") do + choose @conditional_questions[:radiobutton].question_options[0].text click_button 'Save' end @@ -299,17 +297,17 @@ feature 'User answers a dropdown question with a condition' do scenario 'User answers chooses dropdown option with a condition', :js do - condition = create(:condition, question: @dropdown_conditional_question, - option_list: [@dropdown_conditional_question.question_options[2].id], + condition = create(:condition, question: @conditional_questions[:dropdown], + option_list: [@conditional_questions[:dropdown].question_options[2].id], action_type: 'remove', - remove_data: [@textarea_questions[0].id, - @textfield_questions[1].id, - @date_questions[2].id, - @rda_metadata_questions[0].id, - @checkbox_questions[1].id, - @radiobuttons_questions[2].id, - @dropdown_questions[0].id, - @multiselectbox_questions[1].id]) + remove_data: [@non_conditional_questions[:textarea][0].id, + @non_conditional_questions[:textfield][1].id, + @non_conditional_questions[:date][2].id, + @non_conditional_questions[:rda_metadata][0].id, + @non_conditional_questions[:checkbox][1].id, + @non_conditional_questions[:radiobutton][2].id, + @non_conditional_questions[:dropdown][0].id, + @non_conditional_questions[:multiselectbox][1].id]) visit overview_plan_path(@plan) @@ -323,8 +321,8 @@ expect(page).to have_text('24/27 answered') # Answer the dropdown_conditional_question - within("#answer-form-#{@dropdown_conditional_question.id}") do - select(@dropdown_conditional_question.question_options[2].text, from: 'answer_question_option_ids') + within("#answer-form-#{@conditional_questions[:dropdown].id}") do + select(@conditional_questions[:dropdown].question_options[2].text, from: 'answer_question_option_ids') click_button 'Save' end @@ -346,8 +344,8 @@ end # Now select another option for dropdown_conditional_question. - within("#answer-form-#{@dropdown_conditional_question.id}") do - select(@dropdown_conditional_question.question_options[1].text, from: 'answer_question_option_ids') + within("#answer-form-#{@conditional_questions[:dropdown].id}") do + select(@conditional_questions[:dropdown].question_options[1].text, from: 'answer_question_option_ids') click_button 'Save' end @@ -358,21 +356,21 @@ end scenario 'User answers select dropdown option without a condition', :js do - create(:condition, question: @dropdown_conditional_question, - option_list: [@dropdown_conditional_question.question_options[1].id], + create(:condition, question: @conditional_questions[:dropdown], + option_list: [@conditional_questions[:dropdown].question_options[1].id], action_type: 'remove', - remove_data: [@textarea_questions[2].id, @textfield_questions[2].id, - @date_questions[2].id, @rda_metadata_questions[2].id, - @checkbox_questions[2].id, @dropdown_questions[2].id, - @multiselectbox_questions[2].id]) + remove_data: [@non_conditional_questions[:textarea][2].id, @non_conditional_questions[:textfield][2].id, + @non_conditional_questions[:date][2].id, @non_conditional_questions[:rda_metadata][2].id, + @non_conditional_questions[:checkbox][2].id, @non_conditional_questions[:dropdown][2].id, + @non_conditional_questions[:multiselectbox][2].id]) - create(:condition, question: @dropdown_conditional_question, - option_list: [@dropdown_conditional_question.question_options[4].id], + create(:condition, question: @conditional_questions[:dropdown], + option_list: [@conditional_questions[:dropdown].question_options[4].id], action_type: 'remove', - remove_data: [@textarea_questions[0].id, @textfield_questions[0].id, - @date_questions[0].id, @rda_metadata_questions[0].id, - @checkbox_questions[0].id, @dropdown_questions[0].id, - @multiselectbox_questions[0].id]) + remove_data: [@non_conditional_questions[:textarea][0].id, @non_conditional_questions[:textfield][0].id, + @non_conditional_questions[:date][0].id, @non_conditional_questions[:rda_metadata][0].id, + @non_conditional_questions[:checkbox][0].id, @non_conditional_questions[:dropdown][0].id, + @non_conditional_questions[:multiselectbox][0].id]) visit overview_plan_path(@plan) click_link 'Write Plan' @@ -385,8 +383,8 @@ expect(page).to have_text('24/27 answered') # Answer the dropdown_conditional_question. - within("#answer-form-#{@dropdown_conditional_question.id}") do - select(@dropdown_conditional_question.question_options[0].text, from: 'answer_question_option_ids') + within("#answer-form-#{@conditional_questions[:dropdown].id}") do + select(@conditional_questions[:dropdown].question_options[0].text, from: 'answer_question_option_ids') click_button 'Save' end @@ -399,8 +397,8 @@ end describe 'conditions with action_type add_webhook' do scenario 'User answers chooses checkbox option with a condition (with action_type: add_webhook)', :js do - condition = create(:condition, :webhook, question: @checkbox_conditional_question, - option_list: [@checkbox_conditional_question.question_options[2].id]) + condition = create(:condition, :webhook, question: @conditional_questions[:checkbox], + option_list: [@conditional_questions[:checkbox].question_options[2].id]) visit overview_plan_path(@plan) @@ -414,8 +412,8 @@ expect(page).to have_text('24/27 answered') # Answer the checkbox_conditional_question. - within("#answer-form-#{@checkbox_conditional_question.id}") do - check @checkbox_conditional_question.question_options[2].text + within("#answer-form-#{@conditional_questions[:checkbox].id}") do + check @conditional_questions[:checkbox].question_options[2].text end expect(page).to have_text('Answered just now') @@ -437,14 +435,14 @@ # To see structure of email sent see app/views/user_mailer/question_answered.html.erb. # Message should have @user.name, chosen option text and question text. expect(mail.body.encoded).to include(@user.name) - expect(mail.body.encoded).to include(@checkbox_conditional_question.question_options[2].text) - expect(mail.body.encoded).to include(@checkbox_conditional_question.text) + expect(mail.body.encoded).to include(@conditional_questions[:checkbox].question_options[2].text) + expect(mail.body.encoded).to include(@conditional_questions[:checkbox].text) end end scenario 'User answers chooses radiobutton option with a condition (with action_type: add_webhook)', :js do - condition = create(:condition, :webhook, question: @radiobutton_conditional_question, - option_list: [@radiobutton_conditional_question.question_options[0].id]) + condition = create(:condition, :webhook, question: @conditional_questions[:radiobutton], + option_list: [@conditional_questions[:radiobutton].question_options[0].id]) visit overview_plan_path(@plan) @@ -459,8 +457,8 @@ # Now for radiobutton_conditional_question answer, there in no unchoose option, # so we switch options to a different option without any conditions. - within("#answer-form-#{@radiobutton_conditional_question.id}") do - choose @radiobutton_conditional_question.question_options[0].text + within("#answer-form-#{@conditional_questions[:radiobutton].id}") do + choose @conditional_questions[:radiobutton].question_options[0].text end expect(page).to have_text('Answered just now') @@ -482,14 +480,14 @@ # To see structure of email sent see app/views/user_mailer/question_answered.html.erb. # Message should have @user.name, chosen option text and question text. expect(mail.body.encoded).to include(@user.name) - expect(mail.body.encoded).to include(@radiobutton_conditional_question.question_options[0].text) - expect(mail.body.encoded).to include(@radiobutton_conditional_question.text) + expect(mail.body.encoded).to include(@conditional_questions[:radiobutton].question_options[0].text) + expect(mail.body.encoded).to include(@conditional_questions[:radiobutton].text) end end scenario 'User answers chooses dropdown option with a condition (with action_type: add_webhook)', :js do - condition = create(:condition, :webhook, question: @dropdown_conditional_question, - option_list: [@dropdown_conditional_question.question_options[2].id]) + condition = create(:condition, :webhook, question: @conditional_questions[:dropdown], + option_list: [@conditional_questions[:dropdown].question_options[2].id]) visit overview_plan_path(@plan) @@ -503,8 +501,8 @@ expect(page).to have_text('24/27 answered') # Answer the dropdown_conditional_question - within("#answer-form-#{@dropdown_conditional_question.id}") do - select(@dropdown_conditional_question.question_options[2].text, from: 'answer_question_option_ids') + within("#answer-form-#{@conditional_questions[:dropdown].id}") do + select(@conditional_questions[:dropdown].question_options[2].text, from: 'answer_question_option_ids') end expect(page).to have_text('Answered just now') @@ -526,8 +524,8 @@ # To see structure of email sent see app/views/user_mailer/question_answered.html.erb. # Message should have @user.name, chosen option text and question text. expect(mail.body.encoded).to include(@user.name) - expect(mail.body.encoded).to include(@dropdown_conditional_question.question_options[2].text) - expect(mail.body.encoded).to include(@dropdown_conditional_question.text) + expect(mail.body.encoded).to include(@conditional_questions[:dropdown].question_options[2].text) + expect(mail.body.encoded).to include(@conditional_questions[:dropdown].text) end end end From 3f584859388650a5a6490b977a3cbb09c0b9a5b6 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Mon, 5 May 2025 15:43:57 -0600 Subject: [PATCH 03/17] Refactor assigning of answers --- ...troller_with_conditional_questions_spec.rb | 52 +++++-------------- .../questions/conditions_questions_spec.rb | 48 ++++------------- 2 files changed, 23 insertions(+), 77 deletions(-) diff --git a/spec/controllers/answers_controller_with_conditional_questions_spec.rb b/spec/controllers/answers_controller_with_conditional_questions_spec.rb index 3f37ddf307..98393acba6 100644 --- a/spec/controllers/answers_controller_with_conditional_questions_spec.rb +++ b/spec/controllers/answers_controller_with_conditional_questions_spec.rb @@ -33,44 +33,20 @@ @user = @plan.owner # Answer the questions in List2 - @textarea_answers = @non_conditional_questions[:textarea].each.map do |question| - create(:answer, plan: @plan, question: question, user: @user) - end - - @textfield_answers = @non_conditional_questions[:textfield].each.map do |question| - create(:answer, plan: @plan, question: question, user: @user) - end - - @date_answers = @non_conditional_questions[:date].each.map do |question| - create(:answer, plan: @plan, question: question, user: @user) - end - - @rda_metadata_answers = @non_conditional_questions[:rda_metadata].each.map do |question| - create(:answer, plan: @plan, question: question, user: @user) - end - - @checkbox_answers = @non_conditional_questions[:checkbox].each.map do |question| - create(:answer, plan: @plan, question: question, question_options: [question.question_options[2]], user: @user) - end - - @radiobuttons_answers = @non_conditional_questions[:radiobutton].each.map do |question| - create(:answer, plan: @plan, question: question, question_options: [question.question_options[2]], user: @user) - end - - @dropdown_answers = @non_conditional_questions[:dropdown].each.map do |question| - create(:answer, plan: @plan, question: question, question_options: [question.question_options[2]], user: @user) - end - - @multiselectbox_answers = @non_conditional_questions[:multiselectbox].each.map do |question| - create(:answer, plan: @plan, question: question, question_options: [question.question_options[2]], user: @user) + @answers = {} + @non_conditional_questions.each do |question_type, questions| + @answers[question_type] = questions.map do |question| + if %i[textarea textfield date rda_metadata].include?(question_type) + create(:answer, plan: @plan, question: question, user: @user) + else + create(:answer, plan: @plan, question: question, question_options: [question.question_options[2]], user: @user) + end + end end @all_questions_ids = (@conditional_questions.values + @non_conditional_questions.values.flatten).map(&:id) - @all_answers_ids = (@textarea_answers + @textfield_answers + - @date_answers + @rda_metadata_answers + - @checkbox_answers + @radiobuttons_answers + - @dropdown_answers + @multiselectbox_answers).map(&:id) + @all_answers_ids = @answers.values.flatten.map(&:id) sign_in(@user) end @@ -118,10 +94,10 @@ # Check Answers in database (persisted). Expect removed answers to be destroyed. # Answers destroyed eare easier checked using array of ids rather than individually as in example - # expect(Answer.exists?(@textarea_answers[5].id)).to be_falsey. - removed_answers = [@textarea_answers[5].id, @textfield_answers[5].id, - @date_answers[5].id, @rda_metadata_answers[5].id, @checkbox_answers[5].id, - @radiobuttons_answers[5].id, @dropdown_answers[5].id, @multiselectbox_answers[5].id] + # expect(Answer.exists?(@answers[:textarea][5].id)).to be_falsey. + removed_answers = [@answers[:textarea][5].id, @answers[:textfield][5].id, + @answers[:date][5].id, @answers[:rda_metadata][5].id, @answers[:checkbox][5].id, + @answers[:radiobutton][5].id, @answers[:dropdown][5].id, @answers[:multiselectbox][5].id] expect(Answer.where(id: removed_answers).pluck(:id)).to be_empty # Answers left expect(Answer.where(id: @all_answers_ids).pluck(:id)).to match_array( diff --git a/spec/features/questions/conditions_questions_spec.rb b/spec/features/questions/conditions_questions_spec.rb index 26aa9e775b..4ace22b67b 100644 --- a/spec/features/questions/conditions_questions_spec.rb +++ b/spec/features/questions/conditions_questions_spec.rb @@ -37,46 +37,16 @@ @all_questions_ids = (@conditional_questions.values + @non_conditional_questions.values.flatten).map(&:id) # Answer the non-conditional questions - @textarea_answers = @non_conditional_questions[:textarea].each.map do |question| - create(:answer, plan: @plan, question: question, user: @user) - end - - @all_non_conditional_question_answers_ids = @textarea_answers.map(&:id) - - @textfield_answers = @non_conditional_questions[:textfield].each.map do |question| - create(:answer, plan: @plan, question: question, user: @user) - end - @all_non_conditional_question_answers_ids += @textfield_answers.map(&:id) - - @date_answers = @non_conditional_questions[:date].each.map do |question| - create(:answer, plan: @plan, question: question, user: @user) - end - @all_non_conditional_question_answers_ids += @date_answers.map(&:id) - - @rda_metadata_answers = @non_conditional_questions[:rda_metadata].each.map do |question| - create(:answer, plan: @plan, question: question, question_options: [question.question_options[2]], user: @user) - end - @all_non_conditional_question_answers_ids += @rda_metadata_answers.map(&:id) - - @checkbox_answers = @non_conditional_questions[:checkbox].each.map do |question| - create(:answer, plan: @plan, question: question, question_options: [question.question_options[2]], user: @user) - end - @all_non_conditional_question_answers_ids += @checkbox_answers.map(&:id) - - @radiobuttons_answers = @non_conditional_questions[:radiobutton].each.map do |question| - create(:answer, plan: @plan, question: question, question_options: [question.question_options[2]], user: @user) - end - @all_non_conditional_question_answers_ids += @radiobuttons_answers.map(&:id) - - @dropdown_answers = @non_conditional_questions[:dropdown].each.map do |question| - create(:answer, plan: @plan, question: question, question_options: [question.question_options[2]], user: @user) - end - @all_non_conditional_question_answers_ids += @dropdown_answers.map(&:id) - - @multiselectbox_answers = @non_conditional_questions[:multiselectbox].each.map do |question| - create(:answer, plan: @plan, question: question, question_options: [question.question_options[2]], user: @user) + @non_conditional_questions.each do |question_type, questions| + questions.map do |question| + if %i[textarea textfield date rda_metadata].include?(question_type) + create(:answer, plan: @plan, question: question, user: @user) + else + create(:answer, plan: @plan, question: question, question_options: [question.question_options[2]], + user: @user) + end + end end - @all_non_conditional_question_answers_ids += @multiselectbox_answers.map(&:id) sign_in(@user) From 19f9f5afd5ea87e0a43c58aefef71ce14f406f5c Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Tue, 6 May 2025 12:39:22 -0600 Subject: [PATCH 04/17] Refactor / add helper methods --- ...troller_with_conditional_questions_spec.rb | 67 ++++++++++++------- .../questions/conditions_questions_spec.rb | 64 +++++++++++------- 2 files changed, 80 insertions(+), 51 deletions(-) diff --git a/spec/controllers/answers_controller_with_conditional_questions_spec.rb b/spec/controllers/answers_controller_with_conditional_questions_spec.rb index 98393acba6..d9bfe0f448 100644 --- a/spec/controllers/answers_controller_with_conditional_questions_spec.rb +++ b/spec/controllers/answers_controller_with_conditional_questions_spec.rb @@ -11,41 +11,18 @@ @section1, @section2, @section3 = template.sections # Different types of questions (than can have conditional options) - @conditional_questions = { - checkbox: create(:question, :checkbox, section: @section1, options: 5), - radiobutton: create(:question, :radiobuttons, section: @section2, options: 5), - dropdown: create(:question, :dropdown, section: @section3, options: 5) - } + @conditional_questions = create_conditional_questions # Questions that do not have conditional options for adding or removing - @non_conditional_questions = { - textarea: create_list(:question, 7, :textarea, section: @section1), - textfield: create_list(:question, 7, :textfield, section: @section2), - date: create_list(:question, 7, :date, section: @section3), - rda_metadata: create_list(:question, 7, :rda_metadata, section: @section1, options: 3), - checkbox: create_list(:question, 7, :checkbox, section: @section2, options: 3), - radiobutton: create_list(:question, 7, :radiobuttons, section: @section3, options: 3), - dropdown: create_list(:question, 7, :dropdown, section: @section1, options: 3), - multiselectbox: create_list(:question, 7, :multiselectbox, section: @section2, options: 3) - } + @non_conditional_questions = create_non_conditional_questions @plan = create(:plan, :creator, template: template) @user = @plan.owner # Answer the questions in List2 - @answers = {} - @non_conditional_questions.each do |question_type, questions| - @answers[question_type] = questions.map do |question| - if %i[textarea textfield date rda_metadata].include?(question_type) - create(:answer, plan: @plan, question: question, user: @user) - else - create(:answer, plan: @plan, question: question, question_options: [question.question_options[2]], user: @user) - end - end - end + @answers = create_answers @all_questions_ids = (@conditional_questions.values + @non_conditional_questions.values.flatten).map(&:id) - @all_answers_ids = @answers.values.flatten.map(&:id) sign_in(@user) @@ -509,4 +486,42 @@ end end end + + private + + def create_conditional_questions + { + checkbox: create(:question, :checkbox, section: @section1, options: 5), + radiobutton: create(:question, :radiobuttons, section: @section2, options: 5), + dropdown: create(:question, :dropdown, section: @section3, options: 5) + } + end + + def create_non_conditional_questions + { + textarea: create_list(:question, 7, :textarea, section: @section1), + textfield: create_list(:question, 7, :textfield, section: @section2), + date: create_list(:question, 7, :date, section: @section3), + rda_metadata: create_list(:question, 7, :rda_metadata, section: @section1, options: 3), + checkbox: create_list(:question, 7, :checkbox, section: @section2, options: 3), + radiobutton: create_list(:question, 7, :radiobuttons, section: @section3, options: 3), + dropdown: create_list(:question, 7, :dropdown, section: @section1, options: 3), + multiselectbox: create_list(:question, 7, :multiselectbox, section: @section2, options: 3) + } + end + + def create_answers + question_types_with_question_options = %i[checkbox radiobutton dropdown multiselectbox] + answers = {} + @non_conditional_questions.each do |question_type, questions| + answers[question_type] = questions.map do |question| + if question_types_with_question_options.include?(question_type) + create(:answer, plan: @plan, question: question, question_options: [question.question_options[2]], user: @user) + else + create(:answer, plan: @plan, question: question, user: @user) + end + end + end + answers + end end diff --git a/spec/features/questions/conditions_questions_spec.rb b/spec/features/questions/conditions_questions_spec.rb index 4ace22b67b..ad43b6057b 100644 --- a/spec/features/questions/conditions_questions_spec.rb +++ b/spec/features/questions/conditions_questions_spec.rb @@ -14,39 +14,17 @@ @section3 = create(:section, phase: @phase) # Different types of questions (than can have conditional options) - @conditional_questions = { - checkbox: create(:question, :checkbox, section: @section1, options: 5), - radiobutton: create(:question, :radiobuttons, section: @section2, options: 5), - dropdown: create(:question, :dropdown, section: @section3, options: 5) - } + @conditional_questions = create_conditional_questions # Questions that do not have conditional options for adding or removing - @non_conditional_questions = { - textarea: create_list(:question, 3, :textarea, section: @section1), - textfield: create_list(:question, 3, :textfield, section: @section2), - date: create_list(:question, 3, :date, section: @section3), - rda_metadata: create_list(:question, 3, :rda_metadata, section: @section1, options: 5), - checkbox: create_list(:question, 3, :checkbox, section: @section2, options: 5), - radiobutton: create_list(:question, 3, :radiobuttons, section: @section3, options: 5), - dropdown: create_list(:question, 3, :dropdown, section: @section1, options: 5), - multiselectbox: create_list(:question, 3, :multiselectbox, section: @section2, options: 5) - } + @non_conditional_questions = create_non_conditional_questions create(:role, :creator, :editor, :commenter, user: @user, plan: @plan) @all_questions_ids = (@conditional_questions.values + @non_conditional_questions.values.flatten).map(&:id) # Answer the non-conditional questions - @non_conditional_questions.each do |question_type, questions| - questions.map do |question| - if %i[textarea textfield date rda_metadata].include?(question_type) - create(:answer, plan: @plan, question: question, user: @user) - else - create(:answer, plan: @plan, question: question, question_options: [question.question_options[2]], - user: @user) - end - end - end + create_answers sign_in(@user) @@ -499,4 +477,40 @@ end end end + + private + + def create_conditional_questions + { + checkbox: create(:question, :checkbox, section: @section1, options: 5), + radiobutton: create(:question, :radiobuttons, section: @section2, options: 5), + dropdown: create(:question, :dropdown, section: @section3, options: 5) + } + end + + def create_non_conditional_questions + { + textarea: create_list(:question, 3, :textarea, section: @section1), + textfield: create_list(:question, 3, :textfield, section: @section2), + date: create_list(:question, 3, :date, section: @section3), + rda_metadata: create_list(:question, 3, :rda_metadata, section: @section1, options: 5), + checkbox: create_list(:question, 3, :checkbox, section: @section2, options: 5), + radiobutton: create_list(:question, 3, :radiobuttons, section: @section3, options: 5), + dropdown: create_list(:question, 3, :dropdown, section: @section1, options: 5), + multiselectbox: create_list(:question, 3, :multiselectbox, section: @section2, options: 5) + } + end + + def create_answers + @non_conditional_questions.each do |question_type, questions| + questions.map do |question| + if %i[textarea textfield date rda_metadata].include?(question_type) + create(:answer, plan: @plan, question: question, user: @user) + else + create(:answer, plan: @plan, question: question, question_options: [question.question_options[2]], + user: @user) + end + end + end + end end From 30764c14c8b4bac8bf982be9092c564fb6dad4f7 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Mon, 5 May 2025 17:11:43 -0600 Subject: [PATCH 05/17] Refactor `remove_data` handling & add radiobutton -I'm not sure why, but the `:radio_button` type was missing from several remove_list: values. This change adds it to all of the remove_list arrays. --- ...troller_with_conditional_questions_spec.rb | 68 +++++-------------- .../questions/conditions_questions_spec.rb | 34 +++------- 2 files changed, 27 insertions(+), 75 deletions(-) diff --git a/spec/controllers/answers_controller_with_conditional_questions_spec.rb b/spec/controllers/answers_controller_with_conditional_questions_spec.rb index d9bfe0f448..9d1a6bff38 100644 --- a/spec/controllers/answers_controller_with_conditional_questions_spec.rb +++ b/spec/controllers/answers_controller_with_conditional_questions_spec.rb @@ -43,10 +43,7 @@ condition = create(:condition, question: @conditional_questions[:checkbox], option_list: [@conditional_questions[:checkbox].question_options[2].id], action_type: 'remove', - remove_data: [@non_conditional_questions[:textarea][5].id, @non_conditional_questions[:textfield][5].id, - @non_conditional_questions[:date][5].id, @non_conditional_questions[:rda_metadata][5].id, - @non_conditional_questions[:checkbox][5].id, @non_conditional_questions[:radiobutton][5].id, - @non_conditional_questions[:dropdown][5].id, @non_conditional_questions[:multiselectbox][5].id]) + remove_data: non_conditional_questions_ids_by_index(5)) # We chose an option that is in the option_list of the condition defined above. Note that # the text sent by UI is an empty string. @@ -72,9 +69,7 @@ # Check Answers in database (persisted). Expect removed answers to be destroyed. # Answers destroyed eare easier checked using array of ids rather than individually as in example # expect(Answer.exists?(@answers[:textarea][5].id)).to be_falsey. - removed_answers = [@answers[:textarea][5].id, @answers[:textfield][5].id, - @answers[:date][5].id, @answers[:rda_metadata][5].id, @answers[:checkbox][5].id, - @answers[:radiobutton][5].id, @answers[:dropdown][5].id, @answers[:multiselectbox][5].id] + removed_answers = @answers.map { |_, answers| answers[5].id } expect(Answer.where(id: removed_answers).pluck(:id)).to be_empty # Answers left expect(Answer.where(id: @all_answers_ids).pluck(:id)).to match_array( @@ -85,18 +80,12 @@ create(:condition, question: @conditional_questions[:checkbox], option_list: [@conditional_questions[:checkbox].question_options[1].id], action_type: 'remove', - remove_data: [@non_conditional_questions[:textarea][3].id, @non_conditional_questions[:textfield][3].id, - @non_conditional_questions[:date][3].id, @non_conditional_questions[:rda_metadata][3].id, - @non_conditional_questions[:checkbox][3].id, @non_conditional_questions[:dropdown][3].id, - @non_conditional_questions[:multiselectbox][3].id]) + remove_data: non_conditional_questions_ids_by_index(3)) create(:condition, question: @conditional_questions[:checkbox], option_list: [@conditional_questions[:checkbox].question_options[4].id], action_type: 'remove', - remove_data: [@non_conditional_questions[:textarea][0].id, @non_conditional_questions[:textfield][0].id, - @non_conditional_questions[:date][0].id, @non_conditional_questions[:rda_metadata][0].id, - @non_conditional_questions[:checkbox][0].id, @non_conditional_questions[:dropdown][0].id, - @non_conditional_questions[:multiselectbox][0].id]) + remove_data: non_conditional_questions_ids_by_index(0)) # We choose an option that is not in the option_list of the conditions defined above. args = { @@ -119,18 +108,12 @@ condition1 = create(:condition, question: @conditional_questions[:checkbox], option_list: [@conditional_questions[:checkbox].question_options[2].id], action_type: 'remove', - remove_data: [@non_conditional_questions[:textarea][0].id, @non_conditional_questions[:textfield][0].id, - @non_conditional_questions[:date][0].id, @non_conditional_questions[:rda_metadata][0].id, - @non_conditional_questions[:checkbox][0].id, @non_conditional_questions[:dropdown][0].id, - @non_conditional_questions[:multiselectbox][0].id]) + remove_data: non_conditional_questions_ids_by_index(0)) condition2 = create(:condition, question: @conditional_questions[:checkbox], option_list: [@conditional_questions[:checkbox].question_options[4].id], action_type: 'remove', - remove_data: [@non_conditional_questions[:textarea][3].id, @non_conditional_questions[:textfield][3].id, - @non_conditional_questions[:date][3].id, @non_conditional_questions[:rda_metadata][3].id, - @non_conditional_questions[:checkbox][3].id, @non_conditional_questions[:dropdown][3].id, - @non_conditional_questions[:multiselectbox][3].id]) + remove_data: non_conditional_questions_ids_by_index(3)) # We choose options that is in the option_list of the conditions defined above as well as an option # with no condition defined. @@ -160,10 +143,7 @@ condition = create(:condition, question: @conditional_questions[:radiobutton], option_list: [@conditional_questions[:radiobutton].question_options[2].id], action_type: 'remove', - remove_data: [@non_conditional_questions[:textarea][5].id, @non_conditional_questions[:textfield][5].id, - @non_conditional_questions[:date][5].id, @non_conditional_questions[:rda_metadata][5].id, - @non_conditional_questions[:checkbox][5].id, @non_conditional_questions[:radiobutton][5].id, - @non_conditional_questions[:dropdown][5].id, @non_conditional_questions[:multiselectbox][5].id]) + remove_data: non_conditional_questions_ids_by_index(5)) # We choose an option that is in the option_list of the condition defined above. args = { @@ -187,18 +167,12 @@ create(:condition, question: @conditional_questions[:radiobutton], option_list: [@conditional_questions[:radiobutton].question_options[1].id], action_type: 'remove', - remove_data: [@non_conditional_questions[:textarea][3].id, @non_conditional_questions[:textfield][3].id, - @non_conditional_questions[:date][3].id, @non_conditional_questions[:rda_metadata][3].id, - @non_conditional_questions[:checkbox][3].id, @non_conditional_questions[:dropdown][3].id, - @non_conditional_questions[:multiselectbox][3].id]) + remove_data: non_conditional_questions_ids_by_index(3)) create(:condition, question: @conditional_questions[:radiobutton], option_list: [@conditional_questions[:radiobutton].question_options[4].id], action_type: 'remove', - remove_data: [@non_conditional_questions[:textarea][0].id, @non_conditional_questions[:textfield][0].id, - @non_conditional_questions[:date][0].id, @non_conditional_questions[:rda_metadata][0].id, - @non_conditional_questions[:checkbox][0].id, @non_conditional_questions[:dropdown][0].id, - @non_conditional_questions[:multiselectbox][0].id]) + remove_data: non_conditional_questions_ids_by_index(0)) # We choose an option that is not in the option_list of the conditions defined above. args = { @@ -224,10 +198,7 @@ condition = create(:condition, question: @conditional_questions[:dropdown], option_list: [@conditional_questions[:dropdown].question_options[2].id], action_type: 'remove', - remove_data: [@non_conditional_questions[:textarea][5].id, @non_conditional_questions[:textfield][5].id, - @non_conditional_questions[:date][5].id, @non_conditional_questions[:rda_metadata][5].id, - @non_conditional_questions[:checkbox][5].id, @non_conditional_questions[:radiobutton][5].id, - @non_conditional_questions[:dropdown][5].id, @non_conditional_questions[:multiselectbox][5].id]) + remove_data: non_conditional_questions_ids_by_index(5)) # We chose an option that is in the option_list of the condition defined above. args = { @@ -251,18 +222,12 @@ create(:condition, question: @conditional_questions[:dropdown], option_list: [@conditional_questions[:dropdown].question_options[1].id], action_type: 'remove', - remove_data: [@non_conditional_questions[:textarea][3].id, @non_conditional_questions[:textfield][3].id, - @non_conditional_questions[:date][3].id, @non_conditional_questions[:rda_metadata][3].id, - @non_conditional_questions[:checkbox][3].id, @non_conditional_questions[:dropdown][3].id, - @non_conditional_questions[:multiselectbox][3].id]) + remove_data: non_conditional_questions_ids_by_index(3)) create(:condition, question: @conditional_questions[:dropdown], option_list: [@conditional_questions[:dropdown].question_options[4].id], action_type: 'remove', - remove_data: [@non_conditional_questions[:textarea][0].id, @non_conditional_questions[:textfield][0].id, - @non_conditional_questions[:date][0].id, @non_conditional_questions[:rda_metadata][0].id, - @non_conditional_questions[:checkbox][0].id, @non_conditional_questions[:dropdown][0].id, - @non_conditional_questions[:multiselectbox][0].id]) + remove_data: non_conditional_questions_ids_by_index(0)) # We choose an option that is not in the option_list of the conditions defined above. args = { @@ -345,10 +310,7 @@ condition2 = create(:condition, question: @conditional_questions[:checkbox], option_list: [@conditional_questions[:checkbox].question_options[4].id], action_type: 'remove', - remove_data: [@non_conditional_questions[:textarea][3].id, @non_conditional_questions[:textfield][3].id, - @non_conditional_questions[:date][3].id, @non_conditional_questions[:rda_metadata][3].id, - @non_conditional_questions[:checkbox][3].id, @non_conditional_questions[:dropdown][3].id, - @non_conditional_questions[:multiselectbox][3].id]) + remove_data: non_conditional_questions_ids_by_index(3)) # We chose an option that is in the option_list of the condition defined above. Note that # the text sent by UI is an empty string. @@ -510,6 +472,10 @@ def create_non_conditional_questions } end + def non_conditional_questions_ids_by_index(index) + @non_conditional_questions.map { |_, questions| questions[index].id } + end + def create_answers question_types_with_question_options = %i[checkbox radiobutton dropdown multiselectbox] answers = {} diff --git a/spec/features/questions/conditions_questions_spec.rb b/spec/features/questions/conditions_questions_spec.rb index ad43b6057b..a866c4efb8 100644 --- a/spec/features/questions/conditions_questions_spec.rb +++ b/spec/features/questions/conditions_questions_spec.rb @@ -101,18 +101,12 @@ create(:condition, question: @conditional_questions[:checkbox], option_list: [@conditional_questions[:checkbox].question_options[1].id], action_type: 'remove', - remove_data: [@non_conditional_questions[:textarea][2].id, @non_conditional_questions[:textfield][2].id, - @non_conditional_questions[:date][2].id, @non_conditional_questions[:rda_metadata][2].id, - @non_conditional_questions[:checkbox][2].id, @non_conditional_questions[:dropdown][2].id, - @non_conditional_questions[:multiselectbox][2].id]) + remove_data: non_conditional_questions_ids_by_index(2)) create(:condition, question: @conditional_questions[:checkbox], option_list: [@conditional_questions[:checkbox].question_options[4].id], action_type: 'remove', - remove_data: [@non_conditional_questions[:textarea][0].id, @non_conditional_questions[:textfield][0].id, - @non_conditional_questions[:date][0].id, @non_conditional_questions[:rda_metadata][0].id, - @non_conditional_questions[:checkbox][0].id, @non_conditional_questions[:dropdown][0].id, - @non_conditional_questions[:multiselectbox][0].id]) + remove_data: non_conditional_questions_ids_by_index(0)) # We choose an option that is not in the option_list of the conditions defined above. visit overview_plan_path(@plan) @@ -205,18 +199,12 @@ create(:condition, question: @conditional_questions[:radiobutton], option_list: [@conditional_questions[:radiobutton].question_options[1].id], action_type: 'remove', - remove_data: [@non_conditional_questions[:textarea][2].id, @non_conditional_questions[:textfield][2].id, - @non_conditional_questions[:date][2].id, @non_conditional_questions[:rda_metadata][2].id, - @non_conditional_questions[:checkbox][2].id, @non_conditional_questions[:dropdown][2].id, - @non_conditional_questions[:multiselectbox][2].id]) + remove_data: non_conditional_questions_ids_by_index(2)) create(:condition, question: @conditional_questions[:radiobutton], option_list: [@conditional_questions[:radiobutton].question_options[4].id], action_type: 'remove', - remove_data: [@non_conditional_questions[:textarea][0].id, @non_conditional_questions[:textfield][0].id, - @non_conditional_questions[:date][0].id, @non_conditional_questions[:rda_metadata][0].id, - @non_conditional_questions[:checkbox][0].id, @non_conditional_questions[:dropdown][0].id, - @non_conditional_questions[:multiselectbox][0].id]) + remove_data: non_conditional_questions_ids_by_index(0)) # We choose an option that is not in the option_list of the conditions defined above. visit overview_plan_path(@plan) @@ -307,18 +295,12 @@ create(:condition, question: @conditional_questions[:dropdown], option_list: [@conditional_questions[:dropdown].question_options[1].id], action_type: 'remove', - remove_data: [@non_conditional_questions[:textarea][2].id, @non_conditional_questions[:textfield][2].id, - @non_conditional_questions[:date][2].id, @non_conditional_questions[:rda_metadata][2].id, - @non_conditional_questions[:checkbox][2].id, @non_conditional_questions[:dropdown][2].id, - @non_conditional_questions[:multiselectbox][2].id]) + remove_data: non_conditional_questions_ids_by_index(2)) create(:condition, question: @conditional_questions[:dropdown], option_list: [@conditional_questions[:dropdown].question_options[4].id], action_type: 'remove', - remove_data: [@non_conditional_questions[:textarea][0].id, @non_conditional_questions[:textfield][0].id, - @non_conditional_questions[:date][0].id, @non_conditional_questions[:rda_metadata][0].id, - @non_conditional_questions[:checkbox][0].id, @non_conditional_questions[:dropdown][0].id, - @non_conditional_questions[:multiselectbox][0].id]) + remove_data: non_conditional_questions_ids_by_index(0)) visit overview_plan_path(@plan) click_link 'Write Plan' @@ -513,4 +495,8 @@ def create_answers end end end + + def non_conditional_questions_ids_by_index(index) + @non_conditional_questions.map { |_, questions| questions[index].id } + end end From c5227b885ffa832aed3e480dede837617872b3de Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Tue, 6 May 2025 11:24:43 -0600 Subject: [PATCH 06/17] Refactor expect() checks for delivered mail --- ...troller_with_conditional_questions_spec.rb | 62 ++++++------------- .../questions/conditions_questions_spec.rb | 47 +++++--------- 2 files changed, 35 insertions(+), 74 deletions(-) diff --git a/spec/controllers/answers_controller_with_conditional_questions_spec.rb b/spec/controllers/answers_controller_with_conditional_questions_spec.rb index 9d1a6bff38..507ffa6883 100644 --- a/spec/controllers/answers_controller_with_conditional_questions_spec.rb +++ b/spec/controllers/answers_controller_with_conditional_questions_spec.rb @@ -289,17 +289,7 @@ expect(ActionMailer::Base.deliveries.count).to eq(1) webhook_data = JSON.parse(add_webhook_condition.webhook_data) - ActionMailer::Base.deliveries.first do |mail| - expect(mail.to).to eq([webhook_data['email']]) - expect(mail.subject).to eq(webhook_data['subject']) - expect(mail.body.encoded).to include(webhook_data['message']) - # To see structure of email sent see app/views/user_mailer/question_answered.html.erb. - - # Message should have @user.name, chosen option text and question text. - expect(mail.body.encoded).to include(@user.name) - expect(mail.body.encoded).to include(@conditional_questions[:checkbox].question_options[2].text) - expect(mail.body.encoded).to include(@conditional_questions[:checkbox].text) - end + check_delivered_mail_for_webhook_data_and_question_data(webhook_data, :checkbox) end it 'handles multiple checkbox options (one of which is add_webhook condition)' do add_webhook_condition = create(:condition, @@ -342,17 +332,7 @@ expect(ActionMailer::Base.deliveries.count).to eq(1) webhook_data = JSON.parse(add_webhook_condition.webhook_data) - ActionMailer::Base.deliveries.first do |mail| - expect(mail.to).to eq([webhook_data['email']]) - expect(mail.subject).to eq(webhook_data['subject']) - expect(mail.body.encoded).to include(webhook_data['message']) - # To see structure of email sent see app/views/user_mailer/question_answered.html.erb. - - # Message should have @user.name, chosen option text and question text. - expect(mail.body.encoded).to include(@user.name) - expect(mail.body.encoded).to include(@conditional_questions[:checkbox].question_options[2].text) - expect(mail.body.encoded).to include(@conditional_questions[:checkbox].text) - end + check_delivered_mail_for_webhook_data_and_question_data(webhook_data, :checkbox) end it 'handles selection of a dropdown option (with add_webhook condition)' do @@ -388,17 +368,7 @@ expect(ActionMailer::Base.deliveries.count).to eq(1) webhook_data = JSON.parse(add_webhook_condition.webhook_data) - ActionMailer::Base.deliveries.first do |mail| - expect(mail.to).to eq([webhook_data['email']]) - expect(mail.subject).to eq(webhook_data['subject']) - expect(mail.body.encoded).to include(webhook_data['message']) - # To see structure of email sent see app/views/user_mailer/question_answered.html.erb. - - # Message should have @user.name, chosen option text and question text. - expect(mail.body.encoded).to include(@user.name) - expect(mail.body.encoded).to include(@conditional_questions[:dropdown].question_options[2].text) - expect(mail.body.encoded).to include(@conditional_questions[:dropdown].text) - end + check_delivered_mail_for_webhook_data_and_question_data(webhook_data, :dropdown) end it 'handles selection of a radiobutton option (with add_webhook condition)' do @@ -434,17 +404,7 @@ expect(ActionMailer::Base.deliveries.count).to eq(1) webhook_data = JSON.parse(add_webhook_condition.webhook_data) - ActionMailer::Base.deliveries.first do |mail| - expect(mail.to).to eq([webhook_data['email']]) - expect(mail.subject).to eq(webhook_data['subject']) - expect(mail.body.encoded).to include(webhook_data['message']) - # To see structure of email sent see app/views/user_mailer/question_answered.html.erb. - - # Message should have @user.name, chosen option text and question text. - expect(mail.body.encoded).to include(@user.name) - expect(mail.body.encoded).to include(@conditional_questions[:radiobutton].question_options[2].text) - expect(mail.body.encoded).to include(@conditional_questions[:radiobutton].text) - end + check_delivered_mail_for_webhook_data_and_question_data(webhook_data, :radiobutton) end end end @@ -490,4 +450,18 @@ def create_answers end answers end + + def check_delivered_mail_for_webhook_data_and_question_data(webhook_data, question_type) + ActionMailer::Base.deliveries.first do |mail| + expect(mail.to).to eq([webhook_data['email']]) + expect(mail.subject).to eq(webhook_data['subject']) + expect(mail.body.encoded).to include(webhook_data['message']) + # To see structure of email sent see app/views/user_mailer/question_answered.html.erb. + + # Message should have @user.name, chosen option text and question text. + expect(mail.body.encoded).to include(@user.name) + expect(mail.body.encoded).to include(@conditional_questions[question_type].question_options[2].text) + expect(mail.body.encoded).to include(@conditional_questions[question_type].text) + end + end end diff --git a/spec/features/questions/conditions_questions_spec.rb b/spec/features/questions/conditions_questions_spec.rb index a866c4efb8..080c56657c 100644 --- a/spec/features/questions/conditions_questions_spec.rb +++ b/spec/features/questions/conditions_questions_spec.rb @@ -358,16 +358,7 @@ expect(ActionMailer::Base.deliveries.count).to eq(1) webhook_data = JSON.parse(condition.webhook_data) - ActionMailer::Base.deliveries.last do |mail| - expect(mail.to).to eq([webhook_data['email']]) - expect(mail.subject).to eq(webhook_data['subject']) - expect(mail.body.encoded).to include(webhook_data['message']) - # To see structure of email sent see app/views/user_mailer/question_answered.html.erb. - # Message should have @user.name, chosen option text and question text. - expect(mail.body.encoded).to include(@user.name) - expect(mail.body.encoded).to include(@conditional_questions[:checkbox].question_options[2].text) - expect(mail.body.encoded).to include(@conditional_questions[:checkbox].text) - end + check_delivered_mail_for_webhook_data_and_question_data(webhook_data, :checkbox) end scenario 'User answers chooses radiobutton option with a condition (with action_type: add_webhook)', :js do @@ -403,16 +394,7 @@ expect(ActionMailer::Base.deliveries.count).to eq(1) webhook_data = JSON.parse(condition.webhook_data) - ActionMailer::Base.deliveries.last do |mail| - expect(mail.to).to eq([webhook_data['email']]) - expect(mail.subject).to eq(webhook_data['subject']) - expect(mail.body.encoded).to include(webhook_data['message']) - # To see structure of email sent see app/views/user_mailer/question_answered.html.erb. - # Message should have @user.name, chosen option text and question text. - expect(mail.body.encoded).to include(@user.name) - expect(mail.body.encoded).to include(@conditional_questions[:radiobutton].question_options[0].text) - expect(mail.body.encoded).to include(@conditional_questions[:radiobutton].text) - end + check_delivered_mail_for_webhook_data_and_question_data(webhook_data, :radiobutton) end scenario 'User answers chooses dropdown option with a condition (with action_type: add_webhook)', :js do @@ -447,16 +429,7 @@ expect(ActionMailer::Base.deliveries.count).to eq(1) webhook_data = JSON.parse(condition.webhook_data) - ActionMailer::Base.deliveries.last do |mail| - expect(mail.to).to eq([webhook_data['email']]) - expect(mail.subject).to eq(webhook_data['subject']) - expect(mail.body.encoded).to include(webhook_data['message']) - # To see structure of email sent see app/views/user_mailer/question_answered.html.erb. - # Message should have @user.name, chosen option text and question text. - expect(mail.body.encoded).to include(@user.name) - expect(mail.body.encoded).to include(@conditional_questions[:dropdown].question_options[2].text) - expect(mail.body.encoded).to include(@conditional_questions[:dropdown].text) - end + check_delivered_mail_for_webhook_data_and_question_data(webhook_data, :dropdown) end end @@ -499,4 +472,18 @@ def create_answers def non_conditional_questions_ids_by_index(index) @non_conditional_questions.map { |_, questions| questions[index].id } end + + def check_delivered_mail_for_webhook_data_and_question_data(webhook_data, question_type) + ActionMailer::Base.deliveries.first do |mail| + expect(mail.to).to eq([webhook_data['email']]) + expect(mail.subject).to eq(webhook_data['subject']) + expect(mail.body.encoded).to include(webhook_data['message']) + # To see structure of email sent see app/views/user_mailer/question_answered.html.erb. + + # Message should have @user.name, chosen option text and question text. + expect(mail.body.encoded).to include(@user.name) + expect(mail.body.encoded).to include(@conditional_questions[question_type].question_options[2].text) + expect(mail.body.encoded).to include(@conditional_questions[question_type].text) + end + end end From bee170aa63d9c48f62650b5ac46bb2bccca3db70 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Tue, 6 May 2025 11:46:46 -0600 Subject: [PATCH 07/17] Refactor questions to show/hide logic --- ...troller_with_conditional_questions_spec.rb | 63 ++++++------------- 1 file changed, 18 insertions(+), 45 deletions(-) diff --git a/spec/controllers/answers_controller_with_conditional_questions_spec.rb b/spec/controllers/answers_controller_with_conditional_questions_spec.rb index 507ffa6883..2be290c73b 100644 --- a/spec/controllers/answers_controller_with_conditional_questions_spec.rb +++ b/spec/controllers/answers_controller_with_conditional_questions_spec.rb @@ -61,10 +61,7 @@ json = JSON.parse(response.body).with_indifferent_access # Check hide/show questions lists sent to frontend. - expected_to_show_question_ids = @all_questions_ids - condition.remove_data - expected_to_hide_question_ids = condition.remove_data - expect(json[:qn_data][:to_show]).to match_array(expected_to_show_question_ids) - expect(json[:qn_data][:to_hide]).to match_array(expected_to_hide_question_ids) + check_question_ids_to_show_and_hide(json, condition.remove_data) # Check Answers in database (persisted). Expect removed answers to be destroyed. # Answers destroyed eare easier checked using array of ids rather than individually as in example @@ -100,8 +97,7 @@ post :create_or_update, params: { answer: args } json = JSON.parse(response.body).with_indifferent_access - expect(json[:qn_data][:to_show]).to match_array(@all_questions_ids) - expect(json[:qn_data][:to_hide]).to match_array([]) + check_question_ids_to_show_and_hide(json) end it 'handles multiple options (some with conditions) in option_list' do @@ -130,11 +126,8 @@ post :create_or_update, params: { answer: args } json = JSON.parse(response.body).with_indifferent_access - - expected_to_show_question_ids = @all_questions_ids - condition1.remove_data - condition2.remove_data - expected_to_hide_question_ids = condition1.remove_data + condition2.remove_data - expect(json[:qn_data][:to_show]).to match_array(expected_to_show_question_ids) - expect(json[:qn_data][:to_hide]).to match_array(expected_to_hide_question_ids) + remove_data = condition1.remove_data + condition2.remove_data + check_question_ids_to_show_and_hide(json, remove_data) end end # Note: radiobuttons only allow single selection. @@ -158,10 +151,7 @@ post :create_or_update, params: { answer: args } json = JSON.parse(response.body).with_indifferent_access - expected_to_show_question_ids = @all_questions_ids - condition.remove_data - expected_to_hide_question_ids = condition.remove_data - expect(json[:qn_data][:to_show]).to match_array(expected_to_show_question_ids) - expect(json[:qn_data][:to_hide]).to match_array(expected_to_hide_question_ids) + check_question_ids_to_show_and_hide(json, condition.remove_data) end it 'handles single option (without condition) in option_list' do create(:condition, question: @conditional_questions[:radiobutton], @@ -187,8 +177,7 @@ post :create_or_update, params: { answer: args } json = JSON.parse(response.body).with_indifferent_access - expect(json[:qn_data][:to_show]).to match_array(@all_questions_ids) - expect(json[:qn_data][:to_hide]).to match_array([]) + check_question_ids_to_show_and_hide(json) end end @@ -213,10 +202,7 @@ post :create_or_update, params: { answer: args } json = JSON.parse(response.body).with_indifferent_access - expected_to_show_question_ids = @all_questions_ids - condition.remove_data - expected_to_hide_question_ids = condition.remove_data - expect(json[:qn_data][:to_show]).to match_array(expected_to_show_question_ids) - expect(json[:qn_data][:to_hide]).to match_array(expected_to_hide_question_ids) + check_question_ids_to_show_and_hide(json, condition.remove_data) end it 'handles single option (without condition) in option_list' do create(:condition, question: @conditional_questions[:dropdown], @@ -242,8 +228,7 @@ post :create_or_update, params: { answer: args } json = JSON.parse(response.body).with_indifferent_access - expect(json[:qn_data][:to_show]).to match_array(@all_questions_ids) - expect(json[:qn_data][:to_hide]).to match_array([]) + check_question_ids_to_show_and_hide(json) end end end @@ -276,19 +261,14 @@ post :create_or_update, params: { answer: args } json = JSON.parse(response.body).with_indifferent_access - # Check hide/show questions lists sent to frontend. - expected_to_show_question_ids = @all_questions_ids - add_webhook_condition.remove_data - expected_to_hide_question_ids = add_webhook_condition.remove_data - expect(json[:qn_data][:to_show]).to match_array(expected_to_show_question_ids) - expect(json[:qn_data][:to_hide]).to match_array(expected_to_hide_question_ids) + check_question_ids_to_show_and_hide(json, add_webhook_condition.remove_data) # An email should have been sent to the configured recipient in the webhook. # The webhook_data is a Json string of form: # '{"name":"Joe Bloggs","email":"joe.bloggs@example.com","subject":"Large data volume","message":"A message."}' expect(ActionMailer::Base.deliveries.count).to eq(1) webhook_data = JSON.parse(add_webhook_condition.webhook_data) - check_delivered_mail_for_webhook_data_and_question_data(webhook_data, :checkbox) end it 'handles multiple checkbox options (one of which is add_webhook condition)' do @@ -320,18 +300,14 @@ json = JSON.parse(response.body).with_indifferent_access # Check hide/show questions lists sent to frontend. - removed_data = add_webhook_condition.remove_data + condition2.remove_data - expected_to_show_question_ids = @all_questions_ids - removed_data - expected_to_hide_question_ids = add_webhook_condition.remove_data + condition2.remove_data - expect(json[:qn_data][:to_show]).to match_array(expected_to_show_question_ids) - expect(json[:qn_data][:to_hide]).to match_array(expected_to_hide_question_ids) + remove_data = add_webhook_condition.remove_data + condition2.remove_data + check_question_ids_to_show_and_hide(json, remove_data) # An email should have been sent to the configured recipient in the webhook. # The webhook_data is a Json string of form: # '{"name":"Joe Bloggs","email":"joe.bloggs@example.com","subject":"Large data volume","message":"A message."}' expect(ActionMailer::Base.deliveries.count).to eq(1) webhook_data = JSON.parse(add_webhook_condition.webhook_data) - check_delivered_mail_for_webhook_data_and_question_data(webhook_data, :checkbox) end @@ -357,17 +333,13 @@ json = JSON.parse(response.body).with_indifferent_access # Check hide/show questions lists sent to frontend. - expected_to_show_question_ids = @all_questions_ids - add_webhook_condition.remove_data - expected_to_hide_question_ids = add_webhook_condition.remove_data - expect(json[:qn_data][:to_show]).to match_array(expected_to_show_question_ids) - expect(json[:qn_data][:to_hide]).to match_array(expected_to_hide_question_ids) + check_question_ids_to_show_and_hide(json, add_webhook_condition.remove_data) # An email should have been sent to the configured recipient in the webhook. # The webhook_data is a Json string of form: # '{"name":"Joe Bloggs","email":"joe.bloggs@example.com","subject":"Large data volume","message":"A message."}' expect(ActionMailer::Base.deliveries.count).to eq(1) webhook_data = JSON.parse(add_webhook_condition.webhook_data) - check_delivered_mail_for_webhook_data_and_question_data(webhook_data, :dropdown) end @@ -393,17 +365,13 @@ json = JSON.parse(response.body).with_indifferent_access # Check hide/show questions lists sent to frontend. - expected_to_show_question_ids = @all_questions_ids - add_webhook_condition.remove_data - expected_to_hide_question_ids = add_webhook_condition.remove_data - expect(json[:qn_data][:to_show]).to match_array(expected_to_show_question_ids) - expect(json[:qn_data][:to_hide]).to match_array(expected_to_hide_question_ids) + check_question_ids_to_show_and_hide(json, add_webhook_condition.remove_data) # An email should have been sent to the configured recipient in the webhook. # The webhook_data is a Json string of form: # '{"name":"Joe Bloggs","email":"joe.bloggs@example.com","subject":"Large data volume","message":"A message."}' expect(ActionMailer::Base.deliveries.count).to eq(1) webhook_data = JSON.parse(add_webhook_condition.webhook_data) - check_delivered_mail_for_webhook_data_and_question_data(webhook_data, :radiobutton) end end @@ -464,4 +432,9 @@ def check_delivered_mail_for_webhook_data_and_question_data(webhook_data, questi expect(mail.body.encoded).to include(@conditional_questions[question_type].text) end end + + def check_question_ids_to_show_and_hide(json, question_ids_to_hide = []) + expect(json[:qn_data][:to_show]).to match_array(@all_questions_ids - question_ids_to_hide) + expect(json[:qn_data][:to_hide]).to match_array(question_ids_to_hide) + end end From 80c99abcc7e2c0c46e0ecf774a38e1b926aca881 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Tue, 6 May 2025 13:19:22 -0600 Subject: [PATCH 08/17] Move helper methods to helper file --- ...troller_with_conditional_questions_spec.rb | 66 +------------------ .../questions/conditions_questions_spec.rb | 59 +---------------- .../helpers/conditional_questions_helper.rb | 62 +++++++++++++++++ 3 files changed, 68 insertions(+), 119 deletions(-) create mode 100644 spec/support/helpers/conditional_questions_helper.rb diff --git a/spec/controllers/answers_controller_with_conditional_questions_spec.rb b/spec/controllers/answers_controller_with_conditional_questions_spec.rb index 2be290c73b..9ab14bef16 100644 --- a/spec/controllers/answers_controller_with_conditional_questions_spec.rb +++ b/spec/controllers/answers_controller_with_conditional_questions_spec.rb @@ -4,6 +4,7 @@ RSpec.describe AnswersController, type: :controller do include RolesHelper + include ConditionalQuestionsHelper before(:each) do template = create(:template, phases: 1, sections: 3) @@ -11,10 +12,10 @@ @section1, @section2, @section3 = template.sections # Different types of questions (than can have conditional options) - @conditional_questions = create_conditional_questions + @conditional_questions = create_conditional_questions(5) # Questions that do not have conditional options for adding or removing - @non_conditional_questions = create_non_conditional_questions + @non_conditional_questions = create_non_conditional_questions(7, 3) @plan = create(:plan, :creator, template: template) @user = @plan.owner @@ -376,65 +377,4 @@ end end end - - private - - def create_conditional_questions - { - checkbox: create(:question, :checkbox, section: @section1, options: 5), - radiobutton: create(:question, :radiobuttons, section: @section2, options: 5), - dropdown: create(:question, :dropdown, section: @section3, options: 5) - } - end - - def create_non_conditional_questions - { - textarea: create_list(:question, 7, :textarea, section: @section1), - textfield: create_list(:question, 7, :textfield, section: @section2), - date: create_list(:question, 7, :date, section: @section3), - rda_metadata: create_list(:question, 7, :rda_metadata, section: @section1, options: 3), - checkbox: create_list(:question, 7, :checkbox, section: @section2, options: 3), - radiobutton: create_list(:question, 7, :radiobuttons, section: @section3, options: 3), - dropdown: create_list(:question, 7, :dropdown, section: @section1, options: 3), - multiselectbox: create_list(:question, 7, :multiselectbox, section: @section2, options: 3) - } - end - - def non_conditional_questions_ids_by_index(index) - @non_conditional_questions.map { |_, questions| questions[index].id } - end - - def create_answers - question_types_with_question_options = %i[checkbox radiobutton dropdown multiselectbox] - answers = {} - @non_conditional_questions.each do |question_type, questions| - answers[question_type] = questions.map do |question| - if question_types_with_question_options.include?(question_type) - create(:answer, plan: @plan, question: question, question_options: [question.question_options[2]], user: @user) - else - create(:answer, plan: @plan, question: question, user: @user) - end - end - end - answers - end - - def check_delivered_mail_for_webhook_data_and_question_data(webhook_data, question_type) - ActionMailer::Base.deliveries.first do |mail| - expect(mail.to).to eq([webhook_data['email']]) - expect(mail.subject).to eq(webhook_data['subject']) - expect(mail.body.encoded).to include(webhook_data['message']) - # To see structure of email sent see app/views/user_mailer/question_answered.html.erb. - - # Message should have @user.name, chosen option text and question text. - expect(mail.body.encoded).to include(@user.name) - expect(mail.body.encoded).to include(@conditional_questions[question_type].question_options[2].text) - expect(mail.body.encoded).to include(@conditional_questions[question_type].text) - end - end - - def check_question_ids_to_show_and_hide(json, question_ids_to_hide = []) - expect(json[:qn_data][:to_show]).to match_array(@all_questions_ids - question_ids_to_hide) - expect(json[:qn_data][:to_hide]).to match_array(question_ids_to_hide) - end end diff --git a/spec/features/questions/conditions_questions_spec.rb b/spec/features/questions/conditions_questions_spec.rb index 080c56657c..b0601eaed4 100644 --- a/spec/features/questions/conditions_questions_spec.rb +++ b/spec/features/questions/conditions_questions_spec.rb @@ -3,6 +3,7 @@ require 'rails_helper' RSpec.feature 'Question::Conditions questions', type: :feature do + include ConditionalQuestionsHelper before(:each) do @user = create(:user) @template = create(:template, :default, :published) @@ -14,10 +15,10 @@ @section3 = create(:section, phase: @phase) # Different types of questions (than can have conditional options) - @conditional_questions = create_conditional_questions + @conditional_questions = create_conditional_questions(5) # Questions that do not have conditional options for adding or removing - @non_conditional_questions = create_non_conditional_questions + @non_conditional_questions = create_non_conditional_questions(3, 5) create(:role, :creator, :editor, :commenter, user: @user, plan: @plan) @@ -432,58 +433,4 @@ check_delivered_mail_for_webhook_data_and_question_data(webhook_data, :dropdown) end end - - private - - def create_conditional_questions - { - checkbox: create(:question, :checkbox, section: @section1, options: 5), - radiobutton: create(:question, :radiobuttons, section: @section2, options: 5), - dropdown: create(:question, :dropdown, section: @section3, options: 5) - } - end - - def create_non_conditional_questions - { - textarea: create_list(:question, 3, :textarea, section: @section1), - textfield: create_list(:question, 3, :textfield, section: @section2), - date: create_list(:question, 3, :date, section: @section3), - rda_metadata: create_list(:question, 3, :rda_metadata, section: @section1, options: 5), - checkbox: create_list(:question, 3, :checkbox, section: @section2, options: 5), - radiobutton: create_list(:question, 3, :radiobuttons, section: @section3, options: 5), - dropdown: create_list(:question, 3, :dropdown, section: @section1, options: 5), - multiselectbox: create_list(:question, 3, :multiselectbox, section: @section2, options: 5) - } - end - - def create_answers - @non_conditional_questions.each do |question_type, questions| - questions.map do |question| - if %i[textarea textfield date rda_metadata].include?(question_type) - create(:answer, plan: @plan, question: question, user: @user) - else - create(:answer, plan: @plan, question: question, question_options: [question.question_options[2]], - user: @user) - end - end - end - end - - def non_conditional_questions_ids_by_index(index) - @non_conditional_questions.map { |_, questions| questions[index].id } - end - - def check_delivered_mail_for_webhook_data_and_question_data(webhook_data, question_type) - ActionMailer::Base.deliveries.first do |mail| - expect(mail.to).to eq([webhook_data['email']]) - expect(mail.subject).to eq(webhook_data['subject']) - expect(mail.body.encoded).to include(webhook_data['message']) - # To see structure of email sent see app/views/user_mailer/question_answered.html.erb. - - # Message should have @user.name, chosen option text and question text. - expect(mail.body.encoded).to include(@user.name) - expect(mail.body.encoded).to include(@conditional_questions[question_type].question_options[2].text) - expect(mail.body.encoded).to include(@conditional_questions[question_type].text) - end - end end diff --git a/spec/support/helpers/conditional_questions_helper.rb b/spec/support/helpers/conditional_questions_helper.rb new file mode 100644 index 0000000000..e12f78849c --- /dev/null +++ b/spec/support/helpers/conditional_questions_helper.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +module ConditionalQuestionsHelper + def create_conditional_questions(num_options) + { + checkbox: create(:question, :checkbox, section: @section1, options: num_options), + radiobutton: create(:question, :radiobuttons, section: @section2, options: num_options), + dropdown: create(:question, :dropdown, section: @section3, options: num_options) + } + end + + def create_non_conditional_questions(num_questions, num_options) + { + textarea: create_list(:question, num_questions, :textarea, section: @section1), + textfield: create_list(:question, num_questions, :textfield, section: @section2), + date: create_list(:question, num_questions, :date, section: @section3), + rda_metadata: create_list(:question, num_questions, :rda_metadata, section: @section1, options: num_options), + checkbox: create_list(:question, num_questions, :checkbox, section: @section2, options: num_options), + radiobutton: create_list(:question, num_questions, :radiobuttons, section: @section3, options: num_options), + dropdown: create_list(:question, num_questions, :dropdown, section: @section1, options: num_options), + multiselectbox: create_list(:question, num_questions, :multiselectbox, section: @section2, options: num_options) + } + end + + def create_answers + question_types_with_question_options = %i[checkbox radiobutton dropdown multiselectbox] + answers = {} + @non_conditional_questions.each do |question_type, questions| + answers[question_type] = questions.map do |question| + if question_types_with_question_options.include?(question_type) + create(:answer, plan: @plan, question: question, question_options: [question.question_options[2]], user: @user) + else + create(:answer, plan: @plan, question: question, user: @user) + end + end + end + answers + end + + def non_conditional_questions_ids_by_index(index) + @non_conditional_questions.map { |_, questions| questions[index].id } + end + + def check_question_ids_to_show_and_hide(json, question_ids_to_hide = []) + expect(json[:qn_data][:to_show]).to match_array(@all_questions_ids - question_ids_to_hide) + expect(json[:qn_data][:to_hide]).to match_array(question_ids_to_hide) + end + + def check_delivered_mail_for_webhook_data_and_question_data(webhook_data, question_type) + ActionMailer::Base.deliveries.first do |mail| + expect(mail.to).to eq([webhook_data['email']]) + expect(mail.subject).to eq(webhook_data['subject']) + expect(mail.body.encoded).to include(webhook_data['message']) + # To see structure of email sent see app/views/user_mailer/question_answered.html.erb. + + # Message should have @user.name, chosen option text and question text. + expect(mail.body.encoded).to include(@user.name) + expect(mail.body.encoded).to include(@conditional_questions[question_type].question_options[2].text) + expect(mail.body.encoded).to include(@conditional_questions[question_type].text) + end + end +end From 8303dc4628b7aec043a127099196313ba357b3f1 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Tue, 6 May 2025 13:25:32 -0600 Subject: [PATCH 09/17] Address rubocop offences --- spec/features/questions/conditions_questions_spec.rb | 6 ++++-- spec/support/helpers/conditional_questions_helper.rb | 8 +++++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/spec/features/questions/conditions_questions_spec.rb b/spec/features/questions/conditions_questions_spec.rb index b0601eaed4..1a7b25e7bd 100644 --- a/spec/features/questions/conditions_questions_spec.rb +++ b/spec/features/questions/conditions_questions_spec.rb @@ -363,8 +363,10 @@ end scenario 'User answers chooses radiobutton option with a condition (with action_type: add_webhook)', :js do - condition = create(:condition, :webhook, question: @conditional_questions[:radiobutton], - option_list: [@conditional_questions[:radiobutton].question_options[0].id]) + condition = create(:condition, + :webhook, + question: @conditional_questions[:radiobutton], + option_list: [@conditional_questions[:radiobutton].question_options[0].id]) visit overview_plan_path(@plan) diff --git a/spec/support/helpers/conditional_questions_helper.rb b/spec/support/helpers/conditional_questions_helper.rb index e12f78849c..4a1f261c56 100644 --- a/spec/support/helpers/conditional_questions_helper.rb +++ b/spec/support/helpers/conditional_questions_helper.rb @@ -28,7 +28,11 @@ def create_answers @non_conditional_questions.each do |question_type, questions| answers[question_type] = questions.map do |question| if question_types_with_question_options.include?(question_type) - create(:answer, plan: @plan, question: question, question_options: [question.question_options[2]], user: @user) + create(:answer, + plan: @plan, + question: question, + question_options: [question.question_options[2]], + user: @user) else create(:answer, plan: @plan, question: question, user: @user) end @@ -46,6 +50,7 @@ def check_question_ids_to_show_and_hide(json, question_ids_to_hide = []) expect(json[:qn_data][:to_hide]).to match_array(question_ids_to_hide) end + # rubocop:disable Metrics/AbcSize def check_delivered_mail_for_webhook_data_and_question_data(webhook_data, question_type) ActionMailer::Base.deliveries.first do |mail| expect(mail.to).to eq([webhook_data['email']]) @@ -59,4 +64,5 @@ def check_delivered_mail_for_webhook_data_and_question_data(webhook_data, questi expect(mail.body.encoded).to include(@conditional_questions[question_type].text) end end + # rubocop:enable Metrics/AbcSize end From 3852c5635049caad15b77804aee80fd6bd5934d9 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Wed, 7 May 2025 09:18:48 -0600 Subject: [PATCH 10/17] Refactor duplicate code to helper method --- .../questions/conditions_questions_spec.rb | 115 ++++-------------- 1 file changed, 24 insertions(+), 91 deletions(-) diff --git a/spec/features/questions/conditions_questions_spec.rb b/spec/features/questions/conditions_questions_spec.rb index 1a7b25e7bd..f14d82bb11 100644 --- a/spec/features/questions/conditions_questions_spec.rb +++ b/spec/features/questions/conditions_questions_spec.rb @@ -53,16 +53,7 @@ @non_conditional_questions[:dropdown][0].id, @non_conditional_questions[:multiselectbox][1].id]) - visit overview_plan_path(@plan) - - click_link 'Write Plan' - - # Expand all sections - find('a[data-toggle-direction=show]').click - - # Check questions answered in progress bar. - # 24 non-conditional questions in total answered. - expect(page).to have_text('24/27 answered') + go_to_write_plan_page_and_verify_answered # Answer the checkbox_conditional_question. within("#answer-form-#{@conditional_questions[:checkbox].id}") do @@ -109,17 +100,7 @@ action_type: 'remove', remove_data: non_conditional_questions_ids_by_index(0)) - # We choose an option that is not in the option_list of the conditions defined above. - visit overview_plan_path(@plan) - - click_link 'Write Plan' - - # Expand all sections - find('a[data-toggle-direction=show]').click - - # Check questions answered in progress bar. - # 24 non-conditional questions in total answered. - expect(page).to have_text('24/27 answered') + go_to_write_plan_page_and_verify_answered # Answer the checkbox_conditional_question within("#answer-form-#{@conditional_questions[:checkbox].id}") do @@ -148,16 +129,7 @@ @non_conditional_questions[:dropdown][0].id, @non_conditional_questions[:multiselectbox][1].id]) - visit overview_plan_path(@plan) - - click_link 'Write Plan' - - # Expand all sections - find('a[data-toggle-direction=show]').click - - # Check questions answered in progress bar. - # 24 non-conditional questions in total answered. - expect(page).to have_text('24/27 answered') + go_to_write_plan_page_and_verify_answered # Answer the radiobutton_conditional_question. within("#answer-form-#{@conditional_questions[:radiobutton].id}") do @@ -207,17 +179,7 @@ action_type: 'remove', remove_data: non_conditional_questions_ids_by_index(0)) - # We choose an option that is not in the option_list of the conditions defined above. - visit overview_plan_path(@plan) - - click_link 'Write Plan' - - # Expand all sections - find('a[data-toggle-direction=show]').click - - # Check questions answered in progress bar. - # 24 non-conditional questions in total answered. - expect(page).to have_text('24/27 answered') + go_to_write_plan_page_and_verify_answered # Answer the radiobutton_conditional_question. within("#answer-form-#{@conditional_questions[:radiobutton].id}") do @@ -246,16 +208,7 @@ @non_conditional_questions[:dropdown][0].id, @non_conditional_questions[:multiselectbox][1].id]) - visit overview_plan_path(@plan) - - click_link 'Write Plan' - - # Expand all sections - find('a[data-toggle-direction=show]').click - - # Check questions answered in progress bar. - # 24 non-conditional questions in total answered. - expect(page).to have_text('24/27 answered') + go_to_write_plan_page_and_verify_answered # Answer the dropdown_conditional_question within("#answer-form-#{@conditional_questions[:dropdown].id}") do @@ -302,16 +255,8 @@ option_list: [@conditional_questions[:dropdown].question_options[4].id], action_type: 'remove', remove_data: non_conditional_questions_ids_by_index(0)) - visit overview_plan_path(@plan) - - click_link 'Write Plan' - # Expand all sections - find('a[data-toggle-direction=show]').click - - # Check questions answered in progress bar. - # 24 non-conditional questions in total answered. - expect(page).to have_text('24/27 answered') + go_to_write_plan_page_and_verify_answered # Answer the dropdown_conditional_question. within("#answer-form-#{@conditional_questions[:dropdown].id}") do @@ -331,16 +276,7 @@ condition = create(:condition, :webhook, question: @conditional_questions[:checkbox], option_list: [@conditional_questions[:checkbox].question_options[2].id]) - visit overview_plan_path(@plan) - - click_link 'Write Plan' - - # Expand all sections - find('a[data-toggle-direction=show]').click - - # Check questions answered in progress bar. - # 24 non-conditional questions in total answered. - expect(page).to have_text('24/27 answered') + go_to_write_plan_page_and_verify_answered # Answer the checkbox_conditional_question. within("#answer-form-#{@conditional_questions[:checkbox].id}") do @@ -368,16 +304,7 @@ question: @conditional_questions[:radiobutton], option_list: [@conditional_questions[:radiobutton].question_options[0].id]) - visit overview_plan_path(@plan) - - click_link 'Write Plan' - - # Expand all sections - find('a[data-toggle-direction=show]').click - - # Check questions answered in progress bar. - # 24 non-conditional questions in total answered. - expect(page).to have_text('24/27 answered') + go_to_write_plan_page_and_verify_answered # Now for radiobutton_conditional_question answer, there in no unchoose option, # so we switch options to a different option without any conditions. @@ -404,16 +331,7 @@ condition = create(:condition, :webhook, question: @conditional_questions[:dropdown], option_list: [@conditional_questions[:dropdown].question_options[2].id]) - visit overview_plan_path(@plan) - - click_link 'Write Plan' - - # Expand all sections - find('a[data-toggle-direction=show]').click - - # Check questions answered in progress bar. - # 24 non-conditional questions in total answered. - expect(page).to have_text('24/27 answered') + go_to_write_plan_page_and_verify_answered # Answer the dropdown_conditional_question within("#answer-form-#{@conditional_questions[:dropdown].id}") do @@ -435,4 +353,19 @@ check_delivered_mail_for_webhook_data_and_question_data(webhook_data, :dropdown) end end + + private + + def go_to_write_plan_page_and_verify_answered + visit overview_plan_path(@plan) + + click_link 'Write Plan' + + # Expand all sections + find('a[data-toggle-direction=show]').click + + # Check questions answered in progress bar. + # 24 non-conditional questions in total answered. + expect(page).to have_text('24/27 answered') + end end From 025881179601bb0d49ba33ec9004094208a2efbb Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Wed, 7 May 2025 09:25:38 -0600 Subject: [PATCH 11/17] Further refactor of webhook_data delivery check --- .../questions/conditions_questions_spec.rb | 24 +++---------------- .../helpers/conditional_questions_helper.rb | 5 ++++ 2 files changed, 8 insertions(+), 21 deletions(-) diff --git a/spec/features/questions/conditions_questions_spec.rb b/spec/features/questions/conditions_questions_spec.rb index f14d82bb11..b3d0c560a1 100644 --- a/spec/features/questions/conditions_questions_spec.rb +++ b/spec/features/questions/conditions_questions_spec.rb @@ -289,13 +289,7 @@ # Expect one extra answer to be added. expect(page).to have_text('25/27 answered') - # An email should have been sent to the configured recipient in the webhook. - # The webhook_data is a Json string of form: - # '{"name":"Joe Bloggs","email":"joe.bloggs@example.com","subject":"Large data volume","message":"A message."}' - expect(ActionMailer::Base.deliveries.count).to eq(1) - webhook_data = JSON.parse(condition.webhook_data) - - check_delivered_mail_for_webhook_data_and_question_data(webhook_data, :checkbox) + check_delivered_mail_for_webhook_data_and_question_data(JSON.parse(condition.webhook_data), :checkbox) end scenario 'User answers chooses radiobutton option with a condition (with action_type: add_webhook)', :js do @@ -318,13 +312,7 @@ # Expect one extra answer to be added. expect(page).to have_text('25/27 answered') - # An email should have been sent to the configured recipient in the webhook. - # The webhook_data is a Json string of form: - # '{"name":"Joe Bloggs","email":"joe.bloggs@example.com","subject":"Large data volume","message":"A message."}' - expect(ActionMailer::Base.deliveries.count).to eq(1) - webhook_data = JSON.parse(condition.webhook_data) - - check_delivered_mail_for_webhook_data_and_question_data(webhook_data, :radiobutton) + check_delivered_mail_for_webhook_data_and_question_data(JSON.parse(condition.webhook_data), :radiobutton) end scenario 'User answers chooses dropdown option with a condition (with action_type: add_webhook)', :js do @@ -344,13 +332,7 @@ # Expect one extra answer to be added. expect(page).to have_text('25/27 answered') - # An email should have been sent to the configured recipient in the webhook. - # The webhook_data is a Json string of form: - # '{"name":"Joe Bloggs","email":"joe.bloggs@example.com","subject":"Large data volume","message":"A message."}' - expect(ActionMailer::Base.deliveries.count).to eq(1) - webhook_data = JSON.parse(condition.webhook_data) - - check_delivered_mail_for_webhook_data_and_question_data(webhook_data, :dropdown) + check_delivered_mail_for_webhook_data_and_question_data(JSON.parse(condition.webhook_data), :dropdown) end end diff --git a/spec/support/helpers/conditional_questions_helper.rb b/spec/support/helpers/conditional_questions_helper.rb index 4a1f261c56..7d915c150f 100644 --- a/spec/support/helpers/conditional_questions_helper.rb +++ b/spec/support/helpers/conditional_questions_helper.rb @@ -52,6 +52,11 @@ def check_question_ids_to_show_and_hide(json, question_ids_to_hide = []) # rubocop:disable Metrics/AbcSize def check_delivered_mail_for_webhook_data_and_question_data(webhook_data, question_type) + # An email should have been sent to the configured recipient in the webhook. + # The webhook_data is a Json string of form: + # '{"name":"Joe Bloggs","email":"joe.bloggs@example.com","subject":"Large data volume","message":"A message."}' + expect(ActionMailer::Base.deliveries.count).to eq(1) + ActionMailer::Base.deliveries.first do |mail| expect(mail.to).to eq([webhook_data['email']]) expect(mail.subject).to eq(webhook_data['subject']) From b17ba15fc6c53f62c22dfc8fb48bba4c41a86b8b Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Wed, 7 May 2025 12:25:48 -0600 Subject: [PATCH 12/17] Refactor answer-form selector expect checking --- .../questions/conditions_questions_spec.rb | 38 +++++++------------ 1 file changed, 13 insertions(+), 25 deletions(-) diff --git a/spec/features/questions/conditions_questions_spec.rb b/spec/features/questions/conditions_questions_spec.rb index b3d0c560a1..6ad57958fe 100644 --- a/spec/features/questions/conditions_questions_spec.rb +++ b/spec/features/questions/conditions_questions_spec.rb @@ -67,15 +67,7 @@ # 27 - 8 = 19 (Questions left) expect(page).to have_text('17/19 answered') - condition.remove_data.each.map do |question_id| - expect(page).to have_no_selector("#answer-form-#{question_id}") - end - - expected_remaining_question_ids = @all_questions_ids - condition.remove_data - - expected_remaining_question_ids.each.map do |question_id| - expect(page).to have_selector("#answer-form-#{question_id}") - end + check_remove_data_effect_on_answer_form_selectors(condition.remove_data) # Now uncheck checkbox_conditional_question answer. within("#answer-form-#{@conditional_questions[:checkbox].id}") do @@ -144,15 +136,8 @@ # 24 -8 + 1 = 17 (Answers left) # 27 - 8 = 19 (Questions left) expect(page).to have_text('17/19 answered') - condition.remove_data.each.map do |question_id| - expect(page).to have_no_selector("#answer-form-#{question_id}") - end - - expected_remaining_question_ids = @all_questions_ids - condition.remove_data - expected_remaining_question_ids.each.map do |question_id| - expect(page).to have_selector("#answer-form-#{question_id}") - end + check_remove_data_effect_on_answer_form_selectors(condition.remove_data) # Now for radiobutton_conditional_question answer, there in no unchoose option, # so we switch options to a different option without any conditions. @@ -223,15 +208,8 @@ # 24 -8 + 1 = 17 (Answers left) # 27 - 8 = 19 (Questions left) expect(page).to have_text('17/19 answered') - condition.remove_data.each.map do |question_id| - expect(page).to have_no_selector("#answer-form-#{question_id}") - end - - expected_remaining_question_ids = @all_questions_ids - condition.remove_data - expected_remaining_question_ids.each.map do |question_id| - expect(page).to have_selector("#answer-form-#{question_id}") - end + check_remove_data_effect_on_answer_form_selectors(condition.remove_data) # Now select another option for dropdown_conditional_question. within("#answer-form-#{@conditional_questions[:dropdown].id}") do @@ -350,4 +328,14 @@ def go_to_write_plan_page_and_verify_answered # 24 non-conditional questions in total answered. expect(page).to have_text('24/27 answered') end + + def check_remove_data_effect_on_answer_form_selectors(remove_data) + @all_questions_ids.each do |question_id| + if remove_data.include?(question_id) + expect(page).to have_no_selector("#answer-form-#{question_id}") + else + expect(page).to have_selector("#answer-form-#{question_id}") + end + end + end end From bc56752e489b18c5ef0ca68f8ca7fa626815827a Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Tue, 20 May 2025 14:59:12 -0600 Subject: [PATCH 13/17] Improve checking of answer save messages - The `expect(page).to have_text('Answered just now')` was could not be relied upon to perform the desired check for the saved answer. This is because 'Answered just now' is rendered for many answers on the page, rather than just the newly answered one. - This change also removes all `click_button 'Save'` executions. This code already appears to be absent from some saved answers. In addition, all of the saves seem to be auto-executed so this change makes the tests more consistent and removes redundancy. - Some refactoring has also been performed. --- .../questions/conditions_questions_spec.rb | 74 +++++++++++-------- 1 file changed, 44 insertions(+), 30 deletions(-) diff --git a/spec/features/questions/conditions_questions_spec.rb b/spec/features/questions/conditions_questions_spec.rb index 6ad57958fe..da5cf5a964 100644 --- a/spec/features/questions/conditions_questions_spec.rb +++ b/spec/features/questions/conditions_questions_spec.rb @@ -41,6 +41,7 @@ describe 'conditions with action_type remove' do feature 'User answers a checkboxes question with a condition' do scenario 'User answers chooses checkbox option with a condition', :js do + answer_id = @conditional_questions[:checkbox].id condition = create(:condition, question: @conditional_questions[:checkbox], option_list: [@conditional_questions[:checkbox].question_options[2].id], action_type: 'remove', @@ -56,12 +57,11 @@ go_to_write_plan_page_and_verify_answered # Answer the checkbox_conditional_question. - within("#answer-form-#{@conditional_questions[:checkbox].id}") do + within("#answer-form-#{answer_id}") do check @conditional_questions[:checkbox].question_options[2].text - click_button 'Save' end - expect(page).to have_text('Answered just now') + check_answer_save_statuses(answer_id) # Expect 8 questions and answers that have ids in condition.remove_data to be removed, and 1 new answer added: # 24 -8 + 1 = 17 (Answers left) # 27 - 8 = 19 (Questions left) @@ -70,11 +70,11 @@ check_remove_data_effect_on_answer_form_selectors(condition.remove_data) # Now uncheck checkbox_conditional_question answer. - within("#answer-form-#{@conditional_questions[:checkbox].id}") do + within("#answer-form-#{answer_id}") do uncheck @conditional_questions[:checkbox].question_options[2].text - click_button 'Save' end + check_answer_save_statuses(answer_id) # Expect 27 questions to appear again, but the 8 answers that were removed should not be there. # Also 1 answer should be removed as we unchecked @conditional_questions[:checkbox].question_options[2].text # 17 (from previous check) - 1 = 16 @@ -82,6 +82,7 @@ end scenario 'User answers chooses checkbox option without a condition', :js do + answer_id = @conditional_questions[:checkbox].id create(:condition, question: @conditional_questions[:checkbox], option_list: [@conditional_questions[:checkbox].question_options[1].id], action_type: 'remove', @@ -95,12 +96,11 @@ go_to_write_plan_page_and_verify_answered # Answer the checkbox_conditional_question - within("#answer-form-#{@conditional_questions[:checkbox].id}") do + within("#answer-form-#{answer_id}") do check @conditional_questions[:checkbox].question_options[0].text - click_button 'Save' end - expect(page).to have_text('Answered just now') + check_answer_save_statuses(answer_id) # Check questions answered in progress bar. expect(page).to have_text('25/27 answered') @@ -109,6 +109,7 @@ feature 'User answers a radiobutton question with a condition' do scenario 'User answers selects radiobutton option with a condition', :js do + answer_id = @conditional_questions[:radiobutton].id condition = create(:condition, question: @conditional_questions[:radiobutton], option_list: [@conditional_questions[:radiobutton].question_options[2].id], action_type: 'remove', @@ -124,12 +125,11 @@ go_to_write_plan_page_and_verify_answered # Answer the radiobutton_conditional_question. - within("#answer-form-#{@conditional_questions[:radiobutton].id}") do + within("#answer-form-#{answer_id}") do choose @conditional_questions[:radiobutton].question_options[2].text - click_button 'Save' end - expect(page).to have_text('Answered just now') + check_answer_save_statuses(answer_id) # Check questions answered in progress bar. # Expect 8 questions and answers that have ids in condition.remove_data to be removed, and 1 new answer added: @@ -141,11 +141,11 @@ # Now for radiobutton_conditional_question answer, there in no unchoose option, # so we switch options to a different option without any conditions. - within("#answer-form-#{@conditional_questions[:radiobutton].id}") do + within("#answer-form-#{answer_id}") do choose @conditional_questions[:radiobutton].question_options[0].text - click_button 'Save' end + check_answer_save_statuses(answer_id) # Check questions answered in progress bar. # Expect 27 questions to appear again, but the 8 answers that were removed should not be there. # Also 1 answer should be removed as we unchecked @conditional_questions[:radiobutton].question_options[2].text @@ -154,6 +154,7 @@ end scenario 'User answers selects radiobutton option without a condition', :js do + answer_id = @conditional_questions[:radiobutton].id create(:condition, question: @conditional_questions[:radiobutton], option_list: [@conditional_questions[:radiobutton].question_options[1].id], action_type: 'remove', @@ -167,12 +168,11 @@ go_to_write_plan_page_and_verify_answered # Answer the radiobutton_conditional_question. - within("#answer-form-#{@conditional_questions[:radiobutton].id}") do + within("#answer-form-#{answer_id}") do choose @conditional_questions[:radiobutton].question_options[0].text - click_button 'Save' end - expect(page).to have_text('Answered just now') + check_answer_save_statuses(answer_id) # Check questions answered in progress bar. expect(page).to have_text('25/27 answered') @@ -181,6 +181,7 @@ feature 'User answers a dropdown question with a condition' do scenario 'User answers chooses dropdown option with a condition', :js do + answer_id = @conditional_questions[:dropdown].id condition = create(:condition, question: @conditional_questions[:dropdown], option_list: [@conditional_questions[:dropdown].question_options[2].id], action_type: 'remove', @@ -196,12 +197,11 @@ go_to_write_plan_page_and_verify_answered # Answer the dropdown_conditional_question - within("#answer-form-#{@conditional_questions[:dropdown].id}") do + within("#answer-form-#{answer_id}") do select(@conditional_questions[:dropdown].question_options[2].text, from: 'answer_question_option_ids') - click_button 'Save' end - expect(page).to have_text('Answered just now') + check_answer_save_statuses(answer_id) # Check questions answered in progress bar. # Expect 8 questions and answers that have ids in condition.remove_data to be removed, and 1 new answer added: @@ -212,11 +212,11 @@ check_remove_data_effect_on_answer_form_selectors(condition.remove_data) # Now select another option for dropdown_conditional_question. - within("#answer-form-#{@conditional_questions[:dropdown].id}") do + within("#answer-form-#{answer_id}") do select(@conditional_questions[:dropdown].question_options[1].text, from: 'answer_question_option_ids') - click_button 'Save' end + check_answer_save_statuses(answer_id) # Check questions answered in progress bar. # Expect 27 questions to appear again, but the 8 answers that were removed should not be there. # 17 (from previous check as we switched answer from same dropdown) @@ -224,6 +224,7 @@ end scenario 'User answers select dropdown option without a condition', :js do + answer_id = @conditional_questions[:dropdown].id create(:condition, question: @conditional_questions[:dropdown], option_list: [@conditional_questions[:dropdown].question_options[1].id], action_type: 'remove', @@ -237,12 +238,11 @@ go_to_write_plan_page_and_verify_answered # Answer the dropdown_conditional_question. - within("#answer-form-#{@conditional_questions[:dropdown].id}") do + within("#answer-form-#{answer_id}") do select(@conditional_questions[:dropdown].question_options[0].text, from: 'answer_question_option_ids') - click_button 'Save' end - expect(page).to have_text('Answered just now') + check_answer_save_statuses(answer_id) # Check questions answered in progress bar. expect(page).to have_text('25/27 answered') @@ -251,17 +251,18 @@ end describe 'conditions with action_type add_webhook' do scenario 'User answers chooses checkbox option with a condition (with action_type: add_webhook)', :js do + answer_id = @conditional_questions[:checkbox].id condition = create(:condition, :webhook, question: @conditional_questions[:checkbox], option_list: [@conditional_questions[:checkbox].question_options[2].id]) go_to_write_plan_page_and_verify_answered # Answer the checkbox_conditional_question. - within("#answer-form-#{@conditional_questions[:checkbox].id}") do + within("#answer-form-#{answer_id}") do check @conditional_questions[:checkbox].question_options[2].text end - expect(page).to have_text('Answered just now') + check_answer_save_statuses(answer_id) # Check questions answered in progress bar. # Expect one extra answer to be added. @@ -271,6 +272,7 @@ end scenario 'User answers chooses radiobutton option with a condition (with action_type: add_webhook)', :js do + answer_id = @conditional_questions[:radiobutton].id condition = create(:condition, :webhook, question: @conditional_questions[:radiobutton], @@ -280,11 +282,11 @@ # Now for radiobutton_conditional_question answer, there in no unchoose option, # so we switch options to a different option without any conditions. - within("#answer-form-#{@conditional_questions[:radiobutton].id}") do + within("#answer-form-#{answer_id}") do choose @conditional_questions[:radiobutton].question_options[0].text end - expect(page).to have_text('Answered just now') + check_answer_save_statuses(answer_id) # Check questions answered in progress bar. # Expect one extra answer to be added. @@ -294,17 +296,18 @@ end scenario 'User answers chooses dropdown option with a condition (with action_type: add_webhook)', :js do + answer_id = @conditional_questions[:dropdown].id condition = create(:condition, :webhook, question: @conditional_questions[:dropdown], option_list: [@conditional_questions[:dropdown].question_options[2].id]) go_to_write_plan_page_and_verify_answered # Answer the dropdown_conditional_question - within("#answer-form-#{@conditional_questions[:dropdown].id}") do + within("#answer-form-#{answer_id}") do select(@conditional_questions[:dropdown].question_options[2].text, from: 'answer_question_option_ids') end - expect(page).to have_text('Answered just now') + check_answer_save_statuses(answer_id) # Check questions answered in progress bar. # Expect one extra answer to be added. @@ -338,4 +341,15 @@ def check_remove_data_effect_on_answer_form_selectors(remove_data) end end end + + # Checks for 'Saving' and 'Answered just now' messages + def check_answer_save_statuses(answer_id) + within("#answer-status-#{answer_id}") do + saving_span = find('span.status[data-status="saving"]') + expect(saving_span.text).to include('Saving') + # We use `first()` because there are multiple span elements with `saved-at` status + saved_span = first('span.status[data-status="saved-at"]') + expect(saved_span.text).to include('Answered just now') + end + end end From ceaa792027c58395f558b1a62575aadf0b7ad8db Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Thu, 22 May 2025 10:29:41 -0600 Subject: [PATCH 14/17] Remove :creator trait in conditions_questions_spec - The removed `:creator` trait was creating an extra user that these tests don't appear to need. `create(:role, :creator, :editor, :commenter, user: @user, plan: @plan)` on line 23 seems to be assigning the intended user as plan.creator. --- spec/features/questions/conditions_questions_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/questions/conditions_questions_spec.rb b/spec/features/questions/conditions_questions_spec.rb index da5cf5a964..72356d767e 100644 --- a/spec/features/questions/conditions_questions_spec.rb +++ b/spec/features/questions/conditions_questions_spec.rb @@ -7,7 +7,7 @@ before(:each) do @user = create(:user) @template = create(:template, :default, :published) - @plan = create(:plan, :creator, template: @template) + @plan = create(:plan, template: @template) @phase = create(:phase, template: @template) # 3 sections for ensuring that conditions involve questions in different sections. @section1 = create(:section, phase: @phase) From d3839a34889807583ab1f4bc67bf6d4a9b6e404e Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Fri, 23 May 2025 16:32:14 -0600 Subject: [PATCH 15/17] Refactor: Compute num questions/answers --- .../questions/conditions_questions_spec.rb | 105 ++++++++---------- 1 file changed, 49 insertions(+), 56 deletions(-) diff --git a/spec/features/questions/conditions_questions_spec.rb b/spec/features/questions/conditions_questions_spec.rb index 72356d767e..a35f316511 100644 --- a/spec/features/questions/conditions_questions_spec.rb +++ b/spec/features/questions/conditions_questions_spec.rb @@ -23,9 +23,11 @@ create(:role, :creator, :editor, :commenter, user: @user, plan: @plan) @all_questions_ids = (@conditional_questions.values + @non_conditional_questions.values.flatten).map(&:id) + @total_initial_questions = @all_questions_ids.count # Answer the non-conditional questions - create_answers + answers = create_answers + @total_initial_answers = answers.values.flatten.count sign_in(@user) @@ -62,11 +64,7 @@ end check_answer_save_statuses(answer_id) - # Expect 8 questions and answers that have ids in condition.remove_data to be removed, and 1 new answer added: - # 24 -8 + 1 = 17 (Answers left) - # 27 - 8 = 19 (Questions left) - expect(page).to have_text('17/19 answered') - + check_question_and_answer_counts_for_plan(condition.remove_data) check_remove_data_effect_on_answer_form_selectors(condition.remove_data) # Now uncheck checkbox_conditional_question answer. @@ -75,10 +73,10 @@ end check_answer_save_statuses(answer_id) - # Expect 27 questions to appear again, but the 8 answers that were removed should not be there. - # Also 1 answer should be removed as we unchecked @conditional_questions[:checkbox].question_options[2].text - # 17 (from previous check) - 1 = 16 - expect(page).to have_text('16/27 answered') + num_questions, num_answers = question_and_answer_counts_for_plan + # Unchecking the conditional question should unhide all of the `remove_data` questions + # `- 1` from num_answers to account for now unchecked conditional question + expect(page).to have_text("#{num_answers - 1}/#{num_questions} answered") end scenario 'User answers chooses checkbox option without a condition', :js do @@ -101,9 +99,7 @@ end check_answer_save_statuses(answer_id) - - # Check questions answered in progress bar. - expect(page).to have_text('25/27 answered') + check_question_and_answer_counts_for_plan end end @@ -130,13 +126,7 @@ end check_answer_save_statuses(answer_id) - - # Check questions answered in progress bar. - # Expect 8 questions and answers that have ids in condition.remove_data to be removed, and 1 new answer added: - # 24 -8 + 1 = 17 (Answers left) - # 27 - 8 = 19 (Questions left) - expect(page).to have_text('17/19 answered') - + check_question_and_answer_counts_for_plan(condition.remove_data) check_remove_data_effect_on_answer_form_selectors(condition.remove_data) # Now for radiobutton_conditional_question answer, there in no unchoose option, @@ -146,11 +136,7 @@ end check_answer_save_statuses(answer_id) - # Check questions answered in progress bar. - # Expect 27 questions to appear again, but the 8 answers that were removed should not be there. - # Also 1 answer should be removed as we unchecked @conditional_questions[:radiobutton].question_options[2].text - # 17 (from previous check) - 1 = 16 - expect(page).to have_text('17/27 answered') + check_question_and_answer_counts_for_plan end scenario 'User answers selects radiobutton option without a condition', :js do @@ -173,9 +159,7 @@ end check_answer_save_statuses(answer_id) - - # Check questions answered in progress bar. - expect(page).to have_text('25/27 answered') + check_question_and_answer_counts_for_plan end end @@ -202,13 +186,7 @@ end check_answer_save_statuses(answer_id) - - # Check questions answered in progress bar. - # Expect 8 questions and answers that have ids in condition.remove_data to be removed, and 1 new answer added: - # 24 -8 + 1 = 17 (Answers left) - # 27 - 8 = 19 (Questions left) - expect(page).to have_text('17/19 answered') - + check_question_and_answer_counts_for_plan(condition.remove_data) check_remove_data_effect_on_answer_form_selectors(condition.remove_data) # Now select another option for dropdown_conditional_question. @@ -217,10 +195,7 @@ end check_answer_save_statuses(answer_id) - # Check questions answered in progress bar. - # Expect 27 questions to appear again, but the 8 answers that were removed should not be there. - # 17 (from previous check as we switched answer from same dropdown) - expect(page).to have_text('17/27 answered') + check_question_and_answer_counts_for_plan end scenario 'User answers select dropdown option without a condition', :js do @@ -243,9 +218,7 @@ end check_answer_save_statuses(answer_id) - - # Check questions answered in progress bar. - expect(page).to have_text('25/27 answered') + check_question_and_answer_counts_for_plan end end end @@ -263,10 +236,7 @@ end check_answer_save_statuses(answer_id) - - # Check questions answered in progress bar. - # Expect one extra answer to be added. - expect(page).to have_text('25/27 answered') + check_question_and_answer_counts_for_plan check_delivered_mail_for_webhook_data_and_question_data(JSON.parse(condition.webhook_data), :checkbox) end @@ -287,10 +257,7 @@ end check_answer_save_statuses(answer_id) - - # Check questions answered in progress bar. - # Expect one extra answer to be added. - expect(page).to have_text('25/27 answered') + check_question_and_answer_counts_for_plan check_delivered_mail_for_webhook_data_and_question_data(JSON.parse(condition.webhook_data), :radiobutton) end @@ -308,10 +275,7 @@ end check_answer_save_statuses(answer_id) - - # Check questions answered in progress bar. - # Expect one extra answer to be added. - expect(page).to have_text('25/27 answered') + check_question_and_answer_counts_for_plan check_delivered_mail_for_webhook_data_and_question_data(JSON.parse(condition.webhook_data), :dropdown) end @@ -319,6 +283,35 @@ private + def check_question_and_answer_counts_for_plan(condition_remove_data = nil) + if condition_remove_data + check_condition_remove_data_effect_on_plan(condition_remove_data.count) + else + # This is either a :webhook type conditional question, or a non-conditional question + num_questions, num_answers = question_and_answer_counts_for_plan + expect(page).to have_text("#{num_answers}/#{num_questions} answered") + end + end + + def check_condition_remove_data_effect_on_plan(num_removed_answers) + num_questions, num_answers = question_and_answer_counts_for_plan + # The number of plan questions has not changed in the db + expect(num_questions).to eql(@total_initial_questions) + # The number of plan answers in the db has changed: + # - We subract num_removed_answers (i.e. `condition.remove_data.count`) + # - We also `+ 1` to account for the answer saved for the conditional question in the process + expected_num_answers = @total_initial_answers - num_removed_answers + 1 + expect(num_answers).to eql(expected_num_answers) + # Check questions answered in progress bar: + # - `@total_initial_questions - num_removed_answers` accounts for the now hidden (but not deleted) questions + expect(page).to have_text("#{expected_num_answers}/#{@total_initial_questions - num_removed_answers} answered") + end + + def question_and_answer_counts_for_plan + plan = Plan.includes(:questions, :answers).first + [plan.questions.count, plan.answers.count] + end + def go_to_write_plan_page_and_verify_answered visit overview_plan_path(@plan) @@ -328,8 +321,8 @@ def go_to_write_plan_page_and_verify_answered find('a[data-toggle-direction=show]').click # Check questions answered in progress bar. - # 24 non-conditional questions in total answered. - expect(page).to have_text('24/27 answered') + num_questions, num_answers = question_and_answer_counts_for_plan + expect(page).to have_text("#{num_answers}/#{num_questions} answered") end def check_remove_data_effect_on_answer_form_selectors(remove_data) From 8893f32b3917dc4ef9f0f0bec077a6410311a374 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Mon, 2 Jun 2025 17:20:11 -0600 Subject: [PATCH 16/17] Improve test speed by reducing test data creation Removed the overall number of questions and answers in the two changed files. This change should improve test completion speed without affecting test quality. --- ...troller_with_conditional_questions_spec.rb | 56 ++++++++++--------- .../questions/conditions_questions_spec.rb | 10 ++-- 2 files changed, 34 insertions(+), 32 deletions(-) diff --git a/spec/controllers/answers_controller_with_conditional_questions_spec.rb b/spec/controllers/answers_controller_with_conditional_questions_spec.rb index 9ab14bef16..97b988a8cc 100644 --- a/spec/controllers/answers_controller_with_conditional_questions_spec.rb +++ b/spec/controllers/answers_controller_with_conditional_questions_spec.rb @@ -12,10 +12,10 @@ @section1, @section2, @section3 = template.sections # Different types of questions (than can have conditional options) - @conditional_questions = create_conditional_questions(5) + @conditional_questions = create_conditional_questions(3) # Questions that do not have conditional options for adding or removing - @non_conditional_questions = create_non_conditional_questions(7, 3) + @non_conditional_questions = create_non_conditional_questions(3, 3) @plan = create(:plan, :creator, template: template) @user = @plan.owner @@ -41,10 +41,11 @@ # NOTE: Checkboxes allow for multiple options to be selected. context 'with conditional checkbox question' do it 'handles single option (with condition) in option_list ' do + non_conditional_question_index = 0 condition = create(:condition, question: @conditional_questions[:checkbox], option_list: [@conditional_questions[:checkbox].question_options[2].id], action_type: 'remove', - remove_data: non_conditional_questions_ids_by_index(5)) + remove_data: non_conditional_questions_ids_by_index(non_conditional_question_index)) # We chose an option that is in the option_list of the condition defined above. Note that # the text sent by UI is an empty string. @@ -64,10 +65,11 @@ # Check hide/show questions lists sent to frontend. check_question_ids_to_show_and_hide(json, condition.remove_data) - # Check Answers in database (persisted). Expect removed answers to be destroyed. - # Answers destroyed eare easier checked using array of ids rather than individually as in example - # expect(Answer.exists?(@answers[:textarea][5].id)).to be_falsey. - removed_answers = @answers.map { |_, answers| answers[5].id } + # Verify that answers for the `removed_data` questions were deleted from the DB. + # NOTE: `@answers` contains only answers to non-conditional questions. + # So we use `non_conditional_question_index` to locate the corresponding answer + # for each type of non-conditional question. + removed_answers = @answers.map { |_, answers| answers[non_conditional_question_index].id } expect(Answer.where(id: removed_answers).pluck(:id)).to be_empty # Answers left expect(Answer.where(id: @all_answers_ids).pluck(:id)).to match_array( @@ -78,10 +80,10 @@ create(:condition, question: @conditional_questions[:checkbox], option_list: [@conditional_questions[:checkbox].question_options[1].id], action_type: 'remove', - remove_data: non_conditional_questions_ids_by_index(3)) + remove_data: non_conditional_questions_ids_by_index(2)) create(:condition, question: @conditional_questions[:checkbox], - option_list: [@conditional_questions[:checkbox].question_options[4].id], + option_list: [@conditional_questions[:checkbox].question_options[2].id], action_type: 'remove', remove_data: non_conditional_questions_ids_by_index(0)) @@ -103,21 +105,21 @@ it 'handles multiple options (some with conditions) in option_list' do condition1 = create(:condition, question: @conditional_questions[:checkbox], - option_list: [@conditional_questions[:checkbox].question_options[2].id], + option_list: [@conditional_questions[:checkbox].question_options[1].id], action_type: 'remove', remove_data: non_conditional_questions_ids_by_index(0)) condition2 = create(:condition, question: @conditional_questions[:checkbox], - option_list: [@conditional_questions[:checkbox].question_options[4].id], + option_list: [@conditional_questions[:checkbox].question_options[2].id], action_type: 'remove', - remove_data: non_conditional_questions_ids_by_index(3)) + remove_data: non_conditional_questions_ids_by_index(2)) # We choose options that is in the option_list of the conditions defined above as well as an option # with no condition defined. args = { - question_option_ids: [@conditional_questions[:checkbox].question_options[1].id, - @conditional_questions[:checkbox].question_options[2].id, - @conditional_questions[:checkbox].question_options[4].id], + question_option_ids: [@conditional_questions[:checkbox].question_options[0].id, + @conditional_questions[:checkbox].question_options[1].id, + @conditional_questions[:checkbox].question_options[2].id], user_id: @user.id, question_id: @conditional_questions[:checkbox].id, plan_id: @plan.id, @@ -137,7 +139,7 @@ condition = create(:condition, question: @conditional_questions[:radiobutton], option_list: [@conditional_questions[:radiobutton].question_options[2].id], action_type: 'remove', - remove_data: non_conditional_questions_ids_by_index(5)) + remove_data: non_conditional_questions_ids_by_index(2)) # We choose an option that is in the option_list of the condition defined above. args = { @@ -158,10 +160,10 @@ create(:condition, question: @conditional_questions[:radiobutton], option_list: [@conditional_questions[:radiobutton].question_options[1].id], action_type: 'remove', - remove_data: non_conditional_questions_ids_by_index(3)) + remove_data: non_conditional_questions_ids_by_index(2)) create(:condition, question: @conditional_questions[:radiobutton], - option_list: [@conditional_questions[:radiobutton].question_options[4].id], + option_list: [@conditional_questions[:radiobutton].question_options[2].id], action_type: 'remove', remove_data: non_conditional_questions_ids_by_index(0)) @@ -188,7 +190,7 @@ condition = create(:condition, question: @conditional_questions[:dropdown], option_list: [@conditional_questions[:dropdown].question_options[2].id], action_type: 'remove', - remove_data: non_conditional_questions_ids_by_index(5)) + remove_data: non_conditional_questions_ids_by_index(2)) # We chose an option that is in the option_list of the condition defined above. args = { @@ -209,10 +211,10 @@ create(:condition, question: @conditional_questions[:dropdown], option_list: [@conditional_questions[:dropdown].question_options[1].id], action_type: 'remove', - remove_data: non_conditional_questions_ids_by_index(3)) + remove_data: non_conditional_questions_ids_by_index(2)) create(:condition, question: @conditional_questions[:dropdown], - option_list: [@conditional_questions[:dropdown].question_options[4].id], + option_list: [@conditional_questions[:dropdown].question_options[2].id], action_type: 'remove', remove_data: non_conditional_questions_ids_by_index(0)) @@ -276,20 +278,20 @@ add_webhook_condition = create(:condition, :webhook, question: @conditional_questions[:checkbox], - option_list: [@conditional_questions[:checkbox].question_options[2].id]) + option_list: [@conditional_questions[:checkbox].question_options[1].id]) condition2 = create(:condition, question: @conditional_questions[:checkbox], - option_list: [@conditional_questions[:checkbox].question_options[4].id], + option_list: [@conditional_questions[:checkbox].question_options[2].id], action_type: 'remove', - remove_data: non_conditional_questions_ids_by_index(3)) + remove_data: non_conditional_questions_ids_by_index(2)) # We chose an option that is in the option_list of the condition defined above. Note that # the text sent by UI is an empty string. args = { text: '', - question_option_ids: [@conditional_questions[:checkbox].question_options[2].id, - @conditional_questions[:checkbox].question_options[4].id, - @conditional_questions[:checkbox].question_options[1].id], + question_option_ids: [@conditional_questions[:checkbox].question_options[0].id, + @conditional_questions[:checkbox].question_options[1].id, + @conditional_questions[:checkbox].question_options[2].id], user_id: @user.id, question_id: @conditional_questions[:checkbox].id, plan_id: @plan.id, diff --git a/spec/features/questions/conditions_questions_spec.rb b/spec/features/questions/conditions_questions_spec.rb index a35f316511..d7c3b65e6a 100644 --- a/spec/features/questions/conditions_questions_spec.rb +++ b/spec/features/questions/conditions_questions_spec.rb @@ -15,10 +15,10 @@ @section3 = create(:section, phase: @phase) # Different types of questions (than can have conditional options) - @conditional_questions = create_conditional_questions(5) + @conditional_questions = create_conditional_questions(3) # Questions that do not have conditional options for adding or removing - @non_conditional_questions = create_non_conditional_questions(3, 5) + @non_conditional_questions = create_non_conditional_questions(3, 3) create(:role, :creator, :editor, :commenter, user: @user, plan: @plan) @@ -87,7 +87,7 @@ remove_data: non_conditional_questions_ids_by_index(2)) create(:condition, question: @conditional_questions[:checkbox], - option_list: [@conditional_questions[:checkbox].question_options[4].id], + option_list: [@conditional_questions[:checkbox].question_options[2].id], action_type: 'remove', remove_data: non_conditional_questions_ids_by_index(0)) @@ -147,7 +147,7 @@ remove_data: non_conditional_questions_ids_by_index(2)) create(:condition, question: @conditional_questions[:radiobutton], - option_list: [@conditional_questions[:radiobutton].question_options[4].id], + option_list: [@conditional_questions[:radiobutton].question_options[2].id], action_type: 'remove', remove_data: non_conditional_questions_ids_by_index(0)) @@ -206,7 +206,7 @@ remove_data: non_conditional_questions_ids_by_index(2)) create(:condition, question: @conditional_questions[:dropdown], - option_list: [@conditional_questions[:dropdown].question_options[4].id], + option_list: [@conditional_questions[:dropdown].question_options[2].id], action_type: 'remove', remove_data: non_conditional_questions_ids_by_index(0)) From e951b4dd710eaa5dc1da1540f4a0f84ed467343c Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Tue, 3 Jun 2025 17:01:22 -0600 Subject: [PATCH 17/17] Reduce test overhead / Randomly use one of checkbox/radiobutton/dropdown Previously, `spec/features/questions/conditions_questions_spec.rb` tested `remove_data` and `add_webhook` functionality separately for each question type: checkbox, radiobutton, and dropdown. While thorough, this approach added a lot of overhead to test execution time. This change reduces that overhead by randomly choosing only one of checkbox, radiobutton, or dropdown for each of the tests. Over time, repeated executions via the GitHub Actions should ensure that all types continue to be exercised, maintaining coverage while also improving overall test time. --- .../questions/conditions_questions_spec.rb | 252 +++++------------- 1 file changed, 69 insertions(+), 183 deletions(-) diff --git a/spec/features/questions/conditions_questions_spec.rb b/spec/features/questions/conditions_questions_spec.rb index d7c3b65e6a..b35d5e793f 100644 --- a/spec/features/questions/conditions_questions_spec.rb +++ b/spec/features/questions/conditions_questions_spec.rb @@ -41,11 +41,16 @@ # So all Conditions are created with option_list with a single option id. describe 'conditions with action_type remove' do - feature 'User answers a checkboxes question with a condition' do - scenario 'User answers chooses checkbox option with a condition', :js do - answer_id = @conditional_questions[:checkbox].id - condition = create(:condition, question: @conditional_questions[:checkbox], - option_list: [@conditional_questions[:checkbox].question_options[2].id], + feature 'User answers a question with a condition' do + scenario 'User answers chooses an option with a condition', :js do + # Choose a conditional question at random (may be of type :checkbox, :radiobutton, or :dropdown) + question_type = @conditional_questions.keys.sample + conditional_question = @conditional_questions[question_type] + conditional_question_remove_option = conditional_question.question_options[0] + conditional_question_other_option = conditional_question.question_options[1] + answer_id = conditional_question.id + condition = create(:condition, question: conditional_question, + option_list: [conditional_question_remove_option.id], action_type: 'remove', remove_data: [@non_conditional_questions[:textarea][0].id, @non_conditional_questions[:textfield][1].id, @@ -58,163 +63,54 @@ go_to_write_plan_page_and_verify_answered - # Answer the checkbox_conditional_question. + # Answer the conditional_question within("#answer-form-#{answer_id}") do - check @conditional_questions[:checkbox].question_options[2].text + answer_conditional_question(conditional_question_remove_option, question_type) end check_answer_save_statuses(answer_id) check_question_and_answer_counts_for_plan(condition.remove_data) check_remove_data_effect_on_answer_form_selectors(condition.remove_data) - # Now uncheck checkbox_conditional_question answer. + question_option = determine_question_option(conditional_question_remove_option, + conditional_question_other_option, question_type) + # If :checkbox, uncheck the previously checked option + # Else, select a different :dropdown/:radiobutton option within("#answer-form-#{answer_id}") do - uncheck @conditional_questions[:checkbox].question_options[2].text + answer_conditional_question(question_option, question_type, 'uncheck') end check_answer_save_statuses(answer_id) num_questions, num_answers = question_and_answer_counts_for_plan - # Unchecking the conditional question should unhide all of the `remove_data` questions - # `- 1` from num_answers to account for now unchecked conditional question - expect(page).to have_text("#{num_answers - 1}/#{num_questions} answered") - end - - scenario 'User answers chooses checkbox option without a condition', :js do - answer_id = @conditional_questions[:checkbox].id - create(:condition, question: @conditional_questions[:checkbox], - option_list: [@conditional_questions[:checkbox].question_options[1].id], - action_type: 'remove', - remove_data: non_conditional_questions_ids_by_index(2)) - - create(:condition, question: @conditional_questions[:checkbox], - option_list: [@conditional_questions[:checkbox].question_options[2].id], - action_type: 'remove', - remove_data: non_conditional_questions_ids_by_index(0)) - - go_to_write_plan_page_and_verify_answered - - # Answer the checkbox_conditional_question - within("#answer-form-#{answer_id}") do - check @conditional_questions[:checkbox].question_options[0].text - end - - check_answer_save_statuses(answer_id) - check_question_and_answer_counts_for_plan - end - end - - feature 'User answers a radiobutton question with a condition' do - scenario 'User answers selects radiobutton option with a condition', :js do - answer_id = @conditional_questions[:radiobutton].id - condition = create(:condition, question: @conditional_questions[:radiobutton], - option_list: [@conditional_questions[:radiobutton].question_options[2].id], - action_type: 'remove', - remove_data: [@non_conditional_questions[:textarea][0].id, - @non_conditional_questions[:textfield][1].id, - @non_conditional_questions[:date][2].id, - @non_conditional_questions[:rda_metadata][0].id, - @non_conditional_questions[:checkbox][1].id, - @non_conditional_questions[:radiobutton][2].id, - @non_conditional_questions[:dropdown][0].id, - @non_conditional_questions[:multiselectbox][1].id]) - - go_to_write_plan_page_and_verify_answered - - # Answer the radiobutton_conditional_question. - within("#answer-form-#{answer_id}") do - choose @conditional_questions[:radiobutton].question_options[2].text - end - - check_answer_save_statuses(answer_id) - check_question_and_answer_counts_for_plan(condition.remove_data) - check_remove_data_effect_on_answer_form_selectors(condition.remove_data) - - # Now for radiobutton_conditional_question answer, there in no unchoose option, - # so we switch options to a different option without any conditions. - within("#answer-form-#{answer_id}") do - choose @conditional_questions[:radiobutton].question_options[0].text - end - - check_answer_save_statuses(answer_id) - check_question_and_answer_counts_for_plan - end - - scenario 'User answers selects radiobutton option without a condition', :js do - answer_id = @conditional_questions[:radiobutton].id - create(:condition, question: @conditional_questions[:radiobutton], - option_list: [@conditional_questions[:radiobutton].question_options[1].id], - action_type: 'remove', - remove_data: non_conditional_questions_ids_by_index(2)) - - create(:condition, question: @conditional_questions[:radiobutton], - option_list: [@conditional_questions[:radiobutton].question_options[2].id], - action_type: 'remove', - remove_data: non_conditional_questions_ids_by_index(0)) - - go_to_write_plan_page_and_verify_answered - - # Answer the radiobutton_conditional_question. - within("#answer-form-#{answer_id}") do - choose @conditional_questions[:radiobutton].question_options[0].text - end - - check_answer_save_statuses(answer_id) - check_question_and_answer_counts_for_plan - end - end - feature 'User answers a dropdown question with a condition' do - scenario 'User answers chooses dropdown option with a condition', :js do - answer_id = @conditional_questions[:dropdown].id - condition = create(:condition, question: @conditional_questions[:dropdown], - option_list: [@conditional_questions[:dropdown].question_options[2].id], - action_type: 'remove', - remove_data: [@non_conditional_questions[:textarea][0].id, - @non_conditional_questions[:textfield][1].id, - @non_conditional_questions[:date][2].id, - @non_conditional_questions[:rda_metadata][0].id, - @non_conditional_questions[:checkbox][1].id, - @non_conditional_questions[:radiobutton][2].id, - @non_conditional_questions[:dropdown][0].id, - @non_conditional_questions[:multiselectbox][1].id]) - - go_to_write_plan_page_and_verify_answered - - # Answer the dropdown_conditional_question - within("#answer-form-#{answer_id}") do - select(@conditional_questions[:dropdown].question_options[2].text, from: 'answer_question_option_ids') - end - - check_answer_save_statuses(answer_id) - check_question_and_answer_counts_for_plan(condition.remove_data) - check_remove_data_effect_on_answer_form_selectors(condition.remove_data) - - # Now select another option for dropdown_conditional_question. - within("#answer-form-#{answer_id}") do - select(@conditional_questions[:dropdown].question_options[1].text, from: 'answer_question_option_ids') - end - - check_answer_save_statuses(answer_id) - check_question_and_answer_counts_for_plan + # Undoing the conditional question should unhide all of the `remove_data` questions + # `-= 1` is needed for :checkbox because unchecking removes an answer + # (:dropdown and :radiobutton simply select a different answer) + num_answers -= 1 if question_type == :checkbox + expect(page).to have_text("#{num_answers}/#{num_questions} answered") end - scenario 'User answers select dropdown option without a condition', :js do - answer_id = @conditional_questions[:dropdown].id - create(:condition, question: @conditional_questions[:dropdown], - option_list: [@conditional_questions[:dropdown].question_options[1].id], + scenario 'User answers chooses an option without a condition', :js do + # Choose a conditional question at random (may be of type :checkbox, :radiobutton, or :dropdown) + question_type = @conditional_questions.keys.sample + conditional_question = @conditional_questions[question_type] + conditional_question_other_option = conditional_question.question_options[0] + answer_id = conditional_question.id + create(:condition, question: conditional_question, + option_list: [conditional_question.question_options[1].id], action_type: 'remove', remove_data: non_conditional_questions_ids_by_index(2)) - create(:condition, question: @conditional_questions[:dropdown], - option_list: [@conditional_questions[:dropdown].question_options[2].id], + create(:condition, question: conditional_question, + option_list: [conditional_question.question_options[2].id], action_type: 'remove', remove_data: non_conditional_questions_ids_by_index(0)) go_to_write_plan_page_and_verify_answered - # Answer the dropdown_conditional_question. + # Answer the conditional_question within("#answer-form-#{answer_id}") do - select(@conditional_questions[:dropdown].question_options[0].text, from: 'answer_question_option_ids') + answer_conditional_question(conditional_question_other_option, question_type) end check_answer_save_statuses(answer_id) @@ -222,17 +118,22 @@ end end end + describe 'conditions with action_type add_webhook' do - scenario 'User answers chooses checkbox option with a condition (with action_type: add_webhook)', :js do - answer_id = @conditional_questions[:checkbox].id - condition = create(:condition, :webhook, question: @conditional_questions[:checkbox], - option_list: [@conditional_questions[:checkbox].question_options[2].id]) + scenario 'User answers chooses an option with a condition (with action_type: add_webhook)', :js do + # Choose a conditional question at random (may be of type :checkbox, :radiobutton, or :dropdown) + question_type = @conditional_questions.keys.sample + conditional_question = @conditional_questions[question_type] + conditional_question_webhook_option = conditional_question.question_options[2] + answer_id = conditional_question.id + condition = create(:condition, :webhook, question: conditional_question, + option_list: [conditional_question_webhook_option.id]) go_to_write_plan_page_and_verify_answered - # Answer the checkbox_conditional_question. + # Answer the conditional_question within("#answer-form-#{answer_id}") do - check @conditional_questions[:checkbox].question_options[2].text + answer_conditional_question(conditional_question_webhook_option, question_type) end check_answer_save_statuses(answer_id) @@ -240,45 +141,6 @@ check_delivered_mail_for_webhook_data_and_question_data(JSON.parse(condition.webhook_data), :checkbox) end - - scenario 'User answers chooses radiobutton option with a condition (with action_type: add_webhook)', :js do - answer_id = @conditional_questions[:radiobutton].id - condition = create(:condition, - :webhook, - question: @conditional_questions[:radiobutton], - option_list: [@conditional_questions[:radiobutton].question_options[0].id]) - - go_to_write_plan_page_and_verify_answered - - # Now for radiobutton_conditional_question answer, there in no unchoose option, - # so we switch options to a different option without any conditions. - within("#answer-form-#{answer_id}") do - choose @conditional_questions[:radiobutton].question_options[0].text - end - - check_answer_save_statuses(answer_id) - check_question_and_answer_counts_for_plan - - check_delivered_mail_for_webhook_data_and_question_data(JSON.parse(condition.webhook_data), :radiobutton) - end - - scenario 'User answers chooses dropdown option with a condition (with action_type: add_webhook)', :js do - answer_id = @conditional_questions[:dropdown].id - condition = create(:condition, :webhook, question: @conditional_questions[:dropdown], - option_list: [@conditional_questions[:dropdown].question_options[2].id]) - - go_to_write_plan_page_and_verify_answered - - # Answer the dropdown_conditional_question - within("#answer-form-#{answer_id}") do - select(@conditional_questions[:dropdown].question_options[2].text, from: 'answer_question_option_ids') - end - - check_answer_save_statuses(answer_id) - check_question_and_answer_counts_for_plan - - check_delivered_mail_for_webhook_data_and_question_data(JSON.parse(condition.webhook_data), :dropdown) - end end private @@ -345,4 +207,28 @@ def check_answer_save_statuses(answer_id) expect(saved_span.text).to include('Answered just now') end end + + def answer_conditional_question(question_option, question_type, check_type = 'check') + case question_type + when :checkbox + # if it is a checkbox question, we need to know check_type as well ('check' vs 'uncheck') + if check_type == 'check' + check question_option.text + else + uncheck question_option.text + end + when :radiobutton + choose question_option.text + when :dropdown + select(question_option.text, from: 'answer_question_option_ids') + end + end + + def determine_question_option(original_question, new_question, question_type) + # If :checkbox question, we want to return the original question to be unchecked + return original_question if question_type == :checkbox + + # Else we want to return a different question to be selected via :radiobutton or :dropdown + new_question + end end