From b8b4fce58ebd9267cb78bf8b2e5c7260cf9c3905 Mon Sep 17 00:00:00 2001 From: briri Date: Mon, 2 Dec 2024 18:40:24 -0800 Subject: [PATCH 1/2] fixes for broken conditional logic after rails 7.2 upgrade --- .../org_admin/questions_controller.rb | 9 +- app/helpers/conditions_helper.rb | 6 +- app/models/question.rb | 25 +++++- .../org_admin/conditions/_container.html.erb | 2 +- app/views/org_admin/conditions/_form.html.erb | 88 ++++++++++++++----- app/views/org_admin/questions/_form.html.erb | 3 +- 6 files changed, 100 insertions(+), 33 deletions(-) diff --git a/app/controllers/org_admin/questions_controller.rb b/app/controllers/org_admin/questions_controller.rb index 5097038009..48cc970b1c 100644 --- a/app/controllers/org_admin/questions_controller.rb +++ b/app/controllers/org_admin/questions_controller.rb @@ -233,14 +233,15 @@ def sanitize_hash(param_conditions) res = {} hash_of_hashes = param_conditions[0] - hash_of_hashes.each do |cond_name, cond_hash| + + param_conditions.each do |cond_id, cond_hash| sanitized_hash = {} cond_hash.each do |k, v| - v = ActionController::Base.helpers.sanitize(v) if k.start_with?('webhook') - sanitized_hash[k] = v + sanitized_hash[k] = k.start_with?('webhook') ? ActionController::Base.helpers.sanitize(v) : v end - res[cond_name] = sanitized_hash + res[cond_id] = sanitized_hash end + res end diff --git a/app/helpers/conditions_helper.rb b/app/helpers/conditions_helper.rb index 430d1f1938..2df0a2ab1c 100644 --- a/app/helpers/conditions_helper.rb +++ b/app/helpers/conditions_helper.rb @@ -32,7 +32,7 @@ def answer_remove_list(answer, user = nil) rems = cond.remove_data.map(&:to_i) id_list += rems elsif !user.nil? - UserMailer.question_answered(JSON.parse(cond.webhook_data), user, answer, + UserMailer.question_answered(cond.webhook_data, user, answer, chosen.join(' and ')).deliver_now end end @@ -57,7 +57,7 @@ def email_trigger_list(answer) chosen = answer.question_option_ids.sort next unless chosen == opts - email_list << JSON.parse(cond.webhook_data)['email'] if action == 'add_webhook' + email_list << cond.webhook_data['email'] if action == 'add_webhook' end # uniq because could get same remove id from diff conds email_list.uniq.join(',') @@ -191,7 +191,7 @@ def condition_to_text(conditions) return_string += "
#{_('Answering')} " return_string += opts.join(' and ') if cond.action_type == 'add_webhook' - subject_string = text_formatted(JSON.parse(cond.webhook_data)['subject']) + subject_string = text_formatted(cond.webhook_data['subject']) return_string += format(_(' will send an email with subject %{subject_name}'), subject_name: subject_string) else diff --git a/app/models/question.rb b/app/models/question.rb index 7919cb0931..814a57f5f2 100644 --- a/app/models/question.rb +++ b/app/models/question.rb @@ -213,6 +213,28 @@ def update_conditions(param_conditions, old_to_new_opts, question_id_map) end end + + {"0"=>{ + "question_option"=>["3366"], + "action_type"=>"remove", + "remove_question_id"=>["13450"], + "number"=>"0", + "webhook-name"=>"", + "webhook-email"=>"", + "webhook-subject"=>"", + "webhook-message"=>"" + }, + "1"=>{ + "question_option"=>["3365"], + "action_type"=>"add_webhook", + "number"=>"1", + "webhook-name"=>"ME", + "webhook-email"=>"riley.bri@gmail.com", + "webhook-subject"=>"Testing conditional logic", + "webhook-message"=>"Testing conditional logic" + }} + + # rubocop:disable Metrics/MethodLength, Metrics/AbcSize def save_condition(value, opt_map, question_id_map) c = conditions.build @@ -220,6 +242,7 @@ def save_condition(value, opt_map, question_id_map) c.number = value['number'] # question options may have changed so rewrite them c.option_list = value['question_option'] + if opt_map.present? new_question_options = [] c.option_list.each do |qopt| @@ -243,7 +266,7 @@ def save_condition(value, opt_map, question_id_map) email: value['webhook-email'], subject: value['webhook-subject'], message: value['webhook-message'] - }.to_json + } end c.save end diff --git a/app/views/org_admin/conditions/_container.html.erb b/app/views/org_admin/conditions/_container.html.erb index bd54fbb5f3..9de4a77916 100644 --- a/app/views/org_admin/conditions/_container.html.erb +++ b/app/views/org_admin/conditions/_container.html.erb @@ -8,7 +8,7 @@ <%= label(:text, _('Action'), class: "control-label") %>
- <%= _('Remove')%> + <%= _('Target')%>
diff --git a/app/views/org_admin/conditions/_form.html.erb b/app/views/org_admin/conditions/_form.html.erb index 45c67d2f2c..689d00a954 100644 --- a/app/views/org_admin/conditions/_form.html.erb +++ b/app/views/org_admin/conditions/_form.html.erb @@ -1,7 +1,10 @@ -
+<% condition = condition ||= nil %> + +
<% - action_type_arr = [["removes", :remove], ["adds notification", :add_webhook]] - name_start = "conditions[]condition_" + condition_no.to_s + action_type_arr = [["removes", :remove], ["adds notification", :add_webhook]] + # name_start = "conditions[]condition_" + condition_no.to_s + name_start = "conditions[#{condition_no.to_s}]" remove_question_collection = later_question_list(question) condition_exists = local_assigns.has_key? :condition type_default = condition_exists ? (condition[:action_type] == "remove" ? :remove : :add_webhook) : :remove @@ -10,26 +13,65 @@ grouped_options_for_select(remove_question_collection) multiple = (question.question_format.multiselectbox? || question.question_format.checkbox?) %> -
- <%= select_tag(:question_option, options_from_collection_for_select(question.question_options.sort_by(&:number), "id", "text", - condition_exists ? condition[:question_option_id] : question.question_options.sort_by(&:number)[0]), {class: 'selectpicker regular', 'data-style': 'bg-white px-4 py-3', multiple: multiple, name: name_start + "[question_option][]"}) %> -
-
- <%= select_tag(:action_type, options_for_select(action_type_arr, type_default), {name: name_start + "[action_type]", class: 'action-type selectpicker narrow', 'data-style': 'bg-white px-4 py-3'}) %> -
-
-
- <%= select_tag(:remove_question_id, remove_question_group, {name: name_start + "[remove_question_id][]", class: 'selectpicker regular', multiple: true, 'data-style': 'bg-white px-4 py-3'}) %> -
-
- <%= link_to _('Edit email'), '#' %> -
-
- <%= hidden_field_tag(name_start + "[number]", condition_no) %> -
- <%= _('Remove') %> -
+ <%# If this is a new condition then display the interactive controls. otherwise just display the logic %> + <% if condition.nil? %> +
+ <%= select_tag(:question_option, options_from_collection_for_select(question.question_options.sort_by(&:number), "id", "text", + condition_exists ? condition[:question_option_id] : question.question_options.sort_by(&:number)[0]), {class: 'selectpicker regular', 'data-style': 'bg-white px-4 py-3', multiple: multiple, name: name_start + "[question_option][]"}) %> +
+
+ <%= select_tag(:action_type, options_for_select(action_type_arr, type_default), {name: name_start + "[action_type]", class: 'action-type selectpicker narrow', 'data-style': 'bg-white px-4 py-3'}) %> +
+
+
+ <%= select_tag(:remove_question_id, remove_question_group, {name: name_start + "[remove_question_id][]", class: 'selectpicker regular', multiple: true, 'data-style': 'bg-white px-4 py-3'}) %> +
+
+ <%= link_to _('Edit email'), '#' %> +
+
+ <%= hidden_field_tag(name_start + "[number]", condition_no) %> + +
+ <%= _('Remove') %> +
+ + <%= render partial: 'org_admin/conditions/webhook_form', locals: {name_start: name_start, condition_no: condition_no} %> + + <% else %> + <% + qopt = condition[:question_option_id].any? ? QuestionOption.find_by(id: condition[:question_option_id].first): nil + rques = condition[:remove_question_id].any? ? Question.find_by(id: condition[:remove_question_id].first) : nil + %> +
+ <%= qopt[:text]&.slice(0, 25) %> + <%= hidden_field_tag(name_start + "[question_option][]", condition[:question_option_id]) %> +
+
+ <%= condition[:action_type] == 'remove' ? 'Remove' : 'Email' %> + <%= hidden_field_tag(name_start + "[action_type]", condition[:action_type]) %> +
+
+ <% if condition[:remove_question_id].is_a?(Array) && condition[:remove_question_id].any? %> + Question <%= rques[:number] %>: <%= rques.text.gsub(%r{}, '').slice(0, 50) %> + <%= hidden_field_tag(name_start + "[remove_question_id][]", condition[:remove_question_id]) %> + <% else %> + <% + hook_tip = "Name: #{condition[:webhook_data]['name']}\nEmail: #{condition[:webhook_data]['email']}\n" + hook_tip += "Subject: #{condition[:webhook_data]['subject']}\nMessage: #{condition[:webhook_data]['message']}" + %> + <%= condition[:webhook_data]['email'] %> + <%= hidden_field_tag(name_start + "[webhook-email]", condition[:webhook_data]['email']) %> + <%= hidden_field_tag(name_start + "[webhook-name]", condition[:webhook_data]['name']) %> + <%= hidden_field_tag(name_start + "[webhook-subject]", condition[:webhook_data]['subject']) %> + <%= hidden_field_tag(name_start + "[webhook-message]", condition[:webhook_data]['message']) %> + <% end %> +
+ <%= hidden_field_tag(name_start + "[number]", condition_no) %> - <%= render partial: 'org_admin/conditions/webhook_form', locals: {name_start: name_start, condition_no: condition_no} %> +
+ <%= _('Remove') %> +
+ <% end %>
diff --git a/app/views/org_admin/questions/_form.html.erb b/app/views/org_admin/questions/_form.html.erb index 08446b08e5..0c599fa520 100644 --- a/app/views/org_admin/questions/_form.html.erb +++ b/app/views/org_admin/questions/_form.html.erb @@ -46,7 +46,8 @@
<% if question.id != nil && question.question_options[0].text != nil %> - <%= link_to _('Add Conditions'), org_admin_question_open_conditions_path(question_id: question.id, conditions: conditions), class: "add-logic btn btn-default", 'data-loaded': (conditions.size > 0).to_s, remote: true %> + <% cond_lbl = (!conditions.nil? && conditions.any?) ? 'Edit Conditions' : 'Add Conditions' %> + <%= link_to cond_lbl, org_admin_question_open_conditions_path(question_id: question.id, conditions: conditions), class: "add-logic btn btn-default", 'data-loaded': (conditions.size > 0).to_s, remote: true %>

<%= render partial: 'org_admin/conditions/container', locals: { f: f, question: question, conditions: conditions } %> From c0e58090e271e189c637b826d9e1ec93e3cd8c44 Mon Sep 17 00:00:00 2001 From: briri Date: Tue, 3 Dec 2024 08:20:24 -0800 Subject: [PATCH 2/2] cleanup of debug --- .../org_admin/questions_controller.rb | 3 --- app/models/question.rb | 22 ------------------- 2 files changed, 25 deletions(-) diff --git a/app/controllers/org_admin/questions_controller.rb b/app/controllers/org_admin/questions_controller.rb index 48cc970b1c..16a412c1ff 100644 --- a/app/controllers/org_admin/questions_controller.rb +++ b/app/controllers/org_admin/questions_controller.rb @@ -232,8 +232,6 @@ def sanitize_hash(param_conditions) return {} if param_conditions.empty? res = {} - hash_of_hashes = param_conditions[0] - param_conditions.each do |cond_id, cond_hash| sanitized_hash = {} cond_hash.each do |k, v| @@ -241,7 +239,6 @@ def sanitize_hash(param_conditions) end res[cond_id] = sanitized_hash end - res end diff --git a/app/models/question.rb b/app/models/question.rb index 814a57f5f2..1069929962 100644 --- a/app/models/question.rb +++ b/app/models/question.rb @@ -213,28 +213,6 @@ def update_conditions(param_conditions, old_to_new_opts, question_id_map) end end - - {"0"=>{ - "question_option"=>["3366"], - "action_type"=>"remove", - "remove_question_id"=>["13450"], - "number"=>"0", - "webhook-name"=>"", - "webhook-email"=>"", - "webhook-subject"=>"", - "webhook-message"=>"" - }, - "1"=>{ - "question_option"=>["3365"], - "action_type"=>"add_webhook", - "number"=>"1", - "webhook-name"=>"ME", - "webhook-email"=>"riley.bri@gmail.com", - "webhook-subject"=>"Testing conditional logic", - "webhook-message"=>"Testing conditional logic" - }} - - # rubocop:disable Metrics/MethodLength, Metrics/AbcSize def save_condition(value, opt_map, question_id_map) c = conditions.build