From c5fd8af0930e87539c506a2c737905c54841117d Mon Sep 17 00:00:00 2001 From: John Pinto Date: Thu, 27 Mar 2025 14:23:01 +0000 Subject: [PATCH 01/23] Fix issues with Conditional question serialization (offered by @briri from DMPTool).nBased on DMPtool commit CDLUC3#667. Changes proposed in this PR (text from cited PR): - There is a migration file with code for MySQL and Postgres to update the Conditions table to convert JSON Arrays in string format records in the conditions table so that they are JSON Arrays. - 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 - The user canno longer edit a condition. They need to remove it and create a new condition. This applies to the email for 'add notifications' too. 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 conditions for a given question. - 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 of any action type 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. The email content can no longer be edited once saved. It will need to be removed and re-created. - We allow only one question option to be selected when adding a Condition unlike inthe DMPTool version because experience with multiple options chosen has been problematic and buggy when used by users in DMPOnline. - To add a condition you must have selected an Option and Action together with: o if Action is 'remove', you need to select one or more choices in Target. o if Action is 'add notification', you need to fill in all the fields in the 'Send email' popup. Otherwise, the condition will not be saved. --- CHANGELOG.md | 1 + .../org_admin/questions_controller.rb | 8 +- app/helpers/conditions_helper.rb | 17 +- .../orgAdmin/conditions/updateConditions.js | 11 +- app/models/condition.rb | 8 +- app/models/question.rb | 45 ++- app/views/org_admin/conditions/_add.html.erb | 7 + .../org_admin/conditions/_container.html.erb | 7 +- app/views/org_admin/conditions/_form.html.erb | 91 ++++- .../conditions/_webhook_form.html.erb | 8 +- app/views/org_admin/questions/_form.html.erb | 6 +- ...816_update_conditions_json_columns_data.rb | 91 +++++ db/schema.rb | 351 ++++++++++++++++-- 13 files changed, 543 insertions(+), 108 deletions(-) create mode 100644 db/migrate/20250115102816_update_conditions_json_columns_data.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 961e2504fe..0af92cb483 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ - Add pdf handling in `render_respond_to_format_with_error_message` [#3482](https://github.com/DMPRoadmap/roadmap/pull/3482) - Lower PostgreSQL GitHub Action Chrome Version to Address Breaking Changes Between Latest Chrome Version (134) and `/features` Tests [#3491](https://github.com/DMPRoadmap/roadmap/pull/3491) - Bumped dependencies via `bundle update && yarn upgrade` [#3483](https://github.com/DMPRoadmap/roadmap/pull/3483) +- Fixed issues with Conditional Question serialization offered by @briri from PR https://github.com/CDLUC3/dmptool/pull/667 for DMPTool. There is a migration file with code for MySQL and Postgres to update the Conditions table to convert JSON Arrays in string format records in the conditions table so that they are JSON Arrays. ## v4.2.0 diff --git a/app/controllers/org_admin/questions_controller.rb b/app/controllers/org_admin/questions_controller.rb index 4e789df405..2fc4bc4942 100644 --- 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 index 9ea082a326..326e8d32f8 100644 --- a/app/helpers/conditions_helper.rb +++ b/app/helpers/conditions_helper.rb @@ -10,7 +10,7 @@ def remove_list(object) id_list = [] plan_answers = object.answers if object.is_a?(Plan) plan_answers = object[:answers] if object.is_a?(Hash) - return [] unless plan_answers.present? + return [] if plan_answers.blank? plan_answers.each { |answer| id_list += answer_remove_list(answer) } id_list @@ -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(',') @@ -70,7 +70,7 @@ def num_section_answers(plan, section) plan_remove_list = remove_list(plan) plan.answers.each do |answer| next unless answer.question.section_id == section.id && - !plan_remove_list.include?(answer.question_id) && + plan_remove_list.exclude?(answer.question_id) && section.question_ids.include?(answer.question_id) && answer.answered? @@ -107,10 +107,9 @@ def num_section_questions(plan, section, phase = nil) def sections_info(plan) return [] if plan.nil? - info = plan.sections.map do |section| + plan.sections.map do |section| section_info(plan, section) end - info || [] end def section_info(plan, section) @@ -190,7 +189,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 @@ -209,7 +208,7 @@ def condition_to_text(conditions) def text_formatted(object) text = Question.find(object).text if object.is_a?(Integer) text = object if object.is_a?(String) - return 'type error' unless text.present? + return 'type error' if text.blank? cleaned_text = text text = ActionController::Base.helpers.truncate(cleaned_text, length: DISPLAY_LENGTH, @@ -231,7 +230,7 @@ def conditions_to_param_form(conditions) webhook_data: condition.webhook_data } } if param_conditions.key?(title) param_conditions[title].merge!(condition_hash[title]) do |_key, val1, val2| - if val1.is_a?(Array) && !val1.include?(val2[0]) + if val1.is_a?(Array) && val1.exclude?(val2[0]) val1 + val2 else val1 diff --git a/app/javascript/src/orgAdmin/conditions/updateConditions.js b/app/javascript/src/orgAdmin/conditions/updateConditions.js index 2e65989cfb..7daa01eff5 100644 --- a/app/javascript/src/orgAdmin/conditions/updateConditions.js +++ b/app/javascript/src/orgAdmin/conditions/updateConditions.js @@ -15,11 +15,6 @@ export default function updateConditions(id) { addLogicButton.get(0).click(); } } - // set up form-select select boxes for condition options - const setSelectPicker = () => { - // $('.form-select.narrow').selectpicker({ width: 120 }); - // $('.form-select.regular').selectpicker({ width: 150 }); - }; // test if a webhook is selected and set up if so const allowWebhook = (selectObject, webhook = false) => { // webhook false => new condition @@ -30,8 +25,10 @@ export default function updateConditions(id) { // Retreive 'data-bs-target' for modal and create Jquery element const associatedModal = $(condition.find('.pseudo-webhook-btn').attr('data-bs-target')); associatedModal.modal('show'); + condition.find('.display-if-action-remove').hide(); } else { // condition type is remove condition.find('.remove-dropdown').show(); + condition.find('.display-if-action-remove').show(); condition.find('.webhook-replacement').hide(); } } else { // loading already saved conditions @@ -97,11 +94,10 @@ export default function updateConditions(id) { addLogicButton.attr('data-loaded', 'true'); addLogicButton.addClass('disabled'); addLogicButton.blur(); - addLogicButton.text('Conditions'); + addLogicButton.text('Edit Conditions'); if (isObject(content)) { content.html(e.detail[0].container); } - setSelectPicker(); webhookForm(e.detail[0].webhooks, undefined); }); @@ -114,7 +110,6 @@ export default function updateConditions(id) { conditionList.append(e.detail[0].attachment_partial); addDiv.html(e.detail[0].add_link); conditionList.attr('data-loaded', 'false'); - setSelectPicker(); const selectObject = conditionList.find('.form-select.action-type').last(); webhookForm(undefined, selectObject); } diff --git a/app/models/condition.rb b/app/models/condition.rb index fd13793009..3f8bb95107 100644 --- a/app/models/condition.rb +++ b/app/models/condition.rb @@ -27,10 +27,10 @@ # Object that represents a condition of a conditional question class Condition < ApplicationRecord belongs_to :question - enum action_type: %i[remove add_webhook] - serialize :option_list, type: Array - serialize :remove_data, type: Array - serialize :webhook_data, coder: JSON + enum :action_type, { remove: 0, add_webhook: 1 } + serialize :option_list, type: Array, coder: JSON + serialize :remove_data, type: Array, coder: JSON + serialize :webhook_data, type: Hash, coder: JSON # Sort order: Number ASC default_scope { order(number: :asc) } diff --git a/app/models/question.rb b/app/models/question.rb index 0afb049bda..97d41237db 100644 --- a/app/models/question.rb +++ b/app/models/question.rb @@ -145,7 +145,7 @@ def guidance_for_org(org) guidances = {} if theme_ids.any? GuidanceGroup.includes(guidances: :themes) - .where(org_id: org.id).each do |group| + .where(org_id: org.id).find_each do |group| group.guidances.each do |g| g.themes.each do |theme| guidances["#{group.name} " + _('guidance on') + " #{theme.title}"] = g if theme_ids.include? theme.id @@ -196,8 +196,8 @@ def annotations_per_org(org_id) type: Annotation.types[:example_answer]) guidance = annotations.find_by(org_id: org_id, type: Annotation.types[:guidance]) - example_answer = annotations.build(type: :example_answer, text: '', org_id: org_id) unless example_answer.present? - guidance = annotations.build(type: :guidance, text: '', org_id: org_id) unless guidance.present? + example_answer = annotations.build(type: :example_answer, text: '', org_id: org_id) if example_answer.blank? + guidance = annotations.build(type: :guidance, text: '', org_id: org_id) if guidance.blank? [example_answer, guidance] end @@ -206,7 +206,7 @@ def annotations_per_org(org_id) # after versioning def update_conditions(param_conditions, old_to_new_opts, question_id_map) conditions.destroy_all - return unless param_conditions.present? + return if param_conditions.blank? param_conditions.each_value do |value| save_condition(value, old_to_new_opts, question_id_map) @@ -221,20 +221,29 @@ 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'] - unless opt_map.blank? - new_question_options = c.option_list.map do |qopt| - opt_map[qopt] + + if opt_map.present? + new_question_options = [] + c.option_list.map do |qopt| + new_question_options << opt_map[qopt] end - c.option_list = new_question_options || [] + c.option_list = new_question_options end if value['action_type'] == 'remove' c.remove_data = value['remove_question_id'] - unless question_id_map.blank? - new_question_ids = c.remove_data.each do |qid| - question_id_map[qid] + if question_id_map.present? + new_question_ids = [] + c.remove_data.map do |qid| + new_question_ids << question_id_map[qid] end - c.remove_data = new_question_ids || [] + c.remove_data = new_question_ids + end + + # Do not save the condition if the option_list or remove_data is empty + if c.option_list.empty? || c.remove_data.empty? + c.destroy + return end else c.webhook_data = { @@ -242,7 +251,17 @@ def save_condition(value, opt_map, question_id_map) email: value['webhook-email'], subject: value['webhook-subject'], message: value['webhook-message'] - }.to_json + } + + # Do not save the condition if the option_list or if any webhook_data fields is empty + if c.option_list.empty? || + c.webhook_data['name'].blank? || + c.webhook_data['email'].blank? || + c.webhook_data['subject'].blank? || + c.webhook_data['message'].blank? + c.destroy + return + end end c.save end diff --git a/app/views/org_admin/conditions/_add.html.erb b/app/views/org_admin/conditions/_add.html.erb index 85126c6dd6..0ac1558234 100644 --- a/app/views/org_admin/conditions/_add.html.erb +++ b/app/views/org_admin/conditions/_add.html.erb @@ -1,5 +1,12 @@
<%= link_to _('Add condition'), new_org_admin_question_condition_path(question_id: question.id, condition_no: condition_no), remote: true, class: "add-condition" %> +

To add a condition you must have selected an Option and Action together with +

    +
  • if Action is 'remove', you need to select one or more choices in Target.
    • +
  • if Action is 'add notification', you need to fill in all the fields in the 'Send email' popup.
    • +
+ Otherwise, the condition will not be saved. +

