diff --git a/app/controllers/plans_controller.rb b/app/controllers/plans_controller.rb index cb28d3a..20be939 100644 --- a/app/controllers/plans_controller.rb +++ b/app/controllers/plans_controller.rb @@ -43,9 +43,11 @@ # If the template_id is blank then we need to look up the available templates and return JSON if plan_params[:template_id].blank? - templates = template_options(org_id, funder_id) - render json: {"templates": templates.collect{|t| {id: t.id, title: t.title} }}.to_json - + # Something went wrong there should always be a template id + respond_to do |format| + flash[:alert] = _('Unable to identify a suitable template for your plan.') + format.html { redirect_to new_plan_path } + end else # Otherwise create the plan @plan.principal_investigator = current_user.surname.blank? ? nil : "#{current_user.firstname} #{current_user.surname}" @@ -336,7 +338,6 @@ end end - private def plan_params params.require(:plan).permit(:org_id, :org_name, :funder_id, :funder_name, :template_id, :title, :visibility, @@ -427,45 +428,4 @@ end plan.delete(src_plan_key) end - - # Collect all of the templates available for the org+funder combination - # -------------------------------------------------------------------------- - def template_options(org_id, funder_id) - templates = [] - - if org_id.present? || funder_id.present? - if funder_id.blank? - # Load the org's template(s) - if org_id.present? - org = Org.find(org_id) - templates = Template.valid.where(published: true, org: org, customization_of: nil).to_a - end - - else - funder = Org.find(funder_id) - # Load the funder's template(s) - templates = Template.valid.where(published: true, org: funder).to_a - - if org_id.present? - org = Org.find(org_id) - - # Swap out any organisational cusotmizations of a funder template - templates.each do |tmplt| - customization = Template.valid.find_by(published: true, org: org, customization_of: tmplt.dmptemplate_id) - if customization.present? && tmplt.updated_at < customization.created_at - templates.delete(tmplt) - templates << customization - end - end - end - end - end - - # If no templates were available use the generic templates - if templates.empty? - templates << Template.where(is_default: true, published: true).first - end - templates = (templates.count > 0 ? templates.sort{|x,y| x.title <=> y.title} : []) - end - end diff --git a/app/controllers/templates_controller.rb b/app/controllers/templates_controller.rb index b87e673..bbd3201 100644 --- a/app/controllers/templates_controller.rb +++ b/app/controllers/templates_controller.rb @@ -366,5 +366,56 @@ end end + + # Collect all of the templates available for the org+funder combination + # -------------------------------------------------------------------------- + def template_options() + org_id = (plan_params[:org_id] == '-1' ? '' : plan_params[:org_id]) + funder_id = (plan_params[:funder_id] == '-1' ? '' : plan_params[:funder_id]) + authorize Template.new + + templates = [] + + if org_id.present? || funder_id.present? + if funder_id.blank? + # Load the org's template(s) + if org_id.present? + org = Org.find(org_id) + templates = Template.valid.where(published: true, org: org, customization_of: nil).to_a + end + + else + funder = Org.find(funder_id) + # Load the funder's template(s) + templates = Template.valid.where(published: true, org: funder).to_a + + if org_id.present? + org = Org.find(org_id) + + # Swap out any organisational cusotmizations of a funder template + templates.each do |tmplt| + customization = Template.valid.find_by(published: true, org: org, customization_of: tmplt.dmptemplate_id) + if customization.present? && tmplt.updated_at < customization.created_at + templates.delete(tmplt) + templates << customization + end + end + end + end + end + + # If no templates were available use the generic templates + if templates.empty? + templates << Template.where(is_default: true, published: true).first + end + templates = (templates.count > 0 ? templates.sort{|x,y| x.title <=> y.title} : []) + + render json: {"templates": templates.collect{|t| {id: t.id, title: t.title} }}.to_json + end + + private + def plan_params + params.require(:plan).permit(:org_id, :funder_id) + end end diff --git a/app/policies/plan_policy.rb b/app/policies/plan_policy.rb index b15c6e4..8fd7e88 100644 --- a/app/policies/plan_policy.rb +++ b/app/policies/plan_policy.rb @@ -37,10 +37,6 @@ @plan.readable_by?(@user.id) && Role.find_by(user_id: @user.id, plan_id: @plan.id).active end - def possible_templates? - @plan.id.nil? - end - def duplicate? @plan.editable_by?(@user.id) && Role.find_by(user_id: @user.id, plan_id: @plan.id).active end diff --git a/app/policies/template_policy.rb b/app/policies/template_policy.rb index feb94b6..157e9c3 100644 --- a/app/policies/template_policy.rb +++ b/app/policies/template_policy.rb @@ -60,6 +60,12 @@ def admin_copy? user.can_modify_templates? && (template.org_id == user.org_id) end + + # Anyone with an account should be able to get templates for the sepecified research_org + funder + # This policy is applicable to the Create Plan page + def template_options? + user.present? + end class Scope < Scope def resolve diff --git a/app/views/plans/new.html.erb b/app/views/plans/new.html.erb index 7c6f428..2c52c43 100644 --- a/app/views/plans/new.html.erb +++ b/app/views/plans/new.html.erb @@ -17,7 +17,7 @@
<%= f.text_field(:title, class: 'form-control', 'aria-describedby': 'project-title', 'aria-required': 'true', 'data-toggle': 'tooltip', - 'data-content': _('If applying for funding, state the project title exactly as in the proposal.')) %> + title: _('If applying for funding, state the project title exactly as in the proposal.')) %>
<%= label_tag(:is_test, raw("#{check_box_tag(:is_test, "1", false)} #{_('mock project for testing, practice, or educational purposes')}")) %> @@ -64,6 +64,7 @@
+ <%= hidden_field_tag 'template-option-target', template_options_template_url(current_user) %>

