Fix issues with Conditional question serialization (offered by @briri

John Pinto · John Pinto · commit a3bd941780aa · 2024-12-19T12:12:29.000Z
from DMPTool). Based on DMPtool commit CDLUC3#667. Changes proposed in this PR (text from cited PR): Run SQL script to convert existing records in the conditions table so that they are JSON Arrays (see query below) Updated the form partials in app/views/org_admin/conditions/_form.erb.rb to properly send condition data to the controller. Removed all JSON.parse calls in the app/helpers/conditions.rb helper Made the following changes to simplify this patch and to make it a little more user friendly: The "Add Conditions" button will now say "Edit Conditions" if there are any. Updated the column heading over the "thing that happens when the condition is met" from "Remove" to "Target" since the content of the column can either be questions being removed or an email notification being sent. Conditions can be added or removed (not updated anymore) Hovering over the email of an existing condition displays a tooltip that shows the email message, subject, etc. Needed to update underlying data within the table so that they are JSON arrays using MySQL or Postgres scripts in db/migrate-sql-scripts folder: 20241219_UpdateConditions-MySql.sql.txt 20241219_UpdateConditions-Postgres.sql.txt (Note. The scripts have .txt endings to be able to get round .sql in .gitignore.)
diff --git a/app/controllers/org_admin/questions_controller.rb b/app/controllers/org_admin/questions_controller.rb
@@ -227,14 +227,12 @@ def sanitize_hash(param_conditions)
       return {} if param_conditions.empty?
 
       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
@@ -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(',')
@@ -190,7 +190,7 @@ def condition_to_text(conditions)
       return_string += "<dd>#{_('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 <b>send an email</b> with subject %{subject_name}'),
                                 subject_name: subject_string)
       else
diff --git a/app/models/question.rb b/app/models/question.rb
@@ -242,7 +242,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
@@ -4,14 +4,12 @@
       <div class="col-md-3 mb-2 bold">
         <%= label(:text, _('Option'), class: "control-label")%>
       </div>
-      <div class="col-md-3 mb-2">
+      <div class="col-md-6 mb-2">
         <%= label(:text, _('Action'), class: "control-label") %>
       </div>
       <div class="col-md-3 mb-2 bold">
         <%= _('Remove')%>
       </div>
-      <div class="col-md-3">
-      </div>
     </div>
     <% conditions_params =  conditions_to_param_form(conditions).sort_by { |key| key }.to_h %>
     <% conditions_params.each_with_index do |(title, params), i| %> <!-- iterate through conditions sorted by order of creation where i is condition number-->
diff --git 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 %>
+
 <div class="row condition-partial mb-3">
   <%
-   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?)
   %>
-  <div class="col-md-3 pe-2">
-    <%= 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: 'form-select regular', 'data-bs-style': 'dropdown-toggle bg-white px-4 py-3', name: name_start + "[question_option][]"}) %>
-  </div>
-  <div class="col-md-3 pe-2">
-    <%= select_tag(:action_type, options_for_select(action_type_arr, type_default), {name: name_start + "[action_type]", class: 'action-type form-select narrow', 'data-bs-style': 'dropdown-toggle bg-white px-4 py-3'}) %>
-  </div>
-  <div class="col-md-3 pe-2">
-    <div class="remove-dropdown">
-      <%= select_tag(:remove_question_id, remove_question_group, {name: name_start + "[remove_question_id][]", class: 'form-select regular', multiple: true, 'data-bs-style': 'dropdown-toggle bg-white px-4 py-3'}) %>
+
+  <%# If this is a new condition then display the interactive controls. otherwise just display the logic %>
+  <% if condition.nil? %>
+    <div class="col-md-3">
+      <%= 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: 'form-selectpicker regular', 'data-bs-style': 'dropdown-toggle bg-white px-4 py-3', multiple: multiple, name: name_start + "[question_option][]"}) %>
+    </div>
+    <div class="col-md-3 pe-2">
+      <%= select_tag(:action_type, options_for_select(action_type_arr, type_default), {name: name_start + "[action_type]", class: 'action-type form-selectpicker narrow', 'data-bs-style': 'dropdown-toggle bg-white px-4 py-3'}) %>
     </div>
-    <div class="webhook-replacement display-off my-auto text-center">
-      <%= link_to _('Edit email'), '#' %>
+    <div class="col-md-3">
+      <div class="remove-dropdown">
+        <%= select_tag(:remove_question_id, remove_question_group, {name: name_start + "[remove_question_id][]", class: 'form-selectpicker regular', multiple: true, 'data-bs-style': 'dropdown-toggle bg-white px-4 py-3'}) %>
+      </div>
+      <div class="webhook-replacement display-off my-auto text-center">
+        <%= link_to _('Edit email'), '#' %>
+      </div>
+    </div>
+    <%= hidden_field_tag(name_start + "[number]", condition_no) %>
+
+    <div class="col-md-3">
+      <a href="#anotherurl" class="delete-condition"><%= _('Remove') %></a>
     </div>