diff --git a/app/views/org_admin/conditions/_container.html.erb b/app/views/org_admin/conditions/_container.html.erb index 03e4b96af6..0fccf1692c 100644 --- a/app/views/org_admin/conditions/_container.html.erb +++ b/app/views/org_admin/conditions/_container.html.erb @@ -4,13 +4,14 @@
<%= label(:text, _('Option'), class: "control-label")%>
-
+
<%= label(:text, _('Action'), class: "control-label") %>
- <%= _('Remove')%> + <%= label(:text, _('Target'), class: "control-label") %>
-
+
+ <%= label(:text, _('Remove'), class: "control-label") %>
<% conditions_params = conditions_to_param_form(conditions).sort_by { |key| key }.to_h %> diff --git a/app/views/org_admin/conditions/_form.html.erb b/app/views/org_admin/conditions/_form.html.erb index 8a48a3fc5c..f8a9058136 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 @@ -9,27 +12,77 @@ grouped_options_for_select(remove_question_collection, condition[:remove_question_id]) : grouped_options_for_select(remove_question_collection) multiple = (question.question_format.multiselectbox? || question.question_format.checkbox?) + view_email_content_info = _("Hover over the email address to view email content. To change email details you need to remove and add the condition again.") %> -
- <%= 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][]"}) %> -
-
- <%= 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'}) %> -
-
-
- <%= 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? %> +
Add condition
+
+
+
<%= _('Option') %>
+ <%= 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][]"}) %>
-
- <%= link_to _('Edit email'), '#' %> +
+
<%= _('Action') %>
+ <%= 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'}) %>
- <%= hidden_field_tag(name_start + "[number]", condition_no) %> - -
- <%= _('Remove') %> +
+
+
<%= _('Target') %>
+
+ <%= 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'}) %> +
+
+ <%= 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 + rquesArray = condition[:remove_question_id].any? ? Question.where(id: condition[:remove_question_id]) : 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 !rquesArray.nil? %> + <% rquesArray.each do |rques| %> + Question <%= rques[:number] %>: <%= rques.text.gsub(%r{}, '').slice(0, 50) %> + <%= '...' if rques.text.gsub(%r{}, '').length > 50 %> +
+ <% 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']}" + %> + <%= condition[:webhook_data]['email'] %> +
(<%= view_email_content_info %>) - <%= render partial: 'org_admin/conditions/webhook_form', locals: {name_start: name_start, condition_no: condition_no} %> + <%= 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) %> +
+
+ <%= _('Remove') %> +
+ <% end %>
diff --git a/app/views/org_admin/conditions/_webhook_form.html.erb b/app/views/org_admin/conditions/_webhook_form.html.erb index 8871af47c4..9b80a56743 100644 --- a/app/views/org_admin/conditions/_webhook_form.html.erb +++ b/app/views/org_admin/conditions/_webhook_form.html.erb @@ -9,25 +9,25 @@
- <%= label_tag name_start + "[webhook-name]", _('Name'), {'data-error': 'wrong', 'data-success': 'right', class: 'form-label'} %> + <%= label_tag name_start + "[webhook-name]", _('Name'), {'data-error': 'wrong', 'data-success': 'right', class: 'form-label', "aria-required": true, required: true} %> <%= text_field_tag name_start + "[webhook-name]", nil, class: 'form-control validate' %>
- <%= label_tag name_start + "[webhook-email]", _('Email'), {'data-error': 'wrong', 'data-success': 'right', class: 'form-label'} %> + <%= label_tag name_start + "[webhook-email]", _('Email'), {'data-error': 'wrong', 'data-success': 'right', class: 'form-label', "aria-required": true, required: true} %> <%= email_field_tag name_start + "[webhook-email]", nil, { placeholder: _('recipient_email@example.com'), class: 'form-control validate'} %>
- <%= label_tag name_start + "[webhook-subject]", _('Subject'), {'data-error': 'wrong', 'data-success': 'right', class: 'form-label'} %> + <%= label_tag name_start + "[webhook-subject]", _('Subject'), {'data-error': 'wrong', 'data-success': 'right', class: 'form-label', "aria-required": true, required: true} %> <%= text_field_tag name_start + "[webhook-subject]", nil, class: 'form-control validate' %>
- <%= label_tag name_start + "[webhook-message]", _('Message'), {'data-error': 'wrong', 'data-success': 'right', class: 'form-label'} %> + <%= label_tag name_start + "[webhook-message]", _('Message'), {'data-error': 'wrong', 'data-success': 'right', class: 'form-label', "aria-required": true, required: true}%> <%= text_area_tag name_start + "[webhook-message]", nil, { placeholder: _('Email content'), class: 'form-control md-textarea', rows: 4 } %>
diff --git a/app/views/org_admin/questions/_form.html.erb b/app/views/org_admin/questions/_form.html.erb index f966845301..f69ceccfa9 100644 --- a/app/views/org_admin/questions/_form.html.erb +++ b/app/views/org_admin/questions/_form.html.erb @@ -45,8 +45,10 @@ <%= render "/org_admin/question_options/option_fields", f: f, q: question %>
+ <% 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' %> + <%= link_to cond_lbl, 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 %>

<%= render partial: 'org_admin/conditions/container', locals: { f: f, question: question, conditions: conditions } %> @@ -54,7 +56,7 @@