<%= _('Which DMP template would you like to use?') %>

diff --git a/config/routes.rb b/config/routes.rb index d494910..2636d98 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -145,6 +145,7 @@ put 'admin_unpublish' put 'admin_copy' get 'admin_transfer_customization' + get 'template_options', constraints: {format: [:json]} end end diff --git a/db/schema.rb b/db/schema.rb index 2532f14..fbe8055 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -22,9 +22,8 @@ t.datetime "updated_at" end - add_index "annotations", ["org_id"], name: "fk_rails_aca7521f72" - add_index "annotations", ["question_id"], name: "fk_rails_0e08e753b6" - add_index "annotations", ["question_id"], name: "index_annotations_on_question_id" + add_index "annotations", ["org_id"], name: "fk_rails_aca7521f72", using: :btree + add_index "annotations", ["question_id"], name: "index_annotations_on_question_id", using: :btree create_table "answers", force: :cascade do |t| t.text "text", limit: 65535 @@ -45,7 +44,8 @@ t.integer "question_option_id", limit: 4, null: false end - add_index "answers_question_options", ["answer_id"], name: "index_answers_question_options_on_answer_id" + add_index "answers_question_options", ["answer_id"], name: "index_answers_question_options_on_answer_id", using: :btree + add_index "answers_question_options", ["question_option_id"], name: "fk_rails_01ba00b569", using: :btree create_table "exported_plans", force: :cascade do |t| t.integer "plan_id", limit: 4 @@ -97,8 +97,7 @@ t.boolean "published" end - add_index "guidance_groups", ["org_id"], name: "fk_rails_819c1dbbc7" - add_index "guidance_groups", ["org_id"], name: "index_guidance_groups_on_org_id" + add_index "guidance_groups", ["org_id"], name: "index_guidance_groups_on_org_id", using: :btree create_table "guidances", force: :cascade do |t| t.text "text", limit: 65535 @@ -109,8 +108,7 @@ t.boolean "published" end - add_index "guidances", ["guidance_group_id"], name: "fk_rails_20d29da787" - add_index "guidances", ["guidance_group_id"], name: "index_guidances_on_guidance_group_id" + add_index "guidances", ["guidance_group_id"], name: "index_guidances_on_guidance_group_id", using: :btree create_table "identifier_schemes", force: :cascade do |t| t.string "name", limit: 255 @@ -139,9 +137,8 @@ t.datetime "updated_at" end - add_index "notes", ["answer_id"], name: "fk_rails_907f8d48bf" - add_index "notes", ["answer_id"], name: "index_notes_on_answer_id" - add_index "notes", ["user_id"], name: "fk_rails_7f2323ad43" + add_index "notes", ["answer_id"], name: "index_notes_on_answer_id", using: :btree + add_index "notes", ["user_id"], name: "fk_rails_7f2323ad43", using: :btree create_table "org_identifiers", force: :cascade do |t| t.string "identifier", limit: 255 @@ -162,9 +159,8 @@ t.datetime "updated_at" end - add_index "org_token_permissions", ["org_id"], name: "fk_rails_e1db1b22c5" - add_index "org_token_permissions", ["org_id"], name: "index_org_token_permissions_on_org_id" - add_index "org_token_permissions", ["token_permission_type_id"], name: "fk_rails_2aa265f538" + add_index "org_token_permissions", ["org_id"], name: "index_org_token_permissions_on_org_id", using: :btree + add_index "org_token_permissions", ["token_permission_type_id"], name: "fk_rails_2aa265f538", using: :btree create_table "orgs", force: :cascade do |t| t.string "name", limit: 255 @@ -184,11 +180,11 @@ t.string "logo_name", limit: 255 t.string "contact_email", limit: 255 t.integer "org_type", limit: 4, default: 0, null: false - t.string "links", default: "[]" - t.string "contact_name" + t.string "links", limit: 255, default: "[]" + t.string "contact_name", limit: 255 t.boolean "feedback_enabled", default: false - t.string "feedback_email_subject" - t.text "feedback_email_msg" + t.string "feedback_email_subject", limit: 255 + t.text "feedback_email_msg", limit: 65535 end add_index "orgs", ["language_id"], name: "fk_rails_5640112cab" @@ -233,7 +229,7 @@ t.string "data_contact_email", limit: 255 t.string "data_contact_phone", limit: 255 t.string "principal_investigator_email", limit: 255 - t.string "principal_investigator_phone" + t.string "principal_investigator_phone", limit: 255 end add_index "plans", ["template_id"], name: "index_plans_on_template_id" @@ -269,8 +265,7 @@ t.datetime "updated_at" end - add_index "question_options", ["question_id"], name: "fk_rails_b9c5f61cf9" - add_index "question_options", ["question_id"], name: "index_question_options_on_question_id" + add_index "question_options", ["question_id"], name: "index_question_options_on_question_id", using: :btree create_table "questions", force: :cascade do |t| t.text "text", limit: 65535 @@ -292,7 +287,8 @@ t.integer "theme_id", limit: 4, null: false end - add_index "questions_themes", ["question_id"], name: "index_questions_themes_on_question_id" + add_index "questions_themes", ["question_id"], name: "index_questions_themes_on_question_id", using: :btree + add_index "questions_themes", ["theme_id"], name: "fk_rails_0489d5eeba", using: :btree create_table "regions", force: :cascade do |t| t.string "abbreviation", limit: 255 @@ -310,10 +306,8 @@ t.boolean "active", default: true end - add_index "roles", ["plan_id"], name: "fk_rails_a1ce6c2772" - add_index "roles", ["plan_id"], name: "index_roles_on_plan_id" - add_index "roles", ["user_id"], name: "fk_rails_ab35d699f0" - add_index "roles", ["user_id"], name: "index_roles_on_user_id" + add_index "roles", ["plan_id"], name: "index_roles_on_plan_id", using: :btree + add_index "roles", ["user_id"], name: "index_roles_on_user_id", using: :btree create_table "sections", force: :cascade do |t| t.string "title", limit: 255 @@ -378,10 +372,8 @@ t.integer "guidance_id", limit: 4 end - add_index "themes_in_guidance", ["guidance_id"], name: "fk_rails_a5ab9402df" - add_index "themes_in_guidance", ["guidance_id"], name: "index_themes_in_guidance_on_guidance_id" - add_index "themes_in_guidance", ["theme_id"], name: "fk_rails_7d708f6f1e" - add_index "themes_in_guidance", ["theme_id"], name: "index_themes_in_guidance_on_theme_id" + add_index "themes_in_guidance", ["guidance_id"], name: "index_themes_in_guidance_on_guidance_id", using: :btree + add_index "themes_in_guidance", ["theme_id"], name: "index_themes_in_guidance_on_theme_id", using: :btree create_table "token_permission_types", force: :cascade do |t| t.string "token_type", limit: 255 @@ -398,9 +390,8 @@ t.integer "identifier_scheme_id", limit: 4 end - add_index "user_identifiers", ["identifier_scheme_id"], name: "fk_rails_fe95df7db0" - add_index "user_identifiers", ["user_id"], name: "fk_rails_65c9a98cdb" - add_index "user_identifiers", ["user_id"], name: "index_user_identifiers_on_user_id" + add_index "user_identifiers", ["identifier_scheme_id"], name: "fk_rails_fe95df7db0", using: :btree + add_index "user_identifiers", ["user_id"], name: "index_user_identifiers_on_user_id", using: :btree create_table "users", force: :cascade do |t| t.string "firstname", limit: 255 @@ -408,8 +399,8 @@ t.string "email", limit: 255, default: "", null: false t.string "orcid_id", limit: 255 t.string "shibboleth_id", limit: 255 - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false t.string "encrypted_password", limit: 255, default: "" t.string "reset_password_token", limit: 255 t.datetime "reset_password_sent_at" @@ -436,17 +427,54 @@ t.string "recovery_email", limit: 255 end - add_index "users", ["email"], name: "index_users_on_email", unique: true - add_index "users", ["language_id"], name: "fk_rails_45f4f12508" - add_index "users", ["org_id"], name: "fk_rails_e73753bccb" - add_index "users", ["org_id"], name: "index_users_on_org_id" + add_index "users", ["email"], name: "index_users_on_email", unique: true, using: :btree + add_index "users", ["language_id"], name: "fk_rails_45f4f12508", using: :btree + add_index "users", ["org_id"], name: "index_users_on_org_id", using: :btree create_table "users_perms", id: false, force: :cascade do |t| t.integer "user_id", limit: 4 t.integer "perm_id", limit: 4 end - add_index "users_perms", ["perm_id"], name: "fk_rails_457217c31c" - add_index "users_perms", ["user_id"], name: "index_users_perms_on_user_id" + add_index "users_perms", ["perm_id"], name: "fk_rails_457217c31c", using: :btree + add_index "users_perms", ["user_id"], name: "index_users_perms_on_user_id", using: :btree + 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 "guidance_groups", "orgs" + add_foreign_key "guidances", "guidance_groups" + add_foreign_key "notes", "answers" + add_foreign_key "notes", "users" + add_foreign_key "org_identifiers", "identifier_schemes" + add_foreign_key "org_identifiers", "orgs" + add_foreign_key "org_token_permissions", "orgs" + add_foreign_key "org_token_permissions", "token_permission_types" + add_foreign_key "orgs", "languages" + add_foreign_key "orgs", "regions" + add_foreign_key "phases", "templates" + add_foreign_key "plans", "templates" + add_foreign_key "plans_guidance_groups", "guidance_groups" + add_foreign_key "plans_guidance_groups", "plans" + 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 "roles", "plans" + add_foreign_key "roles", "users" + add_foreign_key "sections", "phases" + add_foreign_key "templates", "orgs" + add_foreign_key "themes_in_guidance", "guidances" + add_foreign_key "themes_in_guidance", "themes" + add_foreign_key "user_identifiers", "identifier_schemes" + add_foreign_key "user_identifiers", "users" + add_foreign_key "users", "languages" + add_foreign_key "users", "orgs" + add_foreign_key "users_perms", "perms" + add_foreign_key "users_perms", "users" end diff --git a/lib/assets/javascripts/constants.js b/lib/assets/javascripts/constants.js index 4108911..4313a35 100644 --- a/lib/assets/javascripts/constants.js +++ b/lib/assets/javascripts/constants.js @@ -29,3 +29,6 @@ export const SHIBBOLETH_DISCOVERY_SERVICE_HIDE_LIST = 'Hide list.'; export const SHIBBOLETH_DISCOVERY_SERVICE_SHOW_LIST = 'See the full list of partner institutions.'; + +export const NO_TEMPLATE_FOUND_ERROR = 'Unable to find a suitable template for the research organisation and funder you selected.'; +export const NEW_PLAN_DISABLED_TOOLTIP = 'Please select a research organisation and funder to continue.'; diff --git a/lib/assets/javascripts/utils/notificationHelper.js b/lib/assets/javascripts/utils/notificationHelper.js index bb40b9b..9380dbc 100644 --- a/lib/assets/javascripts/utils/notificationHelper.js +++ b/lib/assets/javascripts/utils/notificationHelper.js @@ -26,3 +26,7 @@ notificationArea.removeClass('hide'); } }; + +export const hideNotifications = () => { + $('#notification-area').addClass('hide'); +}; diff --git a/lib/assets/javascripts/views/plans/new.js b/lib/assets/javascripts/views/plans/new.js index ad7747f..ff19391 100644 --- a/lib/assets/javascripts/views/plans/new.js +++ b/lib/assets/javascripts/views/plans/new.js @@ -1,17 +1,32 @@ import debounce from '../../utils/debounce'; import ariatiseForm from '../../utils/ariatiseForm'; import initAutoComplete from '../../utils/autoComplete'; -import { isObject, isArray } from '../../utils/isType'; +import { isObject, isArray, isString } from '../../utils/isType'; import { isValidText } from '../../utils/isValidInputType'; +import { NO_TEMPLATE_FOUND_ERROR, NEW_PLAN_DISABLED_TOOLTIP } from '../../constants'; +import { renderAlert, hideNotifications } from '../../utils/notificationHelper'; $(() => { + const toggleSubmit = () => { + const tmplt = $('#plan_template_id').find(':selected').val(); + if (isString(tmplt)) { + $('#new_plan button[type="submit"]').removeAttr('disabled') + .removeAttr('data-toggle').removeAttr('title'); + } else { + $('#new_plan button[type="submit"]').attr('disabled', true) + .attr('data-toggle', 'tooltip').attr('title', NEW_PLAN_DISABLED_TOOLTIP); + } + }; + // AJAX error function for available template search const error = () => { - // TODO: add error handling + renderAlert(NO_TEMPLATE_FOUND_ERROR); }; // AJAX success function for available template search const success = (data) => { + hideNotifications(); + if (isObject(data) && isArray(data.templates)) { // Display the available_templates section @@ -28,15 +43,13 @@ $('#multiple-templates').show(); $('#available-templates').fadeIn(); } + toggleSubmit(); } else { error(); } } }; - const getAction = jQueryForm => jQueryForm.attr('action'); - const getMethod = jQueryForm => jQueryForm.attr('method'); - // When one of the autocomplete fields changes, fetch the available templates const handleComboboxChange = debounce(() => { const validOrg = (isValidText($('#plan_org_id').val()) || $('#plan_no_org').prop('checked')); @@ -49,14 +62,11 @@ // Clear out the old template dropdown contents $('#plan_template_id option').remove(); - // Fetch the available templates fbased on the funder and research org selected - const jQueryForm = $('form#new_plan'); - const formElements = jQueryForm.serializeArray(); + // Fetch the available templates based on the funder and research org selected + const qryStr = `?plan[org_id]=${$('#plan_org_id').val()}&plan[funder_id]=${$('#plan_funder_id').val()}`; $.ajax({ - method: getMethod(jQueryForm), - url: getAction(jQueryForm), - data: formElements, - }).done(success, error); + url: `${$('#template-option-target').val()}${qryStr}`, + }).done(success).fail(error); } }, 150); @@ -82,31 +92,29 @@ // When the user checks the 'mock project' box we need to set the // visibility to 'is_test' - $('#is_test').click((e) => { + $('#new_plan #is_test').click((e) => { $('#plan_visibility').val(($(e.currentTarget)[0].checked ? 'is_test' : defaultVisibility)); }); - // When the hidden org and funder id fields change toogle the submit button - $('#plan_org_id, #plan_funder_id').change(() => { - handleComboboxChange(); - }); - // Make sure the checkbox is unchecked if we're entering text - $('.js-combobox').keyup((e) => { + $('#new_plan #plan_org_name, #new_plan #plan_funder_name').on('keyup blur', (e) => { const whichOne = $(e.currentTarget).prop('id').split('_')[1]; $(`#plan_no_${whichOne}`).prop('checked', false); + handleComboboxChange(); }); // If the user clicks the no Org/Funder checkbox disable the dropdown // and hide clear button - $('#plan_no_org, #plan_no_funder').click((e) => { + $('#new_plan #plan_no_org, #new_plan #plan_no_funder').click((e) => { const whichOne = $(e.currentTarget).prop('id').split('_')[2]; handleCheckboxClick(whichOne, e.currentTarget.checked); }); // Initialize the form - $('#available-templates').hide(); + $('#new_plan #available-templates').hide(); handleComboboxChange(); + toggleSubmit(); + if ($('#plan_no_org').prop('checked')) { handleCheckboxClick('org', $('#plan_no_org').prop('checked')); } diff --git a/test/integration/template_selection_test.rb b/test/integration/template_selection_test.rb index 04113ec..6602144 100644 --- a/test/integration/template_selection_test.rb +++ b/test/integration/template_selection_test.rb @@ -33,7 +33,7 @@ sign_in @researcher - post plans_path(format: :json), {plan: {org_id: @template.org.id}} + get "#{template_options_template_path(@researcher)}?plan[org_id]=#{@template.org_id}" assert_response :success json = JSON.parse(@response.body) @@ -46,7 +46,7 @@ template = version_template(template) # Make sure the published version is used - post plans_path(format: :json), {plan: {org_id: @template.org.id}} + get "#{template_options_template_path(@researcher)}?plan[org_id]=#{@template.org_id}" assert_response :success json = JSON.parse(@response.body) @@ -60,7 +60,7 @@ sign_in @researcher - post plans_path(format: :json), {plan: {org_id: @template.org.id}} + get "#{template_options_template_path(@researcher)}?plan[org_id]=#{@template.org_id}" assert_response :success json = JSON.parse(@response.body) @@ -76,7 +76,7 @@ sign_in @researcher - post plans_path(format: :json), {plan: {org_id: nil}} + get "#{template_options_template_path(@researcher)}?plan[org_id]=" assert_response :success json = JSON.parse(@response.body) @@ -88,7 +88,7 @@ test 'plan gets org template when no funder' do sign_in @researcher - post plans_path(format: :json), {plan: {org_id: @org.id, funder_id: nil}} + get "#{template_options_template_path(@researcher)}?plan[org_id]=#{@org.id}&plan[funder_id]=" assert_response :success json = JSON.parse(@response.body) @@ -100,7 +100,7 @@ test 'plan gets funder template when no org' do sign_in @researcher - post plans_path(format: :json), {plan: {org_id: nil, funder_id: @funder.id}} + get "#{template_options_template_path(@researcher)}?plan[org_id]=&plan[funder_id]=#{@funder.id}" assert_response :success json = JSON.parse(@response.body) @@ -112,7 +112,7 @@ test 'plan gets funder template when org has no customization' do sign_in @researcher - post plans_path(format: :json), {plan: {org_id: @org.id, funder_id: @funder.id}} + get "#{template_options_template_path(@researcher)}?plan[org_id]=#{@org.id}&plan[funder_id]=#{@funder.id}" assert_response :success json = JSON.parse(@response.body) @@ -130,7 +130,7 @@ sign_in @researcher - post plans_path(format: :json), {plan: {org_id: @org.id, funder_id: @funder.id}} + get "#{template_options_template_path(@researcher)}?plan[org_id]=#{@org.id}&plan[funder_id]=#{@funder.id}" assert_response :success json = JSON.parse(@response.body) @@ -147,7 +147,7 @@ sign_in @researcher - post plans_path(format: :json), {plan: {org_id: @org.id, funder_id: @funder.id}} + get "#{template_options_template_path(@researcher)}?plan[org_id]=#{@org.id}&plan[funder_id]=#{@funder.id}" assert_response :success json = JSON.parse(@response.body)