-  </div>
-  <%= hidden_field_tag(name_start + "[number]", condition_no) %>
 
-  <div class="col-md-3">
-    <a href="#anotherurl" class="delete-condition"><%= _('Remove') %></a>
-  </div>
+    <%= render partial: 'org_admin/conditions/webhook_form', locals: {name_start: name_start, condition_no: condition_no} %>
 
-  <%= 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
+    rquesArray = condition[:remove_question_id].any? ? Question.where(id: condition[:remove_question_id]) : []
+    %>
+
+    <div class="col-md-3 pe-2">
+      <%= condition[:action_type] == 'remove' ? 'Remove' : 'Email' %>
+      <%= hidden_field_tag(name_start + "[action_type]", condition[:action_type]) %>
+    </div>
+    <div class="col-md-6 pe-2">
+      <% if condition[:remove_question_id].is_a?(Array) && condition[:remove_question_id].any? %>  
+        <% rquesArray.each do |rques| %>
+           <%=  rques.text.gsub(%r{</?p>}, '').slice(0, 50)  %><br>
+        <% end %>
+        <%= 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']}"
+        %>
+        <span title="<%= hook_tip %>"><%= condition[:webhook_data]['email'] %></span>
+        <%= 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 %>
+    </div>
+    <%= hidden_field_tag(name_start + "[number]", condition_no) %>
+
+    <div class="col-md-3">
+      <a href="#anotherurl" class="delete-condition"><%= _('Remove') %></a>
+    </div>
+  <% end %>
 </div>
diff --git a/app/views/org_admin/questions/_form.html.erb b/app/views/org_admin/questions/_form.html.erb
@@ -45,8 +45,11 @@
         <%= render "/org_admin/question_options/option_fields", f: f, q: question %>
         <!--display for selecting comment box when multiple choice is being used-->
       </div>
+    
       <% 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-secondary", 'data-loaded': (conditions.size > 0).to_s, remote: true %>
+        <% cond_lbl = (!conditions.nil? && conditions.any?) ? 'Edit Conditions' : 'Add Conditions' %>
+        <%= cond_lbl %>
+        <%= 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 %>
         <div id="content" class="col-md-offset-2">
           <p>
             <%= render partial: 'org_admin/conditions/container', locals: { f: f, question: question, conditions: conditions } %>
diff --git a/db/migrate-sql-scripts/20241219_UpdateConditions-MySql.sql.txt b/db/migrate-sql-scripts/20241219_UpdateConditions-MySql.sql.txt
@@ -0,0 +1,34 @@
+UPDATE conditions
+SET
+    option_list = CONCAT (
+        CONCAT (
+            '[',
+            REPLACE (
+                REPLACE (
+                    REPLACE (option_list, '---\n- \'', '"'),
+                    '\'\n- \'',
+                    '","'
+                ),
+                '\'\n',
+                '"'
+            )
+        ),
+        ']'
+    ),
+    remove_data = CONCAT (
+        CONCAT (
+            '[',
+            REPLACE (
+                REPLACE (
+                    REPLACE (remove_data, '---\n- \'', '"'),
+                    '\'\n- \'',
+                    '","'
+                ),
+                '\'\n',
+                '"'
+            )
+        ),
+        ']'
+    )
+where
+    option_list like '---%';
diff --git a/db/migrate-sql-scripts/20241219_UpdateConditions-Postgres.sql.txt b/db/migrate-sql-scripts/20241219_UpdateConditions-Postgres.sql.txt
@@ -0,0 +1,34 @@
+UPDATE conditions
+SET
+    option_list = concat (
+        '[',
+        regexp_replace (
+            regexp_replace (
+                regexp_replace (option_list, '---(\r|\n)-', '', 'g'),
+                '(\r|\n)-',
+                ',',
+                'g'
+            ),
+            '\r|\n',
+            '',
+            'g'
+        ),
+        ']'
+    ),
+    remove_data = concat (
+        '[',
+        regexp_replace (
+            regexp_replace (
+                regexp_replace (remove_data, '---(\r|\n)-', '', 'g'),
+                '(\r|\n)-',
+                ',',
+                'g'
+            ),
+            '\r|\n',
+            '',
+            'g'
+        ),
+        ']'
+    )
+where
+    option_list like '---%';