<% else %>
- <%= link_to _('Add Conditions'), '#', class: "add-logic btn btn-secondary disabled" %> + <%= link_to _('Edit Conditions'), '#', class: "add-logic btn btn-secondary disabled" %>
<% end %>
diff --git a/db/migrate/20250115102816_update_conditions_json_columns_data.rb b/db/migrate/20250115102816_update_conditions_json_columns_data.rb new file mode 100644 index 0000000000..21605b7212 --- /dev/null +++ b/db/migrate/20250115102816_update_conditions_json_columns_data.rb @@ -0,0 +1,91 @@ +# This migration runs SQL for MySQL databases or POSTGRESQL database (default). +class UpdateConditionsJsonColumnsData < ActiveRecord::Migration[7.1] + # rubocop:disable Metrics/MethodLength + def change + if ActiveRecord::Base.connection.adapter_name.downcase.include?('mysql') + # MySQL sql + execute <<-SQL + UPDATE conditions + SET + option_list = CONCAT( + '[', + REPLACE( + REPLACE( + REPLACE(option_list, '---\r\n-', ''), + '\r\n-', + ',' + ), + '\r\n', + '' + ), + ']' + ), + remove_data = CONCAT( + '[', + REPLACE( + REPLACE( + REPLACE(remove_data, '---\r\n-', ''), + '\r\n-', + ',' + ), + '\r\n', + '' + ), + ']' + ) + WHERE option_list LIKE '---%'; + SQL + else + # POSTGRES SQL + execute <<-SQL + UPDATE conditions +SET + option_list = concat ( + '[', + regexp_replace ( + regexp_replace ( + regexp_replace ( + regexp_replace (option_list, '---(\r|\n)-', '', 'g'), + '(\r|\n)-', + ',', + 'g' + ), + '\r|\n', + '', + 'g' + ), + '''', + '"', + 'g'), + ']' + ), + remove_data = concat ( + '[', + regexp_replace ( + regexp_replace ( + regexp_replace ( + regexp_replace (remove_data, '---(\r|\n)-', '', 'g'), + '(\r|\n)-', + ',', + 'g' + ), + '\r|\n', + '', + 'g' + ), + '''', + '"', + 'g'), + ']' + ) +WHERE + option_list LIKE '---%'; + SQL + end + end + # rubocop:enable Metrics/MethodLength + + def down + # Add rollback logic if needed + end +end diff --git a/db/schema.rb b/db/schema.rb index ac3ee9619c..48bddc6e84 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,10 +10,25 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2024_06_13_141451) do +ActiveRecord::Schema[7.1].define(version: 2025_01_15_102816) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" + create_table "admin_users", id: :serial, force: :cascade do |t| + t.string "email", default: "", null: false + t.string "encrypted_password", default: "", null: false + t.string "reset_password_token" + t.datetime "reset_password_sent_at", precision: nil + t.datetime "remember_created_at", precision: nil + t.integer "sign_in_count", default: 0 + t.datetime "current_sign_in_at", precision: nil + t.datetime "last_sign_in_at", precision: nil + t.string "current_sign_in_ip" + t.string "last_sign_in_ip" + t.datetime "created_at", precision: nil + t.datetime "updated_at", precision: nil + end + create_table "annotations", id: :serial, force: :cascade do |t| t.integer "question_id" t.integer "org_id" @@ -22,7 +37,6 @@ t.datetime "created_at", precision: nil t.datetime "updated_at", precision: nil t.string "versionable_id", limit: 36 - t.index ["org_id"], name: "fk_rails_aca7521f72" t.index ["question_id"], name: "index_annotations_on_question_id" t.index ["versionable_id"], name: "index_annotations_on_versionable_id" end @@ -35,11 +49,14 @@ t.datetime "created_at", precision: nil t.datetime "updated_at", precision: nil t.integer "lock_version", default: 0 - t.index ["plan_id"], name: "fk_rails_84a6005a3e" t.index ["plan_id"], name: "index_answers_on_plan_id" - t.index ["question_id"], name: "fk_rails_3d5ed4418f" t.index ["question_id"], name: "index_answers_on_question_id" - t.index ["user_id"], name: "fk_rails_584be190c2" + end + + create_table "answers_options", id: false, force: :cascade do |t| + t.integer "answer_id", null: false + t.integer "option_id", null: false + t.index ["answer_id", "option_id"], name: "index_answers_options_on_answer_id_and_option_id" end create_table "answers_question_options", id: false, force: :cascade do |t| @@ -53,20 +70,21 @@ t.string "description" t.string "homepage" t.string "contact_name" - t.string "contact_email" + t.string "contact_email", null: false t.string "client_id", null: false t.string "client_secret", null: false t.datetime "last_access", precision: nil t.datetime "created_at", precision: nil, null: false t.datetime "updated_at", precision: nil, null: false t.integer "org_id" - t.text "redirect_uri" - t.string "scopes", default: "", null: false - t.boolean "confidential", default: true - t.boolean "trusted", default: false - t.integer "callback_method" - t.string "callback_uri" - t.index ["name"], name: "index_oauth_applications_on_name" + t.index ["name"], name: "index_api_clients_on_name" + end + + create_table "api_servers", force: :cascade do |t| + t.string "title" + t.string "description" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false end create_table "conditions", id: :serial, force: :cascade do |t| @@ -91,7 +109,6 @@ t.datetime "created_at", precision: nil t.datetime "updated_at", precision: nil t.index ["email"], name: "index_contributors_on_email" - t.index ["name", "id", "org_id"], name: "index_contrib_id_and_org_id" t.index ["org_id"], name: "index_contributors_on_org_id" t.index ["plan_id"], name: "index_contributors_on_plan_id" t.index ["roles"], name: "index_contributors_on_roles" @@ -106,6 +123,18 @@ t.index ["org_id"], name: "index_departments_on_org_id" end + create_table "dmptemplates", id: :serial, force: :cascade do |t| + t.string "title" + t.text "description" + t.boolean "published" + t.integer "user_id" + t.integer "organisation_id" + t.datetime "created_at", precision: nil + t.datetime "updated_at", precision: nil + t.string "locale" + t.boolean "is_default" + end + create_table "exported_plans", id: :serial, force: :cascade do |t| t.integer "plan_id" t.integer "user_id" @@ -130,6 +159,37 @@ t.index ["user_id"], name: "index_external_api_access_tokens_on_user_id" end + create_table "file_types", id: :serial, force: :cascade do |t| + t.string "name" + t.string "icon_name" + t.integer "icon_size" + t.string "icon_location" + t.datetime "created_at", precision: nil + t.datetime "updated_at", precision: nil + end + + create_table "file_uploads", id: :serial, force: :cascade do |t| + t.string "name" + t.string "title" + t.text "description" + t.integer "size" + t.boolean "published" + t.string "location" + t.integer "file_type_id" + t.datetime "created_at", precision: nil + t.datetime "updated_at", precision: nil + end + + create_table "friendly_id_slugs", id: :serial, force: :cascade do |t| + t.string "slug", null: false + t.integer "sluggable_id", null: false + t.string "sluggable_type", limit: 40 + t.datetime "created_at", precision: nil + t.index ["slug", "sluggable_type"], name: "index_friendly_id_slugs_on_slug_and_sluggable_type", unique: true + t.index ["sluggable_id"], name: "index_friendly_id_slugs_on_sluggable_id" + t.index ["sluggable_type"], name: "index_friendly_id_slugs_on_sluggable_type" + end + create_table "guidance_groups", id: :serial, force: :cascade do |t| t.string "name" t.integer "org_id" @@ -140,6 +200,12 @@ t.index ["org_id"], name: "index_guidance_groups_on_org_id" end + create_table "guidance_in_group", id: false, force: :cascade do |t| + t.integer "guidance_id", null: false + t.integer "guidance_group_id", null: false + t.index ["guidance_id", "guidance_group_id"], name: "index_guidance_in_group_on_guidance_id_and_guidance_group_id" + end + create_table "guidances", id: :serial, force: :cascade do |t| t.text "text" t.integer "guidance_group_id" @@ -155,10 +221,9 @@ t.boolean "active" t.datetime "created_at", precision: nil t.datetime "updated_at", precision: nil - t.string "logo_url" - t.string "identifier_prefix" + t.text "logo_url" + t.text "identifier_prefix" t.integer "context" - t.string "external_service" end create_table "identifiers", id: :serial, force: :cascade do |t| @@ -221,7 +286,6 @@ t.datetime "created_at", precision: nil t.datetime "updated_at", precision: nil t.index ["answer_id"], name: "index_notes_on_answer_id" - t.index ["user_id"], name: "fk_rails_7f2323ad43" end create_table "notification_acknowledgements", id: :serial, force: :cascade do |t| @@ -246,13 +310,95 @@ t.boolean "enabled", default: true end + create_table "oauth_access_grants", force: :cascade do |t| + t.bigint "resource_owner_id", null: false + t.bigint "application_id", null: false + t.string "token", null: false + t.integer "expires_in", null: false + t.text "redirect_uri", null: false + t.string "scopes", default: "", null: false + t.datetime "created_at", precision: nil, null: false + t.datetime "revoked_at", precision: nil + t.index ["application_id"], name: "index_oauth_access_grants_on_application_id" + t.index ["resource_owner_id"], name: "index_oauth_access_grants_on_resource_owner_id" + t.index ["token"], name: "index_oauth_access_grants_on_token", unique: true + end + + create_table "oauth_access_tokens", force: :cascade do |t| + t.bigint "resource_owner_id" + t.bigint "application_id", null: false + t.string "token", null: false + t.string "refresh_token" + t.integer "expires_in" + t.string "scopes" + t.datetime "created_at", precision: nil, null: false + t.datetime "revoked_at", precision: nil + t.string "previous_refresh_token", default: "", null: false + t.index ["application_id"], name: "index_oauth_access_tokens_on_application_id" + t.index ["refresh_token"], name: "index_oauth_access_tokens_on_refresh_token", unique: true + t.index ["resource_owner_id"], name: "index_oauth_access_tokens_on_resource_owner_id" + t.index ["token"], name: "index_oauth_access_tokens_on_token", unique: true + end + + create_table "oauth_applications", force: :cascade do |t| + t.string "name", null: false + t.string "uid", null: false + t.string "secret", null: false + t.text "redirect_uri", null: false + t.string "scopes", default: "", null: false + t.boolean "confidential", default: true, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["uid"], name: "index_oauth_applications_on_uid", unique: true + end + + create_table "option_warnings", id: :serial, force: :cascade do |t| + t.integer "organisation_id" + t.integer "option_id" + t.text "text" + t.datetime "created_at", precision: nil + t.datetime "updated_at", precision: nil + end + + create_table "options", id: :serial, force: :cascade do |t| + t.integer "question_id" + t.string "text" + t.integer "number" + t.boolean "is_default" + t.datetime "created_at", precision: nil + t.datetime "updated_at", precision: nil + end + create_table "org_token_permissions", id: :serial, force: :cascade do |t| t.integer "org_id" t.integer "token_permission_type_id" t.datetime "created_at", precision: nil t.datetime "updated_at", precision: nil t.index ["org_id"], name: "index_org_token_permissions_on_org_id" - t.index ["token_permission_type_id"], name: "fk_rails_2aa265f538" + end + + create_table "organisation_types", id: :serial, force: :cascade do |t| + t.string "name" + t.text "description" + t.datetime "created_at", precision: nil + t.datetime "updated_at", precision: nil + end + + create_table "organisations", id: :serial, force: :cascade do |t| + t.string "name" + t.string "abbreviation" + t.text "description" + t.string "target_url" + t.integer "logo_file_id" + t.integer "banner_file_id" + t.integer "organisation_type_id" + t.string "domain" + t.integer "wayfless_entity" + t.integer "stylesheet_file_id" + t.datetime "created_at", precision: nil + t.datetime "updated_at", precision: nil + t.integer "parent_id" + t.boolean "is_other" end create_table "orgs", id: :serial, force: :cascade do |t| @@ -269,15 +415,26 @@ t.string "contact_email" t.integer "org_type", default: 0, null: false t.text "links" + t.string "contact_name" t.boolean "feedback_enabled", default: false t.text "feedback_msg" - t.string "contact_name" t.boolean "managed", default: false, null: false - t.string "api_create_plan_email_subject" - t.text "api_create_plan_email_body" t.string "helpdesk_email" - t.index ["language_id"], name: "fk_rails_5640112cab" - t.index ["region_id"], name: "fk_rails_5a6adf6bab" + t.boolean "add_question_identifiers", default: false, null: false + end + + create_table "pages", id: :serial, force: :cascade do |t| + t.string "title" + t.text "body_text" + t.string "slug" + t.integer "menu" + t.integer "menu_position" + t.string "target_url" + t.string "location" + t.boolean "public" + t.integer "organisation_id" + t.datetime "created_at", precision: nil + t.datetime "updated_at", precision: nil end create_table "perms", id: :serial, force: :cascade do |t| @@ -299,6 +456,15 @@ t.index ["versionable_id"], name: "index_phases_on_versionable_id" end + create_table "plan_sections", id: :serial, force: :cascade do |t| + t.integer "user_id" + t.integer "section_id" + t.integer "plan_id" + t.datetime "created_at", precision: nil + t.datetime "updated_at", precision: nil + t.datetime "release_time", precision: nil + end + create_table "plans", id: :serial, force: :cascade do |t| t.string "title" t.integer "template_id" @@ -312,19 +478,17 @@ t.integer "org_id" t.integer "funder_id" t.integer "grant_id" - t.integer "api_client_id" t.datetime "start_date", precision: nil t.datetime "end_date", precision: nil + t.bigint "research_domain_id" t.boolean "ethical_issues" t.text "ethical_issues_description" t.string "ethical_issues_report" t.integer "funding_status" - t.bigint "research_domain_id" - t.index ["api_client_id"], name: "index_plans_on_api_client_id" t.index ["funder_id"], name: "index_plans_on_funder_id" t.index ["grant_id"], name: "index_plans_on_grant_id" t.index ["org_id"], name: "index_plans_on_org_id" - t.index ["research_domain_id"], name: "index_plans_on_fos_id" + t.index ["research_domain_id"], name: "index_plans_on_research_domain_id" t.index ["template_id"], name: "index_plans_on_template_id" end @@ -332,8 +496,6 @@ t.integer "guidance_group_id" t.integer "plan_id" t.index ["guidance_group_id", "plan_id"], name: "index_plans_guidance_groups_on_guidance_group_id_and_plan_id" - t.index ["guidance_group_id"], name: "fk_rails_ec1c5524d7" - t.index ["plan_id"], name: "fk_rails_13d0671430" end create_table "prefs", id: :serial, force: :cascade do |t| @@ -341,6 +503,40 @@ t.integer "user_id" end + create_table "project_groups", id: :serial, force: :cascade do |t| + t.boolean "project_creator" + t.boolean "project_editor" + t.integer "user_id" + t.integer "project_id" + t.datetime "created_at", precision: nil + t.datetime "updated_at", precision: nil + t.boolean "project_administrator" + end + + create_table "project_guidance", id: false, force: :cascade do |t| + t.integer "project_id", null: false + t.integer "guidance_group_id", null: false + t.index ["project_id", "guidance_group_id"], name: "index_project_guidance_on_project_id_and_guidance_group_id" + end + + create_table "projects", id: :serial, force: :cascade do |t| + t.string "title" + t.text "note" + t.boolean "locked" + t.integer "dmptemplate_id" + t.datetime "created_at", precision: nil + t.datetime "updated_at", precision: nil + t.string "slug" + t.integer "organisation_id" + t.string "grant_number" + t.string "identifier" + t.string "description" + t.string "principal_investigator" + t.string "principal_investigator_identifier" + t.string "data_contact" + t.index ["slug"], name: "index_projects_on_slug", unique: true + end + create_table "question_format_labels", id: false, force: :cascade do |t| t.integer "id" t.string "description" @@ -359,6 +555,16 @@ t.integer "formattype", default: 0 end + create_table "question_identifiers", force: :cascade do |t| + t.integer "question_id" + t.string "value" + t.string "name" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.string "versionable_id", limit: 36 + t.index ["versionable_id"], name: "index_question_identifiers_on_versionable_id" + end + create_table "question_options", id: :serial, force: :cascade do |t| t.integer "question_id" t.string "text" @@ -367,6 +573,7 @@ t.datetime "created_at", precision: nil t.datetime "updated_at", precision: nil t.string "versionable_id", limit: 36 + t.string "answer_identifier" t.index ["question_id"], name: "index_question_options_on_question_id" t.index ["versionable_id"], name: "index_question_options_on_versionable_id" end @@ -382,7 +589,6 @@ t.boolean "option_comment_display", default: true t.boolean "modifiable" t.string "versionable_id", limit: 36 - t.index ["question_format_id"], name: "fk_rails_4fbc38c8c7" t.index ["section_id"], name: "index_questions_on_section_id" t.index ["versionable_id"], name: "index_questions_on_versionable_id" end @@ -462,6 +668,7 @@ t.datetime "created_at", precision: nil, null: false t.datetime "updated_at", precision: nil, null: false t.bigint "license_id" + t.string "doi_url" t.index ["license_id"], name: "index_research_outputs_on_license_id" t.index ["output_type"], name: "index_research_outputs_on_output_type" t.index ["plan_id"], name: "index_research_outputs_on_plan_id" @@ -492,7 +699,7 @@ end create_table "sessions", id: :serial, force: :cascade do |t| - t.string "session_id", limit: 64, null: false + t.string "session_id", null: false t.text "data" t.datetime "created_at", precision: nil t.datetime "updated_at", precision: nil @@ -501,10 +708,10 @@ end create_table "settings", id: :serial, force: :cascade do |t| - t.string "var" + t.string "var", null: false t.text "value" t.integer "target_id", null: false - t.string "target_type" + t.string "target_type", null: false t.datetime "created_at", precision: nil, null: false t.datetime "updated_at", precision: nil, null: false end @@ -533,6 +740,14 @@ t.index ["subscriber_id", "subscriber_type", "plan_id"], name: "index_subscribers_on_identifiable_and_plan_id" end + create_table "suggested_answers", id: :serial, force: :cascade do |t| + t.integer "question_id" + t.integer "organisation_id" + t.text "text" + t.datetime "created_at", precision: nil + t.datetime "updated_at", precision: nil + end + create_table "templates", id: :serial, force: :cascade do |t| t.string "title" t.text "description" @@ -548,6 +763,8 @@ t.integer "family_id" t.boolean "archived" t.text "links" + t.bigint "api_server_id" + t.index ["api_server_id"], name: "index_templates_on_api_server_id" t.index ["family_id", "version"], name: "index_templates_on_family_id_and_version", unique: true t.index ["family_id"], name: "index_templates_on_family_id" t.index ["org_id", "family_id"], name: "template_organisation_dmptemplate_index" @@ -584,13 +801,42 @@ t.index ["org_id"], name: "index_trackers_on_org_id" end + create_table "user_org_roles", id: :serial, force: :cascade do |t| + t.integer "user_id" + t.integer "organisation_id" + t.integer "user_role_type_id" + t.datetime "created_at", precision: nil + t.datetime "updated_at", precision: nil + end + + create_table "user_role_types", id: :serial, force: :cascade do |t| + t.string "name" + t.text "description" + t.datetime "created_at", precision: nil + t.datetime "updated_at", precision: nil + end + + create_table "user_statuses", id: :serial, force: :cascade do |t| + t.string "name" + t.text "description" + t.datetime "created_at", precision: nil + t.datetime "updated_at", precision: nil + end + + create_table "user_types", id: :serial, force: :cascade do |t| + t.string "name" + t.text "description" + t.datetime "created_at", precision: nil + t.datetime "updated_at", precision: nil + end + create_table "users", id: :serial, force: :cascade do |t| t.string "firstname" t.string "surname" t.string "email", limit: 80, default: "", null: false t.datetime "created_at", precision: nil, null: false t.datetime "updated_at", precision: nil, null: false - t.string "encrypted_password" + t.string "encrypted_password", default: "" t.string "reset_password_token" t.datetime "reset_password_sent_at", precision: nil t.datetime "remember_created_at", precision: nil @@ -614,29 +860,43 @@ t.string "invited_by_type" t.integer "language_id" t.string "recovery_email" - t.string "ldap_password" - t.string "ldap_username" t.boolean "active", default: true t.integer "department_id" t.datetime "last_api_access", precision: nil - t.index ["department_id"], name: "fk_rails_f29bf9cdf2" - t.index ["email"], name: "index_users_on_email" - t.index ["language_id"], name: "fk_rails_45f4f12508" + t.index ["email"], name: "index_users_on_email", unique: true t.index ["org_id"], name: "index_users_on_org_id" end create_table "users_perms", id: false, force: :cascade do |t| t.integer "user_id" t.integer "perm_id" - t.index ["perm_id"], name: "fk_rails_457217c31c" t.index ["user_id"], name: "index_users_perms_on_user_id" end + create_table "users_roles", id: false, force: :cascade do |t| + t.integer "user_id" + t.integer "role_id" + t.index ["user_id", "role_id"], name: "index_users_roles_on_user_id_and_role_id" + end + + create_table "versions", id: :serial, force: :cascade do |t| + t.string "title" + t.text "description" + t.integer "published" + t.integer "number" + t.integer "phase_id" + t.datetime "created_at", precision: nil + t.datetime "updated_at", precision: nil + t.index ["phase_id"], name: "index_versions_on_phase_id" + end + add_foreign_key "annotations", "orgs" add_foreign_key "annotations", "questions" add_foreign_key "answers", "plans" add_foreign_key "answers", "questions" add_foreign_key "answers", "users" + add_foreign_key "answers_question_options", "answers" + add_foreign_key "answers_question_options", "question_options" add_foreign_key "conditions", "questions" add_foreign_key "guidance_groups", "orgs" add_foreign_key "guidances", "guidance_groups" @@ -644,6 +904,10 @@ add_foreign_key "notes", "users" add_foreign_key "notification_acknowledgements", "notifications" add_foreign_key "notification_acknowledgements", "users" + add_foreign_key "oauth_access_grants", "oauth_applications", column: "application_id" + add_foreign_key "oauth_access_grants", "users", column: "resource_owner_id" + add_foreign_key "oauth_access_tokens", "oauth_applications", column: "application_id" + add_foreign_key "oauth_access_tokens", "users", column: "resource_owner_id" add_foreign_key "org_token_permissions", "orgs" add_foreign_key "org_token_permissions", "token_permission_types" add_foreign_key "orgs", "languages" @@ -656,11 +920,14 @@ add_foreign_key "question_options", "questions" add_foreign_key "questions", "question_formats" add_foreign_key "questions", "sections" + add_foreign_key "questions_themes", "questions" + add_foreign_key "questions_themes", "themes" add_foreign_key "research_domains", "research_domains", column: "parent_id" add_foreign_key "research_outputs", "licenses" add_foreign_key "roles", "plans" add_foreign_key "roles", "users" add_foreign_key "sections", "phases" + add_foreign_key "templates", "api_servers" add_foreign_key "templates", "orgs" add_foreign_key "themes_in_guidance", "guidances" add_foreign_key "themes_in_guidance", "themes" @@ -668,4 +935,6 @@ add_foreign_key "users", "departments" add_foreign_key "users", "languages" add_foreign_key "users", "orgs" + add_foreign_key "users_perms", "perms" + add_foreign_key "users_perms", "users" end From 7ffc8156f993f3d82864111695a52fc486b07a8d Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Thu, 27 Mar 2025 11:37:10 -0600 Subject: [PATCH 02/23] `bin/rails db:migrate` --- db/schema.rb | 349 ++++++--------------------------------------------- 1 file changed, 40 insertions(+), 309 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index 48bddc6e84..3b6036f2bf 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -14,21 +14,6 @@ # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" - create_table "admin_users", id: :serial, force: :cascade do |t| - t.string "email", default: "", null: false - t.string "encrypted_password", default: "", null: false - t.string "reset_password_token" - t.datetime "reset_password_sent_at", precision: nil - t.datetime "remember_created_at", precision: nil - t.integer "sign_in_count", default: 0 - t.datetime "current_sign_in_at", precision: nil - t.datetime "last_sign_in_at", precision: nil - t.string "current_sign_in_ip" - t.string "last_sign_in_ip" - t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil - end - create_table "annotations", id: :serial, force: :cascade do |t| t.integer "question_id" t.integer "org_id" @@ -37,6 +22,7 @@ t.datetime "created_at", precision: nil t.datetime "updated_at", precision: nil t.string "versionable_id", limit: 36 + t.index ["org_id"], name: "fk_rails_aca7521f72" t.index ["question_id"], name: "index_annotations_on_question_id" t.index ["versionable_id"], name: "index_annotations_on_versionable_id" end @@ -49,14 +35,11 @@ t.datetime "created_at", precision: nil t.datetime "updated_at", precision: nil t.integer "lock_version", default: 0 + t.index ["plan_id"], name: "fk_rails_84a6005a3e" t.index ["plan_id"], name: "index_answers_on_plan_id" + t.index ["question_id"], name: "fk_rails_3d5ed4418f" t.index ["question_id"], name: "index_answers_on_question_id" - end - - create_table "answers_options", id: false, force: :cascade do |t| - t.integer "answer_id", null: false - t.integer "option_id", null: false - t.index ["answer_id", "option_id"], name: "index_answers_options_on_answer_id_and_option_id" + t.index ["user_id"], name: "fk_rails_584be190c2" end create_table "answers_question_options", id: false, force: :cascade do |t| @@ -70,21 +53,20 @@ t.string "description" t.string "homepage" t.string "contact_name" - t.string "contact_email", null: false + t.string "contact_email" t.string "client_id", null: false t.string "client_secret", null: false t.datetime "last_access", precision: nil t.datetime "created_at", precision: nil, null: false t.datetime "updated_at", precision: nil, null: false t.integer "org_id" - t.index ["name"], name: "index_api_clients_on_name" - end - - create_table "api_servers", force: :cascade do |t| - t.string "title" - t.string "description" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.text "redirect_uri" + t.string "scopes", default: "", null: false + t.boolean "confidential", default: true + t.boolean "trusted", default: false + t.integer "callback_method" + t.string "callback_uri" + t.index ["name"], name: "index_oauth_applications_on_name" end create_table "conditions", id: :serial, force: :cascade do |t| @@ -109,6 +91,7 @@ t.datetime "created_at", precision: nil t.datetime "updated_at", precision: nil t.index ["email"], name: "index_contributors_on_email" + t.index ["name", "id", "org_id"], name: "index_contrib_id_and_org_id" t.index ["org_id"], name: "index_contributors_on_org_id" t.index ["plan_id"], name: "index_contributors_on_plan_id" t.index ["roles"], name: "index_contributors_on_roles" @@ -123,18 +106,6 @@ t.index ["org_id"], name: "index_departments_on_org_id" end - create_table "dmptemplates", id: :serial, force: :cascade do |t| - t.string "title" - t.text "description" - t.boolean "published" - t.integer "user_id" - t.integer "organisation_id" - t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil - t.string "locale" - t.boolean "is_default" - end - create_table "exported_plans", id: :serial, force: :cascade do |t| t.integer "plan_id" t.integer "user_id" @@ -159,37 +130,6 @@ t.index ["user_id"], name: "index_external_api_access_tokens_on_user_id" end - create_table "file_types", id: :serial, force: :cascade do |t| - t.string "name" - t.string "icon_name" - t.integer "icon_size" - t.string "icon_location" - t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil - end - - create_table "file_uploads", id: :serial, force: :cascade do |t| - t.string "name" - t.string "title" - t.text "description" - t.integer "size" - t.boolean "published" - t.string "location" - t.integer "file_type_id" - t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil - end - - create_table "friendly_id_slugs", id: :serial, force: :cascade do |t| - t.string "slug", null: false - t.integer "sluggable_id", null: false - t.string "sluggable_type", limit: 40 - t.datetime "created_at", precision: nil - t.index ["slug", "sluggable_type"], name: "index_friendly_id_slugs_on_slug_and_sluggable_type", unique: true - t.index ["sluggable_id"], name: "index_friendly_id_slugs_on_sluggable_id" - t.index ["sluggable_type"], name: "index_friendly_id_slugs_on_sluggable_type" - end - create_table "guidance_groups", id: :serial, force: :cascade do |t| t.string "name" t.integer "org_id" @@ -200,12 +140,6 @@ t.index ["org_id"], name: "index_guidance_groups_on_org_id" end - create_table "guidance_in_group", id: false, force: :cascade do |t| - t.integer "guidance_id", null: false - t.integer "guidance_group_id", null: false - t.index ["guidance_id", "guidance_group_id"], name: "index_guidance_in_group_on_guidance_id_and_guidance_group_id" - end - create_table "guidances", id: :serial, force: :cascade do |t| t.text "text" t.integer "guidance_group_id" @@ -221,9 +155,10 @@ t.boolean "active" t.datetime "created_at", precision: nil t.datetime "updated_at", precision: nil - t.text "logo_url" - t.text "identifier_prefix" + t.string "logo_url" + t.string "identifier_prefix" t.integer "context" + t.string "external_service" end create_table "identifiers", id: :serial, force: :cascade do |t| @@ -286,6 +221,7 @@ t.datetime "created_at", precision: nil t.datetime "updated_at", precision: nil t.index ["answer_id"], name: "index_notes_on_answer_id" + t.index ["user_id"], name: "fk_rails_7f2323ad43" end create_table "notification_acknowledgements", id: :serial, force: :cascade do |t| @@ -310,95 +246,13 @@ t.boolean "enabled", default: true end - create_table "oauth_access_grants", force: :cascade do |t| - t.bigint "resource_owner_id", null: false - t.bigint "application_id", null: false - t.string "token", null: false - t.integer "expires_in", null: false - t.text "redirect_uri", null: false - t.string "scopes", default: "", null: false - t.datetime "created_at", precision: nil, null: false - t.datetime "revoked_at", precision: nil - t.index ["application_id"], name: "index_oauth_access_grants_on_application_id" - t.index ["resource_owner_id"], name: "index_oauth_access_grants_on_resource_owner_id" - t.index ["token"], name: "index_oauth_access_grants_on_token", unique: true - end - - create_table "oauth_access_tokens", force: :cascade do |t| - t.bigint "resource_owner_id" - t.bigint "application_id", null: false - t.string "token", null: false - t.string "refresh_token" - t.integer "expires_in" - t.string "scopes" - t.datetime "created_at", precision: nil, null: false - t.datetime "revoked_at", precision: nil - t.string "previous_refresh_token", default: "", null: false - t.index ["application_id"], name: "index_oauth_access_tokens_on_application_id" - t.index ["refresh_token"], name: "index_oauth_access_tokens_on_refresh_token", unique: true - t.index ["resource_owner_id"], name: "index_oauth_access_tokens_on_resource_owner_id" - t.index ["token"], name: "index_oauth_access_tokens_on_token", unique: true - end - - create_table "oauth_applications", force: :cascade do |t| - t.string "name", null: false - t.string "uid", null: false - t.string "secret", null: false - t.text "redirect_uri", null: false - t.string "scopes", default: "", null: false - t.boolean "confidential", default: true, null: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.index ["uid"], name: "index_oauth_applications_on_uid", unique: true - end - - create_table "option_warnings", id: :serial, force: :cascade do |t| - t.integer "organisation_id" - t.integer "option_id" - t.text "text" - t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil - end - - create_table "options", id: :serial, force: :cascade do |t| - t.integer "question_id" - t.string "text" - t.integer "number" - t.boolean "is_default" - t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil - end - create_table "org_token_permissions", id: :serial, force: :cascade do |t| t.integer "org_id" t.integer "token_permission_type_id" t.datetime "created_at", precision: nil t.datetime "updated_at", precision: nil t.index ["org_id"], name: "index_org_token_permissions_on_org_id" - end - - create_table "organisation_types", id: :serial, force: :cascade do |t| - t.string "name" - t.text "description" - t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil - end - - create_table "organisations", id: :serial, force: :cascade do |t| - t.string "name" - t.string "abbreviation" - t.text "description" - t.string "target_url" - t.integer "logo_file_id" - t.integer "banner_file_id" - t.integer "organisation_type_id" - t.string "domain" - t.integer "wayfless_entity" - t.integer "stylesheet_file_id" - t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil - t.integer "parent_id" - t.boolean "is_other" + t.index ["token_permission_type_id"], name: "fk_rails_2aa265f538" end create_table "orgs", id: :serial, force: :cascade do |t| @@ -415,26 +269,15 @@ t.string "contact_email" t.integer "org_type", default: 0, null: false t.text "links" - t.string "contact_name" t.boolean "feedback_enabled", default: false t.text "feedback_msg" + t.string "contact_name" t.boolean "managed", default: false, null: false + t.string "api_create_plan_email_subject" + t.text "api_create_plan_email_body" t.string "helpdesk_email" - t.boolean "add_question_identifiers", default: false, null: false - end - - create_table "pages", id: :serial, force: :cascade do |t| - t.string "title" - t.text "body_text" - t.string "slug" - t.integer "menu" - t.integer "menu_position" - t.string "target_url" - t.string "location" - t.boolean "public" - t.integer "organisation_id" - t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil + t.index ["language_id"], name: "fk_rails_5640112cab" + t.index ["region_id"], name: "fk_rails_5a6adf6bab" end create_table "perms", id: :serial, force: :cascade do |t| @@ -456,15 +299,6 @@ t.index ["versionable_id"], name: "index_phases_on_versionable_id" end - create_table "plan_sections", id: :serial, force: :cascade do |t| - t.integer "user_id" - t.integer "section_id" - t.integer "plan_id" - t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil - t.datetime "release_time", precision: nil - end - create_table "plans", id: :serial, force: :cascade do |t| t.string "title" t.integer "template_id" @@ -478,17 +312,19 @@ t.integer "org_id" t.integer "funder_id" t.integer "grant_id" + t.integer "api_client_id" t.datetime "start_date", precision: nil t.datetime "end_date", precision: nil - t.bigint "research_domain_id" t.boolean "ethical_issues" t.text "ethical_issues_description" t.string "ethical_issues_report" t.integer "funding_status" + t.bigint "research_domain_id" + t.index ["api_client_id"], name: "index_plans_on_api_client_id" t.index ["funder_id"], name: "index_plans_on_funder_id" t.index ["grant_id"], name: "index_plans_on_grant_id" t.index ["org_id"], name: "index_plans_on_org_id" - t.index ["research_domain_id"], name: "index_plans_on_research_domain_id" + t.index ["research_domain_id"], name: "index_plans_on_fos_id" t.index ["template_id"], name: "index_plans_on_template_id" end @@ -496,6 +332,8 @@ t.integer "guidance_group_id" t.integer "plan_id" t.index ["guidance_group_id", "plan_id"], name: "index_plans_guidance_groups_on_guidance_group_id_and_plan_id" + t.index ["guidance_group_id"], name: "fk_rails_ec1c5524d7" + t.index ["plan_id"], name: "fk_rails_13d0671430" end create_table "prefs", id: :serial, force: :cascade do |t| @@ -503,40 +341,6 @@ t.integer "user_id" end - create_table "project_groups", id: :serial, force: :cascade do |t| - t.boolean "project_creator" - t.boolean "project_editor" - t.integer "user_id" - t.integer "project_id" - t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil - t.boolean "project_administrator" - end - - create_table "project_guidance", id: false, force: :cascade do |t| - t.integer "project_id", null: false - t.integer "guidance_group_id", null: false - t.index ["project_id", "guidance_group_id"], name: "index_project_guidance_on_project_id_and_guidance_group_id" - end - - create_table "projects", id: :serial, force: :cascade do |t| - t.string "title" - t.text "note" - t.boolean "locked" - t.integer "dmptemplate_id" - t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil - t.string "slug" - t.integer "organisation_id" - t.string "grant_number" - t.string "identifier" - t.string "description" - t.string "principal_investigator" - t.string "principal_investigator_identifier" - t.string "data_contact" - t.index ["slug"], name: "index_projects_on_slug", unique: true - end - create_table "question_format_labels", id: false, force: :cascade do |t| t.integer "id" t.string "description" @@ -555,16 +359,6 @@ t.integer "formattype", default: 0 end - create_table "question_identifiers", force: :cascade do |t| - t.integer "question_id" - t.string "value" - t.string "name" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.string "versionable_id", limit: 36 - t.index ["versionable_id"], name: "index_question_identifiers_on_versionable_id" - end - create_table "question_options", id: :serial, force: :cascade do |t| t.integer "question_id" t.string "text" @@ -573,7 +367,6 @@ t.datetime "created_at", precision: nil t.datetime "updated_at", precision: nil t.string "versionable_id", limit: 36 - t.string "answer_identifier" t.index ["question_id"], name: "index_question_options_on_question_id" t.index ["versionable_id"], name: "index_question_options_on_versionable_id" end @@ -589,6 +382,7 @@ t.boolean "option_comment_display", default: true t.boolean "modifiable" t.string "versionable_id", limit: 36 + t.index ["question_format_id"], name: "fk_rails_4fbc38c8c7" t.index ["section_id"], name: "index_questions_on_section_id" t.index ["versionable_id"], name: "index_questions_on_versionable_id" end @@ -668,7 +462,6 @@ t.datetime "created_at", precision: nil, null: false t.datetime "updated_at", precision: nil, null: false t.bigint "license_id" - t.string "doi_url" t.index ["license_id"], name: "index_research_outputs_on_license_id" t.index ["output_type"], name: "index_research_outputs_on_output_type" t.index ["plan_id"], name: "index_research_outputs_on_plan_id" @@ -699,7 +492,7 @@ end create_table "sessions", id: :serial, force: :cascade do |t| - t.string "session_id", null: false + t.string "session_id", limit: 64, null: false t.text "data" t.datetime "created_at", precision: nil t.datetime "updated_at", precision: nil @@ -708,10 +501,10 @@ end create_table "settings", id: :serial, force: :cascade do |t| - t.string "var", null: false + t.string "var" t.text "value" t.integer "target_id", null: false - t.string "target_type", null: false + t.string "target_type" t.datetime "created_at", precision: nil, null: false t.datetime "updated_at", precision: nil, null: false end @@ -740,14 +533,6 @@ t.index ["subscriber_id", "subscriber_type", "plan_id"], name: "index_subscribers_on_identifiable_and_plan_id" end - create_table "suggested_answers", id: :serial, force: :cascade do |t| - t.integer "question_id" - t.integer "organisation_id" - t.text "text" - t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil - end - create_table "templates", id: :serial, force: :cascade do |t| t.string "title" t.text "description" @@ -763,8 +548,6 @@ t.integer "family_id" t.boolean "archived" t.text "links" - t.bigint "api_server_id" - t.index ["api_server_id"], name: "index_templates_on_api_server_id" t.index ["family_id", "version"], name: "index_templates_on_family_id_and_version", unique: true t.index ["family_id"], name: "index_templates_on_family_id" t.index ["org_id", "family_id"], name: "template_organisation_dmptemplate_index" @@ -801,42 +584,13 @@ t.index ["org_id"], name: "index_trackers_on_org_id" end - create_table "user_org_roles", id: :serial, force: :cascade do |t| - t.integer "user_id" - t.integer "organisation_id" - t.integer "user_role_type_id" - t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil - end - - create_table "user_role_types", id: :serial, force: :cascade do |t| - t.string "name" - t.text "description" - t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil - end - - create_table "user_statuses", id: :serial, force: :cascade do |t| - t.string "name" - t.text "description" - t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil - end - - create_table "user_types", id: :serial, force: :cascade do |t| - t.string "name" - t.text "description" - t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil - end - create_table "users", id: :serial, force: :cascade do |t| t.string "firstname" t.string "surname" t.string "email", limit: 80, default: "", null: false t.datetime "created_at", precision: nil, null: false t.datetime "updated_at", precision: nil, null: false - t.string "encrypted_password", default: "" + t.string "encrypted_password" t.string "reset_password_token" t.datetime "reset_password_sent_at", precision: nil t.datetime "remember_created_at", precision: nil @@ -860,43 +614,29 @@ t.string "invited_by_type" t.integer "language_id" t.string "recovery_email" + t.string "ldap_password" + t.string "ldap_username" t.boolean "active", default: true t.integer "department_id" t.datetime "last_api_access", precision: nil - t.index ["email"], name: "index_users_on_email", unique: true + t.index ["department_id"], name: "fk_rails_f29bf9cdf2" + t.index ["email"], name: "index_users_on_email" + t.index ["language_id"], name: "fk_rails_45f4f12508" t.index ["org_id"], name: "index_users_on_org_id" end create_table "users_perms", id: false, force: :cascade do |t| t.integer "user_id" t.integer "perm_id" + t.index ["perm_id"], name: "fk_rails_457217c31c" t.index ["user_id"], name: "index_users_perms_on_user_id" end - create_table "users_roles", id: false, force: :cascade do |t| - t.integer "user_id" - t.integer "role_id" - t.index ["user_id", "role_id"], name: "index_users_roles_on_user_id_and_role_id" - end - - create_table "versions", id: :serial, force: :cascade do |t| - t.string "title" - t.text "description" - t.integer "published" - t.integer "number" - t.integer "phase_id" - t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil - t.index ["phase_id"], name: "index_versions_on_phase_id" - end - add_foreign_key "annotations", "orgs" add_foreign_key "annotations", "questions" add_foreign_key "answers", "plans" add_foreign_key "answers", "questions" add_foreign_key "answers", "users" - add_foreign_key "answers_question_options", "answers" - add_foreign_key "answers_question_options", "question_options" add_foreign_key "conditions", "questions" add_foreign_key "guidance_groups", "orgs" add_foreign_key "guidances", "guidance_groups" @@ -904,10 +644,6 @@ add_foreign_key "notes", "users" add_foreign_key "notification_acknowledgements", "notifications" add_foreign_key "notification_acknowledgements", "users" - add_foreign_key "oauth_access_grants", "oauth_applications", column: "application_id" - add_foreign_key "oauth_access_grants", "users", column: "resource_owner_id" - add_foreign_key "oauth_access_tokens", "oauth_applications", column: "application_id" - add_foreign_key "oauth_access_tokens", "users", column: "resource_owner_id" add_foreign_key "org_token_permissions", "orgs" add_foreign_key "org_token_permissions", "token_permission_types" add_foreign_key "orgs", "languages" @@ -920,14 +656,11 @@ add_foreign_key "question_options", "questions" add_foreign_key "questions", "question_formats" add_foreign_key "questions", "sections" - add_foreign_key "questions_themes", "questions" - add_foreign_key "questions_themes", "themes" add_foreign_key "research_domains", "research_domains", column: "parent_id" add_foreign_key "research_outputs", "licenses" add_foreign_key "roles", "plans" add_foreign_key "roles", "users" add_foreign_key "sections", "phases" - add_foreign_key "templates", "api_servers" add_foreign_key "templates", "orgs" add_foreign_key "themes_in_guidance", "guidances" add_foreign_key "themes_in_guidance", "themes" @@ -935,6 +668,4 @@ add_foreign_key "users", "departments" add_foreign_key "users", "languages" add_foreign_key "users", "orgs" - add_foreign_key "users_perms", "perms" - add_foreign_key "users_perms", "users" end From da93d722091662fa696ecf4e0f827299cc974f80 Mon Sep 17 00:00:00 2001 From: John Pinto Date: Wed, 2 Apr 2025 11:07:01 +0100 Subject: [PATCH 03/23] Fix for Conditional model bug for the webhook_data which was typed as a Hash. Removing type fixes issue. --- app/models/condition.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/condition.rb b/app/models/condition.rb index 3f8bb95107..4887de31df 100644 --- a/app/models/condition.rb +++ b/app/models/condition.rb @@ -30,7 +30,7 @@ class Condition < ApplicationRecord enum :action_type, { remove: 0, add_webhook: 1 } serialize :option_list, type: Array, coder: JSON serialize :remove_data, type: Array, coder: JSON - serialize :webhook_data, type: Hash, coder: JSON + serialize :webhook_data, coder: JSON # Sort order: Number ASC default_scope { order(number: :asc) } From dba3999b5bf97f25b7dbd9f73c80a6fd104afd4d Mon Sep 17 00:00:00 2001 From: John Pinto Date: Thu, 3 Apr 2025 10:41:33 +0100 Subject: [PATCH 04/23] Updated the comment for param_conditions parameter for method sanitize_hash in QuestionsController to reflect the change structure. It is now a hash of a hash. --- .../org_admin/questions_controller.rb | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/app/controllers/org_admin/questions_controller.rb b/app/controllers/org_admin/questions_controller.rb index 2fc4bc4942..77fed52609 100644 --- a/app/controllers/org_admin/questions_controller.rb +++ b/app/controllers/org_admin/questions_controller.rb @@ -211,17 +211,21 @@ def destroy private - # param_conditions looks like: - # [ - # { - # "conditions_N" => { - # name: ... - # subject ... - # ... - # } - # ... - # } - # ] + # param_conditions is a hash of a hash like this (example where + # action_types is "remove" and "add_webhook" respectively): + # Parameters: + # {"0"=>{"question_option"=>["212159"], + # "action_type"=>"remove", + # "remove_question_id"=>["191471 191472"], + # "number"=>"0"}, + # "1"=>{"question_option"=>["212160"], + # "action_type"=>"add_webhook", + # "number"=>"1", + # "webhook-name"=>"DMP Admin", + # "webhook-email"=>"dmp-admin@example.com", + # "webhook-subject"=>"Woodcote cillum quis elit consectetur epsom", + # "webhook-message"=>"Labore ut epsom downs exercitation ...."} + # } def sanitize_hash(param_conditions) return {} if param_conditions.nil? return {} if param_conditions.empty? From a58017c934aaf742e8be9b9e0843f0192bd119fe Mon Sep 17 00:00:00 2001 From: John Pinto Date: Thu, 3 Apr 2025 18:47:36 +0100 Subject: [PATCH 05/23] Updated the check (!conditions.nil? && conditions.any?) in tag from <% cond_lbl = (!conditions.nil? && conditions.any?) ? 'Edit Conditions' : 'Add Conditions' %> to concise version <% cond_lbl = conditions&.any? ? 'Edit Conditions' : 'Add Conditions' %>. --- app/views/org_admin/questions/_form.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/org_admin/questions/_form.html.erb b/app/views/org_admin/questions/_form.html.erb index f69ceccfa9..5d715390d1 100644 --- a/app/views/org_admin/questions/_form.html.erb +++ b/app/views/org_admin/questions/_form.html.erb @@ -47,7 +47,7 @@
<% if question.id != nil && question.question_options[0].text != nil %> - <% cond_lbl = (!conditions.nil? && conditions.any?) ? 'Edit Conditions' : 'Add Conditions' %> + <% cond_lbl = 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-secondary", 'data-loaded': (conditions.size > 0).to_s, remote: true %>

From 06b0db5326c50be6bfcb0ac9a8d8401f29f41971 Mon Sep 17 00:00:00 2001 From: John Pinto Date: Fri, 4 Apr 2025 10:29:33 +0100 Subject: [PATCH 06/23] Updated the tag in app/views/org_admin/conditions/_form.html.erb <% condition = condition ||= nil %> with concise expression as in <% condition ||= nil %>. --- app/views/org_admin/conditions/_form.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/org_admin/conditions/_form.html.erb b/app/views/org_admin/conditions/_form.html.erb index f8a9058136..3a7877f098 100644 --- a/app/views/org_admin/conditions/_form.html.erb +++ b/app/views/org_admin/conditions/_form.html.erb @@ -1,4 +1,4 @@ -<% condition = condition ||= nil %> +<% condition ||= nil %>

<% From 739aad0e1e51b2031d67984d30a084468ecff191 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Fri, 4 Apr 2025 10:38:10 -0600 Subject: [PATCH 07/23] Refactor mapping of `remove_data` & `option_list` This change uses `.map` to refactor/simplify the mapping of `option_list` and `remove_data`. It also removes the needed for temporary arrays. --- app/models/question.rb | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/app/models/question.rb b/app/models/question.rb index 97d41237db..58bfa7533f 100644 --- a/app/models/question.rb +++ b/app/models/question.rb @@ -221,24 +221,11 @@ 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.map do |qopt| - new_question_options << opt_map[qopt] - end - c.option_list = new_question_options - end + c.option_list = c.option_list.map { |qopt| opt_map[qopt] } if opt_map.present? if value['action_type'] == 'remove' c.remove_data = value['remove_question_id'] - if question_id_map.present? - new_question_ids = [] - c.remove_data.map do |qid| - new_question_ids << question_id_map[qid] - end - c.remove_data = new_question_ids - end + c.remove_data = c.remove_data.map { |qid| question_id_map[qid] } if question_id_map.present? # Do not save the condition if the option_list or remove_data is empty if c.option_list.empty? || c.remove_data.empty? From f6a232b47f2c8ff3ee594a3670130377652597ee Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Fri, 4 Apr 2025 11:41:53 -0600 Subject: [PATCH 08/23] Refactor webhook_data validation and construction Refactored `save_condition` method via new `handle_webhook_data` helper method. - Helper method returns nil if any of the required webhook_data fields are absent. Else, it constructs and returns the webhook_data hash - Returning nil when any fields are absent enables us to simplify the if check that immediately follows (now line 240) to `if c.option_list.empty? || c.webhook_data.nil?` - Overall, these changes simplify the `save_condition` method and even remove a previous rubocop offense. --- app/models/question.rb | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/app/models/question.rb b/app/models/question.rb index 58bfa7533f..cdaf3df006 100644 --- a/app/models/question.rb +++ b/app/models/question.rb @@ -213,7 +213,7 @@ def update_conditions(param_conditions, old_to_new_opts, question_id_map) end end - # rubocop:disable Metrics/MethodLength, Metrics/AbcSize + # rubocop:disable Metrics/AbcSize # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity def save_condition(value, opt_map, question_id_map) c = conditions.build @@ -233,19 +233,10 @@ def save_condition(value, opt_map, question_id_map) return end else - c.webhook_data = { - name: value['webhook-name'], - email: value['webhook-email'], - subject: value['webhook-subject'], - message: value['webhook-message'] - } - - # Do not save the condition if the option_list or if any webhook_data fields is empty - if c.option_list.empty? || - c.webhook_data['name'].blank? || - c.webhook_data['email'].blank? || - c.webhook_data['subject'].blank? || - c.webhook_data['message'].blank? + c.webhook_data = handle_webhook_data(value) + + # Do not save the condition if the option_list is empty or webhook_data is nil + if c.option_list.empty? || c.webhook_data.nil? c.destroy return end @@ -253,7 +244,7 @@ def save_condition(value, opt_map, question_id_map) c.save end # rubocop:enable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity - # rubocop:enable Metrics/MethodLength, Metrics/AbcSize + # rubocop:enable Metrics/AbcSize private @@ -280,4 +271,17 @@ def check_remove_conditions end end # rubocop:enable Metrics/AbcSize + + def handle_webhook_data(value) + # return nil if any of the webhook fields are blank + return if %w[webhook-name webhook-email webhook-subject webhook-message].any? { |key| value[key].blank? } + + # else return the constructed webhook_data hash + { + name: value['webhook-name'], + email: value['webhook-email'], + subject: value['webhook-subject'], + message: value['webhook-message'] + } + end end From 82984e25bdd039246a8dfebb8d0ab78b57fe4f17 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Fri, 4 Apr 2025 11:55:00 -0600 Subject: [PATCH 09/23] Refactor handling of `c.option_list.empty?` - Moved `c.option_list.empty?` immediately after `c.option_list` assignment to streamline logic. - This change reduces unnecessary processing of `c.remove_data` and `c.webhook_data`. - Isolating the `c.option_list.empty?` check also simplifies subsequent checks for `c.remove_data.empty?` and c.webhook_data.nil?` later in the code. --- app/models/question.rb | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/app/models/question.rb b/app/models/question.rb index cdaf3df006..9eb035d359 100644 --- a/app/models/question.rb +++ b/app/models/question.rb @@ -222,21 +222,24 @@ def save_condition(value, opt_map, question_id_map) # question options may have changed so rewrite them c.option_list = value['question_option'] c.option_list = c.option_list.map { |qopt| opt_map[qopt] } if opt_map.present? + # Do not save the condition if the option_list is empty + if c.option_list.empty? + c.destroy + return + end if value['action_type'] == 'remove' c.remove_data = value['remove_question_id'] c.remove_data = c.remove_data.map { |qid| question_id_map[qid] } if question_id_map.present? - - # Do not save the condition if the option_list or remove_data is empty - if c.option_list.empty? || c.remove_data.empty? + # Do not save the condition if remove_data is empty + if c.remove_data.empty? c.destroy return end else c.webhook_data = handle_webhook_data(value) - - # Do not save the condition if the option_list is empty or webhook_data is nil - if c.option_list.empty? || c.webhook_data.nil? + # Do not save the condition if webhook_data is nil + if c.webhook_data.nil? c.destroy return end From 49b9f7d4408c68d77594a59ae57de18c6a5e1d31 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Fri, 4 Apr 2025 12:48:35 -0600 Subject: [PATCH 10/23] Refactor option_list and remove_data handling - Moved logic for handling option_list and remove_data into separate helper methods (handle_option_list and handle_remove_data). - This simplifies our `save_condition` method and resolves some previously disabled rubocop offenses. --- app/models/question.rb | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/app/models/question.rb b/app/models/question.rb index 9eb035d359..2c7031a3c5 100644 --- a/app/models/question.rb +++ b/app/models/question.rb @@ -214,14 +214,13 @@ def update_conditions(param_conditions, old_to_new_opts, question_id_map) end # rubocop:disable Metrics/AbcSize - # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity def save_condition(value, opt_map, question_id_map) c = conditions.build c.action_type = value['action_type'] c.number = value['number'] + # question options may have changed so rewrite them - c.option_list = value['question_option'] - c.option_list = c.option_list.map { |qopt| opt_map[qopt] } if opt_map.present? + c.option_list = handle_option_list(value, opt_map) # Do not save the condition if the option_list is empty if c.option_list.empty? c.destroy @@ -229,8 +228,7 @@ def save_condition(value, opt_map, question_id_map) end if value['action_type'] == 'remove' - c.remove_data = value['remove_question_id'] - c.remove_data = c.remove_data.map { |qid| question_id_map[qid] } if question_id_map.present? + c.remove_data = handle_remove_data(value, question_id_map) # Do not save the condition if remove_data is empty if c.remove_data.empty? c.destroy @@ -246,7 +244,6 @@ def save_condition(value, opt_map, question_id_map) end c.save end - # rubocop:enable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity # rubocop:enable Metrics/AbcSize private @@ -275,6 +272,22 @@ def check_remove_conditions end # rubocop:enable Metrics/AbcSize + def handle_option_list(value, opt_map) + if opt_map.present? + value['question_option'].map { |qopt| opt_map[qopt] } + else + value['question_option'] + end + end + + def handle_remove_data(value, question_id_map) + if question_id_map.present? + value['remove_question_id'].map { |qid| question_id_map[qid] } + else + value['remove_question_id'] + end + end + def handle_webhook_data(value) # return nil if any of the webhook fields are blank return if %w[webhook-name webhook-email webhook-subject webhook-message].any? { |key| value[key].blank? } From 115ea702768a1470aad964dfe3d0773e56410076 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Fri, 4 Apr 2025 12:55:48 -0600 Subject: [PATCH 11/23] Put back `# rubocop:disable Metrics/MethodLength` --- app/models/question.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/question.rb b/app/models/question.rb index 2c7031a3c5..6f7d006750 100644 --- a/app/models/question.rb +++ b/app/models/question.rb @@ -213,7 +213,7 @@ def update_conditions(param_conditions, old_to_new_opts, question_id_map) end end - # rubocop:disable Metrics/AbcSize + # rubocop:disable Metrics/AbcSize, Metrics/MethodLength def save_condition(value, opt_map, question_id_map) c = conditions.build c.action_type = value['action_type'] @@ -244,7 +244,7 @@ def save_condition(value, opt_map, question_id_map) end c.save end - # rubocop:enable Metrics/AbcSize + # rubocop:enable Metrics/AbcSize, Metrics/MethodLength private From 0fde2537ddcd3b6e56aa106f4d0103acb6177447 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Mon, 7 Apr 2025 09:50:53 -0600 Subject: [PATCH 12/23] Update CHANGELOG.md This is --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0af92cb483..df8056bee6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ - Lower PostgreSQL GitHub Action Chrome Version to Address Breaking Changes Between Latest Chrome Version (134) and `/features` Tests [#3491](https://github.com/DMPRoadmap/roadmap/pull/3491) - Bumped dependencies via `bundle update && yarn upgrade` [#3483](https://github.com/DMPRoadmap/roadmap/pull/3483) - Fixed issues with Conditional Question serialization offered by @briri from PR https://github.com/CDLUC3/dmptool/pull/667 for DMPTool. There is a migration file with code for MySQL and Postgres to update the Conditions table to convert JSON Arrays in string format records in the conditions table so that they are JSON Arrays. +- Refactor `Question.save_condition` [#3501](https://github.com/DMPRoadmap/roadmap/pull/3501) ## v4.2.0 From 0c2e2d2dc0ddc39c4951413caf834e5c62db6137 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Mon, 7 Apr 2025 13:04:05 -0600 Subject: [PATCH 13/23] Document callers of conditions/form partial Adding a comment to help with maintainability of `app/views/org_admin/conditions/_form.html.erb` --- app/views/org_admin/conditions/_form.html.erb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/views/org_admin/conditions/_form.html.erb b/app/views/org_admin/conditions/_form.html.erb index 3a7877f098..e42c2c271e 100644 --- a/app/views/org_admin/conditions/_form.html.erb +++ b/app/views/org_admin/conditions/_form.html.erb @@ -1,3 +1,8 @@ +<%# This partial is called from the following files: + - app/controllers/org_admin/conditions_controller.rb + - app/views/org_admin/conditions/_container.html.erb + %> + <% condition ||= nil %>
From 97677d4d8ed9833389233d70b663889c700e934d Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Mon, 7 Apr 2025 11:30:01 -0600 Subject: [PATCH 14/23] Remove commented-out code --- app/views/org_admin/conditions/_form.html.erb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/views/org_admin/conditions/_form.html.erb b/app/views/org_admin/conditions/_form.html.erb index e42c2c271e..4bd0d28e89 100644 --- a/app/views/org_admin/conditions/_form.html.erb +++ b/app/views/org_admin/conditions/_form.html.erb @@ -8,7 +8,6 @@
<% 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 From c7abc11e3ade9ccb56ea40f9cc629d3ed9a00834 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Mon, 7 Apr 2025 11:28:27 -0600 Subject: [PATCH 15/23] Remove unused variable from conditions/form It appears that the following previous commit removed the need for this variable: https://github.com/DMPRoadmap/roadmap/commit/da4033b2be1a6ac4d4e59761104bbd63b84526db#diff-9b4ef24d6aebd8b59257b1df4b7bd77adc5873a46a803a69a7ff146266d9658fR3-L15 --- app/views/org_admin/conditions/_form.html.erb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/views/org_admin/conditions/_form.html.erb b/app/views/org_admin/conditions/_form.html.erb index 4bd0d28e89..0ed43b039e 100644 --- a/app/views/org_admin/conditions/_form.html.erb +++ b/app/views/org_admin/conditions/_form.html.erb @@ -15,7 +15,6 @@ remove_question_group = condition_exists ? grouped_options_for_select(remove_question_collection, condition[:remove_question_id]) : grouped_options_for_select(remove_question_collection) - multiple = (question.question_format.multiselectbox? || question.question_format.checkbox?) view_email_content_info = _("Hover over the email address to view email content. To change email details you need to remove and add the condition again.") %> From b2eaac4f01924a91d30d07d0bb03d5a26c8d7618 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Mon, 7 Apr 2025 14:59:35 -0600 Subject: [PATCH 16/23] Replace `condition_exists` w/ `condition.present?` - https://github.com/DMPRoadmap/roadmap/pull/3497 introduces `<% condition ||= nil %>` on line 6. - This refactor replaces the usage of `condition_exists` with `condition.present?` - `app/views/org_admin/conditions/_container.html.erb` appears to be the only `app/views/org_admin/conditions/_form.html.erb` that passes the `locals : { condition` variable. The `<% condition ||= nil %>` code on line 6 should be sufficient for performing this check. - Additionally, if `condition == nil` and `condition_exists == true` ever occurred, then we would encounter `NoMethodError` exceptions on lines 13 and 27. --- app/views/org_admin/conditions/_form.html.erb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/app/views/org_admin/conditions/_form.html.erb b/app/views/org_admin/conditions/_form.html.erb index 0ed43b039e..7c76a2bf06 100644 --- a/app/views/org_admin/conditions/_form.html.erb +++ b/app/views/org_admin/conditions/_form.html.erb @@ -10,9 +10,8 @@ action_type_arr = [["removes", :remove], ["adds notification", :add_webhook]] 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 - remove_question_group = condition_exists ? + type_default = condition.present? ? (condition[:action_type] == "remove" ? :remove : :add_webhook) : :remove + remove_question_group = condition.present? ? grouped_options_for_select(remove_question_collection, condition[:remove_question_id]) : grouped_options_for_select(remove_question_collection) view_email_content_info = _("Hover over the email address to view email content. To change email details you need to remove and add the condition again.") @@ -25,7 +24,7 @@
<%= _('Option') %>
<%= 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][]"}) %> + condition.present? ? 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][]"}) %>
<%= _('Action') %>
From e651186c517fde909d24cdac1dfc52b55127c739 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Mon, 7 Apr 2025 15:08:07 -0600 Subject: [PATCH 17/23] Remove redundant `type_default` variable - `<%= select_tag(:action_type, options_for_select(action_type_arr, type_default)` was only being executed when `if condition.nil?` was true (now line 20). Additionally, that was the only line that was using the `type_default` variable. - Thus, no conditional is required for the assigning of `type_default` and we can just use `:remove` explicitly on line 30 now. --- app/views/org_admin/conditions/_form.html.erb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/views/org_admin/conditions/_form.html.erb b/app/views/org_admin/conditions/_form.html.erb index 7c76a2bf06..69effd0053 100644 --- a/app/views/org_admin/conditions/_form.html.erb +++ b/app/views/org_admin/conditions/_form.html.erb @@ -10,7 +10,6 @@ action_type_arr = [["removes", :remove], ["adds notification", :add_webhook]] name_start = "conditions[#{condition_no.to_s}]" remove_question_collection = later_question_list(question) - type_default = condition.present? ? (condition[:action_type] == "remove" ? :remove : :add_webhook) : :remove remove_question_group = condition.present? ? grouped_options_for_select(remove_question_collection, condition[:remove_question_id]) : grouped_options_for_select(remove_question_collection) @@ -28,7 +27,7 @@
<%= _('Action') %>
- <%= 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'}) %> + <%= select_tag(:action_type, options_for_select(action_type_arr, :remove), {name: name_start + "[action_type]", class: 'action-type form-select narrow', 'data-bs-style': 'dropdown-toggle bg-white px-4 py-3'}) %>
From dc04625e834fb07153c668f387714a62f244eb51 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Mon, 7 Apr 2025 15:09:53 -0600 Subject: [PATCH 18/23] Remove redundant conditional check The modified code was and is only executed when `condition.nil?` is true (line 20). Thus, the ternary operator always evaluated to the else. --- app/views/org_admin/conditions/_form.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/org_admin/conditions/_form.html.erb b/app/views/org_admin/conditions/_form.html.erb index 69effd0053..459e191c48 100644 --- a/app/views/org_admin/conditions/_form.html.erb +++ b/app/views/org_admin/conditions/_form.html.erb @@ -23,7 +23,7 @@
<%= _('Option') %>
<%= select_tag(:question_option, options_from_collection_for_select(question.question_options.sort_by(&:number), "id", "text", - condition.present? ? 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][]"}) %> + 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][]"}) %>
<%= _('Action') %>
From 5d0ec1e4d781b90dd3972aeea7c4373bb58a21c1 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Tue, 8 Apr 2025 09:06:25 -0600 Subject: [PATCH 19/23] Remove redundant conditional check - Line 35 is the only that uses the `remove_question_group` variable. However, line 35 is only executed when `condition.nil? == true` (line 18). --- app/views/org_admin/conditions/_form.html.erb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/views/org_admin/conditions/_form.html.erb b/app/views/org_admin/conditions/_form.html.erb index 459e191c48..32c3068127 100644 --- a/app/views/org_admin/conditions/_form.html.erb +++ b/app/views/org_admin/conditions/_form.html.erb @@ -10,9 +10,7 @@ action_type_arr = [["removes", :remove], ["adds notification", :add_webhook]] name_start = "conditions[#{condition_no.to_s}]" remove_question_collection = later_question_list(question) - remove_question_group = condition.present? ? - grouped_options_for_select(remove_question_collection, condition[:remove_question_id]) : - grouped_options_for_select(remove_question_collection) + remove_question_group = grouped_options_for_select(remove_question_collection) view_email_content_info = _("Hover over the email address to view email content. To change email details you need to remove and add the condition again.") %> From 3a1106770b13f10502486f5cdf15bc79fb2b12db Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Tue, 8 Apr 2025 10:35:10 -0600 Subject: [PATCH 20/23] Refactor: Split conditions/_form into two This change refactors `app/views/org_admin/conditions/_form.html.erb`. The change moves the logic for new conditions to `app/views/org_admin/conditions/_new_condition_form.erb` and existing conditions to `app/views/org_admin/conditions/_existing_condition_display.erb`. --- .../_existing_condition_display.erb | 38 +++++++++ app/views/org_admin/conditions/_form.html.erb | 83 +++++-------------- .../conditions/_new_condition_form.erb | 28 +++++++ 3 files changed, 85 insertions(+), 64 deletions(-) create mode 100644 app/views/org_admin/conditions/_existing_condition_display.erb create mode 100644 app/views/org_admin/conditions/_new_condition_form.erb diff --git a/app/views/org_admin/conditions/_existing_condition_display.erb b/app/views/org_admin/conditions/_existing_condition_display.erb new file mode 100644 index 0000000000..21705f29aa --- /dev/null +++ b/app/views/org_admin/conditions/_existing_condition_display.erb @@ -0,0 +1,38 @@ + <% + qopt = condition[:question_option_id].any? ? QuestionOption.find_by(id: condition[:question_option_id].first): nil + rquesArray = condition[:remove_question_id].any? ? Question.where(id: condition[:remove_question_id]) : 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 !rquesArray.nil? %> + <% rquesArray.each do |rques| %> + Question <%= rques[:number] %>: <%= rques.text.gsub(%r{}, '').slice(0, 50) %> + <%= '...' if rques.text.gsub(%r{}, '').length > 50 %> +
+ <% 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']}" + %> + <%= condition[:webhook_data]['email'] %> +
(<%= view_email_content_info %>) + + <%= 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) %> +
+
+ <%= _('Remove') %> +
diff --git a/app/views/org_admin/conditions/_form.html.erb b/app/views/org_admin/conditions/_form.html.erb index 32c3068127..2b6e032de5 100644 --- a/app/views/org_admin/conditions/_form.html.erb +++ b/app/views/org_admin/conditions/_form.html.erb @@ -16,72 +16,27 @@ <%# If this is a new condition then display the interactive controls. otherwise just display the logic %> <% if condition.nil? %> -
Add condition
-
-
-
<%= _('Option') %>
- <%= select_tag(:question_option, options_from_collection_for_select(question.question_options.sort_by(&:number), "id", "text", - 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][]"}) %> -
-
-
<%= _('Action') %>
- <%= select_tag(:action_type, options_for_select(action_type_arr, :remove), {name: name_start + "[action_type]", class: 'action-type form-select narrow', 'data-bs-style': 'dropdown-toggle bg-white px-4 py-3'}) %> -
-
-
-
-
<%= _('Target') %>
-
- <%= 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'}) %> -
-
- <%= 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} %> -
+ <%= render partial: 'org_admin/conditions/new_condition_form', + locals: { action_type_arr: action_type_arr, + condition_no: condition_no, + question: question, + name_start: name_start, + remove_question_group: remove_question_group, + view_email_content_info: view_email_content_info + } + %> + <% else %> - <% - qopt = condition[:question_option_id].any? ? QuestionOption.find_by(id: condition[:question_option_id].first): nil - rquesArray = condition[:remove_question_id].any? ? Question.where(id: condition[:remove_question_id]) : nil + <%= render partial: 'org_admin/conditions/existing_condition_display', + locals: { action_type_arr: action_type_arr, + condition: condition, + condition_no: condition_no, + name_start: name_start, + question: question, + remove_question_group: remove_question_group, + view_email_content_info: view_email_content_info + } %> -
- <%= 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 !rquesArray.nil? %> - <% rquesArray.each do |rques| %> - Question <%= rques[:number] %>: <%= rques.text.gsub(%r{}, '').slice(0, 50) %> - <%= '...' if rques.text.gsub(%r{}, '').length > 50 %> -
- <% 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']}" - %> - <%= condition[:webhook_data]['email'] %> -
(<%= view_email_content_info %>) - <%= 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) %> -
-
- <%= _('Remove') %> -
<% end %>
diff --git a/app/views/org_admin/conditions/_new_condition_form.erb b/app/views/org_admin/conditions/_new_condition_form.erb new file mode 100644 index 0000000000..cf38cb3e37 --- /dev/null +++ b/app/views/org_admin/conditions/_new_condition_form.erb @@ -0,0 +1,28 @@ +
Add condition
+
+
+
<%= _('Option') %>
+ <%= select_tag(:question_option, options_from_collection_for_select(question.question_options.sort_by(&:number), "id", "text", + 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][]"}) %> +
+
+
<%= _('Action') %>
+ <%= select_tag(:action_type, options_for_select(action_type_arr, :remove), {name: name_start + "[action_type]", class: 'action-type form-select narrow', 'data-bs-style': 'dropdown-toggle bg-white px-4 py-3'}) %> +
+
+
+
+
<%= _('Target') %>
+
+ <%= 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'}) %> +
+
+ <%= 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} %> +
From 4724bc7e499878208a99cc3dafa53329b457fe6b Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Tue, 8 Apr 2025 11:01:46 -0600 Subject: [PATCH 21/23] Cleanup after `condtions/_form` refactor - ` action_type_arr` and `remove_question_group` are only needed in `conditions/_new_condition_form.erb`. The assignments have been moved directly to that partial. - - ` view_email_content_info` is only needed in `conditions/_new_condition_form.erb`. The assignment has been moved directly to that partial. --- .../_existing_condition_display.erb | 1 + app/views/org_admin/conditions/_form.html.erb | 31 ++++++------------- .../conditions/_new_condition_form.erb | 6 ++++ 3 files changed, 17 insertions(+), 21 deletions(-) diff --git a/app/views/org_admin/conditions/_existing_condition_display.erb b/app/views/org_admin/conditions/_existing_condition_display.erb index 21705f29aa..b2a76765f4 100644 --- a/app/views/org_admin/conditions/_existing_condition_display.erb +++ b/app/views/org_admin/conditions/_existing_condition_display.erb @@ -1,6 +1,7 @@ <% qopt = condition[:question_option_id].any? ? QuestionOption.find_by(id: condition[:question_option_id].first): nil rquesArray = condition[:remove_question_id].any? ? Question.where(id: condition[:remove_question_id]) : nil + view_email_content_info = _("Hover over the email address to view email content. To change email details you need to remove and add the condition again.") %>
<%= qopt[:text]&.slice(0, 25) %> diff --git a/app/views/org_admin/conditions/_form.html.erb b/app/views/org_admin/conditions/_form.html.erb index 2b6e032de5..0fe405972b 100644 --- a/app/views/org_admin/conditions/_form.html.erb +++ b/app/views/org_admin/conditions/_form.html.erb @@ -3,40 +3,29 @@ - app/views/org_admin/conditions/_container.html.erb %> -<% condition ||= nil %> -
<% - action_type_arr = [["removes", :remove], ["adds notification", :add_webhook]] + condition ||= nil name_start = "conditions[#{condition_no.to_s}]" - remove_question_collection = later_question_list(question) - remove_question_group = grouped_options_for_select(remove_question_collection) - view_email_content_info = _("Hover over the email address to view email content. To change email details you need to remove and add the condition again.") %> <%# If this is a new condition then display the interactive controls. otherwise just display the logic %> <% if condition.nil? %> <%= render partial: 'org_admin/conditions/new_condition_form', - locals: { action_type_arr: action_type_arr, - condition_no: condition_no, - question: question, + locals: { condition_no: condition_no, name_start: name_start, - remove_question_group: remove_question_group, - view_email_content_info: view_email_content_info + question: question } %> <% else %> - <%= render partial: 'org_admin/conditions/existing_condition_display', - locals: { action_type_arr: action_type_arr, - condition: condition, - condition_no: condition_no, - name_start: name_start, - question: question, - remove_question_group: remove_question_group, - view_email_content_info: view_email_content_info - } - %> + <%= render partial: 'org_admin/conditions/existing_condition_display', + locals: { condition: condition, + condition_no: condition_no, + name_start: name_start, + question: question + } + %> <% end %>
diff --git a/app/views/org_admin/conditions/_new_condition_form.erb b/app/views/org_admin/conditions/_new_condition_form.erb index cf38cb3e37..49222c9de6 100644 --- a/app/views/org_admin/conditions/_new_condition_form.erb +++ b/app/views/org_admin/conditions/_new_condition_form.erb @@ -1,3 +1,9 @@ + <% + action_type_arr = [["removes", :remove], ["adds notification", :add_webhook]] + remove_question_collection = later_question_list(question) + remove_question_group = grouped_options_for_select(remove_question_collection) + %> +
Add condition
From 8f952be0d6747bead18056ca9896b3fd4107ce46 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Wed, 9 Apr 2025 08:49:19 -0600 Subject: [PATCH 22/23] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0af92cb483..d744dfd3b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ - Lower PostgreSQL GitHub Action Chrome Version to Address Breaking Changes Between Latest Chrome Version (134) and `/features` Tests [#3491](https://github.com/DMPRoadmap/roadmap/pull/3491) - Bumped dependencies via `bundle update && yarn upgrade` [#3483](https://github.com/DMPRoadmap/roadmap/pull/3483) - Fixed issues with Conditional Question serialization offered by @briri from PR https://github.com/CDLUC3/dmptool/pull/667 for DMPTool. There is a migration file with code for MySQL and Postgres to update the Conditions table to convert JSON Arrays in string format records in the conditions table so that they are JSON Arrays. +- Refactor `org_admin/conditions/_form.html.erb` [#3502](https://github.com/DMPRoadmap/roadmap/pull/3502) ## v4.2.0 From c0f6981e907576796e8219011704dfc1a4327ae8 Mon Sep 17 00:00:00 2001 From: aaronskiba Date: Wed, 9 Apr 2025 10:10:12 -0600 Subject: [PATCH 23/23] Enable translations with gettext method NOTE: For app/views/org_admin/conditions/_existing_condition_display.erb, `hook_tip` refers to the pop up message that is rendered when hovering over the email address in the "Target" section of an "Email" type action. We are only enabling for the titles here ('Name', 'Email', 'Subject', and 'Message'), not the actual corresponding values. --- app/views/org_admin/conditions/_add.html.erb | 9 +++++---- .../org_admin/conditions/_existing_condition_display.erb | 8 +++++--- app/views/org_admin/conditions/_new_condition_form.erb | 2 +- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/app/views/org_admin/conditions/_add.html.erb b/app/views/org_admin/conditions/_add.html.erb index 0ac1558234..f2ff72d72a 100644 --- a/app/views/org_admin/conditions/_add.html.erb +++ b/app/views/org_admin/conditions/_add.html.erb @@ -1,12 +1,13 @@
<%= link_to _('Add condition'), new_org_admin_question_condition_path(question_id: question.id, condition_no: condition_no), remote: true, class: "add-condition" %> -

To add a condition you must have selected an Option and Action together with +

+ <%= _('To add a condition you must have selected an Option and Action together with') %>

    -
  • if Action is 'remove', you need to select one or more choices in Target.
    • -
  • if Action is 'add notification', you need to fill in all the fields in the 'Send email' popup.
    • +
  • <%= _("if Action is 'remove', you need to select one or more choices in Target.") %>
    • +
  • <%= _("if Action is 'add notification', you need to fill in all the fields in the 'Send email' popup.") %>
- Otherwise, the condition will not be saved. + <%= _('Otherwise, the condition will not be saved.') %>

diff --git a/app/views/org_admin/conditions/_existing_condition_display.erb b/app/views/org_admin/conditions/_existing_condition_display.erb index b2a76765f4..85ad7efdac 100644 --- a/app/views/org_admin/conditions/_existing_condition_display.erb +++ b/app/views/org_admin/conditions/_existing_condition_display.erb @@ -8,7 +8,7 @@ <%= hidden_field_tag(name_start + "[question_option][]", condition[:question_option_id]) %>
- <%= condition[:action_type] == 'remove' ? 'Remove' : 'Email' %> + <%= condition[:action_type] == 'remove' ? _('Remove') : _('Email') %> <%= hidden_field_tag(name_start + "[action_type]", condition[:action_type]) %>
@@ -21,8 +21,10 @@ <%= 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']}" + hook_tip = "#{_('Name')}: #{condition[:webhook_data]['name']}\n" + hook_tip += "#{_('Email')}: #{condition[:webhook_data]['email']}\n" + hook_tip += "#{_('Subject')}: #{condition[:webhook_data]['subject']}\n" + hook_tip += "#{_('Message')}: #{condition[:webhook_data]['message']}" %> <%= condition[:webhook_data]['email'] %>
(<%= view_email_content_info %>) diff --git a/app/views/org_admin/conditions/_new_condition_form.erb b/app/views/org_admin/conditions/_new_condition_form.erb index 49222c9de6..7b42c99f10 100644 --- a/app/views/org_admin/conditions/_new_condition_form.erb +++ b/app/views/org_admin/conditions/_new_condition_form.erb @@ -4,7 +4,7 @@ remove_question_group = grouped_options_for_select(remove_question_collection) %> -
Add condition
+
<%= _('Add condition') %>
<%= _('Option') %>