diff --git a/app/controllers/plans_controller.rb b/app/controllers/plans_controller.rb index 6031374..623d18e 100644 --- a/app/controllers/plans_controller.rb +++ b/app/controllers/plans_controller.rb @@ -32,29 +32,30 @@ def create @plan = Plan.new authorize @plan + + # We set these ids to -1 on the page to trick ariatiseForm into allowing the autocomplete to be blank if + # the no org/funder checkboxes are checked off + org_id = (plan_params[:org_id] == '-1' ? '' : plan_params[:org_id]) + funder_id = (plan_params[:funder_id] == '-1' ? '' : plan_params[:funder_id]) - @plan.principal_investigator = current_user.surname.blank? ? nil : "#{current_user.firstname} #{current_user.surname}" - @plan.principal_investigator_email = current_user.email - - orcid = current_user.identifier_for(IdentifierScheme.find_by(name: 'orcid')) - @plan.principal_investigator_identifier = orcid.identifier unless orcid.nil? - - @plan.funder_name = plan_params[:funder_name] - - @plan.visibility = (plan_params['visibility'].blank? ? Rails.application.config.default_plan_visibility : - plan_params[:visibility]) - - # If a template hasn't been identified look for the available templates + # If the template_id is blank then we need to look up the available templates and return JSON if plan_params[:template_id].blank? - template_options(plan_params[:org_id], plan_params[:funder_id]) - - # Return the 'Select a template' section - respond_to do |format| - format.js {} - end - - # Otherwise create the plan + templates = template_options(org_id, funder_id) + render json: {"templates": templates.collect{|t| {id: t.id, title: t.title} }}.to_json + else + # Otherwise create the plan + @plan.principal_investigator = current_user.surname.blank? ? nil : "#{current_user.firstname} #{current_user.surname}" + @plan.principal_investigator_email = current_user.email + + orcid = current_user.identifier_for(IdentifierScheme.find_by(name: 'orcid')) + @plan.principal_investigator_identifier = orcid.identifier unless orcid.nil? + + @plan.funder_name = plan_params[:funder_name] + + @plan.visibility = (plan_params['visibility'].blank? ? Rails.application.config.default_plan_visibility : + plan_params[:visibility]) + @plan.template = Template.find(plan_params[:template_id]) if plan_params[:title].blank? @@ -68,9 +69,8 @@ @plan.assign_creator(current_user) # pre-select org's guidance - ggs = GuidanceGroup.where(org_id: plan_params[:org_id], - optional_subset: false, - published: true) + ggs = GuidanceGroup.where(org_id: org_id, optional_subset: false, published: true) + if !ggs.blank? then @plan.guidance_groups << ggs end default = Template.find_by(is_default: true) @@ -90,17 +90,14 @@ msg += " #{_('This plan is based on the')} #{@plan.template.org.name} template." end - flash[:notice] = msg - respond_to do |format| - format.js { render js: "window.location='#{plan_url(@plan)}?editing=true'" } + format.html { redirect_to plan_path(@plan), notice: msg } end else # Something went wrong so report the issue to the user - flash[:alert] = failed_create_error(@plan, 'Plan') respond_to do |format| - format.js {} + format.html { redirect_to new_plan_path, alert: failed_create_error(@plan, 'Plan') } end end end @@ -327,7 +324,6 @@ private - def plan_params params.require(:plan).permit(:org_id, :org_name, :funder_id, :funder_name, :template_id, :title, :visibility, :grant_number, :description, :identifier, :principal_investigator, @@ -421,46 +417,42 @@ # Collect all of the templates available for the org+funder combination # -------------------------------------------------------------------------- def template_options(org_id, funder_id) - @templates = [] + 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 - @msg = _("We found multiple DMP templates corresponding to the research organisation.") if @templates.count > 1 + 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 + 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| + 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 + templates.delete(tmplt) + templates << customization end end end - - @msg = _("We found multiple DMP templates corresponding to the funder.") if @templates.count > 1 end end # If no templates were available use the generic templates - if @templates.empty? - @msg = _("Using the generic Data Management Plan") - @templates << Template.where(is_default: true, published: true).first + if templates.empty? + templates << Template.where(is_default: true, published: true).first end - @templates = @templates.sort{|x,y| x.title <=> y.title } if @templates.count > 1 + templates = (templates.count > 0 ? templates.sort{|x,y| x.title <=> y.title} : []) end end diff --git a/app/views/layouts/_es5_scripts.html.erb b/app/views/layouts/_es5_scripts.html.erb index 6045c17..b88ee02 100644 --- a/app/views/layouts/_es5_scripts.html.erb +++ b/app/views/layouts/_es5_scripts.html.erb @@ -33,8 +33,6 @@ <%= javascript_include_tag 'views/home/index.js' %> <%= javascript_include_tag 'views/orgs/admin_edit.js' %> <%= javascript_include_tag 'views/orgs/shibboleth_ds.js' %> - <%= javascript_include_tag 'views/plans/available_templates.js' %> - <%= javascript_include_tag 'views/plans/index.js' %> <%= javascript_include_tag 'views/registrations/sign_in_sign_up.js' %> <%= javascript_include_tag 'views/shared/login_form.js' %> <%= javascript_include_tag 'views/shared/register_form.js' %> diff --git a/app/views/plans/new.html.erb b/app/views/plans/new.html.erb index 515cd05..af9500f 100644 --- a/app/views/plans/new.html.erb +++ b/app/views/plans/new.html.erb @@ -10,7 +10,7 @@
- <%= form_for @plan, html: {method: :post, id: 'create-plan'}, remote: true do |f| %> + <%= form_for Plan.new, url: plans_path do |f| %>

<%= _('What research project are you planning?') %>

@@ -63,16 +63,20 @@
-
-
- -
-
- +
+

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

+
+
+ <%= select_tag(:plan_template_id, "", name: 'plan[template_id]', + class: 'form-control', 'aria-describedby': 'template-selection') %> +
+
+ <%= _('Only one DMP template is available for those research and funding organisations.') %> + <%= _('We found multiple DMP templates corresponding to your funder.') %> +
- <%= f.hidden_field(:template_id) %> <%= f.hidden_field(:visibility, value: @is_test ? 'is_test' : Rails.application.config.default_plan_visibility) %> <%= f.button(_('Create plan'), class: "btn btn-primary", type: "submit") %> <% end %> diff --git a/lib/assets/javascripts/application.js b/lib/assets/javascripts/application.js index 55662be..b8834fe 100644 --- a/lib/assets/javascripts/application.js +++ b/lib/assets/javascripts/application.js @@ -6,6 +6,7 @@ import './views/phases/edit'; import './views/plans/download'; import './views/plans/edit_details'; +import './views/plans/index'; import './views/plans/new'; import './views/plans/share'; import './views/devise/registrations/edit'; diff --git a/lib/assets/javascripts/spec/autoCompleteSpec.js b/lib/assets/javascripts/spec/autoCompleteSpec.js index 08965ea..22278d4 100644 --- a/lib/assets/javascripts/spec/autoCompleteSpec.js +++ b/lib/assets/javascripts/spec/autoCompleteSpec.js @@ -14,13 +14,4 @@ fixture.cleanup(); $('body').html(''); }); - - it('shows/hides the clear button correctly', () => { - - }); - - it('Selects the correct id based on the item selected in the combobox', () => { - - }); - }); diff --git a/lib/assets/javascripts/utils/autoComplete.js b/lib/assets/javascripts/utils/autoComplete.js index 44b77f1..83376fb 100644 --- a/lib/assets/javascripts/utils/autoComplete.js +++ b/lib/assets/javascripts/utils/autoComplete.js @@ -13,7 +13,7 @@ if (crosswalk && idField) { const json = JSON.parse(`${$(crosswalk).val().replace(/\\"/g, '"').replace(/\\'/g, '\'')}`); const selection = json[$(el).val()]; - $(idField).val(selection === 'undefined' ? '' : selection).change(); + $(`#${idField}`).val(selection === undefined ? '' : selection).change(); } }; @@ -39,10 +39,10 @@ const debounced = debounce((e) => { toggleClearButton(e); updateIdField(e); - }, 500); + }, 300); // When the value in the combobox changes update the hidden id field - $(el).on('keyup', (e) => { + $(el).on('keyup, blur', (e) => { debounced($(e.currentTarget)); }); @@ -54,5 +54,8 @@ // Show/hide the clear button on page load toggleClearButton(el); + + // Update the hidden ID field on initialize + updateIdField(el); }); }; diff --git a/lib/assets/javascripts/views/plans/new.js b/lib/assets/javascripts/views/plans/new.js index 815c832..0605727 100644 --- a/lib/assets/javascripts/views/plans/new.js +++ b/lib/assets/javascripts/views/plans/new.js @@ -1,41 +1,85 @@ import ariatiseForm from '../../utils/ariatiseForm'; import initAutoComplete from '../../utils/autoComplete'; +import { isObject, isArray } from '../../utils/isType'; -const handleCheckboxClick = (name, checked) => { - $(`#plan_${name}_name`).prop('disabled', checked); - $('#plan_template_id').val('').change(); - $('#available-templates').fadeOut(); +$(() => { + // AJAX success function for available template search + const success = (data) => { + if (isObject(data) && + isArray(data.templates)) { + // Display the available_templates section + if (data.templates.length > 0) { + data.templates.forEach((t) => { + $('#plan_template_id').append(``); + }); + // If there is only one template, show it but disable the dropdown and hide + // the 'Multiple templates found message' + if (data.templates.length === 1) { + $('#plan_template_id option').attr('selected', 'true'); + $('#single-template').show(); + $('#multiple-templates').hide(); + } else { + $('#single-template').hide(); + $('#multiple-templates').show(); + } + $('#available-templates').fadeIn(); + } else { + $('#available-templates').fadeOut(); + // TODO adequate error handling for no templates returned + // (this would mean there is no default template!) + } + } + }; + // AJAX error function for available template search + const error = () => { + // TODO adequate error handling for network error + }; + const getAction = jQueryForm => jQueryForm.attr('action'); + const getMethod = jQueryForm => jQueryForm.attr('method'); - if (checked) { - $(`#plan_${name}_name`).val(''); - $(`#plan_${name}_id`).val('-1').change(); - $(`#plan_${name}_name`).siblings('.combobox-clear-button').hide(); - } else { - $(`#plan_${name}_id`).val('').change(); - } -}; + // When one of the autocomplete fields changes, fetch the available templates + const handleComboboxChange = () => { + const validOrg = ($('#plan_org_id').val().trim().length > 0 || $('#plan_no_org').prop('checked')); + const validFunder = ($('#plan_funder_id').val().trim().length > 0 || $('#plan_no_funder').prop('checked')); -const handleComboboxChange = () => { - const validOrg = ($('#plan_org_id').val().trim().length > 0 || $('#plan_no_org').prop('checked')); - const validFunder = ($('#plan_funder_id').val().trim().length > 0 || $('#plan_no_funder').prop('checked')); + if (!validOrg || !validFunder) { + $('#available-templates').fadeOut(); + $('#plan_template_id').val(''); + } else { + // Clear out the old template dropdown contents + $('#plan_template_id option').remove(); - if (!validOrg || !validFunder) { + // Fetch the available templates fbased on the funder and research org selected + const jQueryForm = $('form'); + const formElements = jQueryForm.serializeArray(); + $.ajax({ + method: getMethod(jQueryForm), + url: getAction(jQueryForm), + data: formElements, + }).done(success, error); + } + }; + + // When one of the checkboxes is clicked, disable the autocomplete input and clear its contents + const handleCheckboxClick = (name, checked) => { + $(`#plan_${name}_name`).prop('disabled', checked); + $('#plan_template_id').val('').change(); $('#available-templates').fadeOut(); - $('#plan_template_id').val(''); - } -}; -$().ready(() => { + if (checked) { + $(`#plan_${name}_name`).val(''); + $(`#plan_${name}_id`).val('-1'); + $(`#plan_${name}_name`).siblings('.combobox-clear-button').hide(); + } else { + $(`#plan_${name}_id`).val(''); + } + handleComboboxChange(); + }; + initAutoComplete(); - ariatiseForm({ selector: '#create-plan' }); - + ariatiseForm({ selector: '#new_plan' }); const defaultVisibility = $('#plan_visibility').val(); - // Initialize the form - handleComboboxChange(); - handleCheckboxClick('org', $('#plan_no_org').prop('checked')); - handleCheckboxClick('funder', $('#plan_no_funder').prop('checked')); - // When the user checks the 'mock project' box we need to set the // visibility to 'is_test' $('#is_test').click((e) => { @@ -60,8 +104,12 @@ handleCheckboxClick(whichOne, e.currentTarget.checked); }); - // When the form receives a valid template id enable the button - $('#plan_template_id').change((e) => { - $('#create_plan_submit').attr('aria-disabled', ($(e.currentTarget).val().trim().length <= 0)); - }); + // Initialize the form + handleComboboxChange(); + if ($('#plan_no_org').prop('checked')) { + handleCheckboxClick('org', $('#plan_no_org').prop('checked')); + } + if ($('#plan_no_funder').prop('checked')) { + handleCheckboxClick('funder', $('#plan_no_funder').prop('checked')); + } }); diff --git a/test/functional/plans_controller_test.rb b/test/functional/plans_controller_test.rb index a17531e..ef6a4f4 100644 --- a/test/functional/plans_controller_test.rb +++ b/test/functional/plans_controller_test.rb @@ -86,14 +86,16 @@ sign_in @user - post plans_path(format: :js), params + post plans_path(), params assert flash[:notice].start_with?('Successfully') && flash[:notice].include?('created') - assert_response :success - assert assigns(:plan) - assert_equal "Testing Create", Plan.last.title, "expected the record to have been created" + assert_response :redirect + + new_plan = Plan.last + assert_redirected_to plan_url(new_plan) + assert_equal "Testing Create", new_plan.title, "expected the record to have been created" # assert that the default visibility is used when none is specified - assert_equal Rails.application.config.default_plan_visibility, Plan.last.visibility, "Expected the plan to have been assigned the default visibility" + assert_equal Rails.application.config.default_plan_visibility, new_plan.visibility, "Expected the plan to have been assigned the default visibility" end # GET /plan/:id (plan_path) diff --git a/test/integration/template_selection_test.rb b/test/integration/template_selection_test.rb index 2567954..04113ec 100644 --- a/test/integration/template_selection_test.rb +++ b/test/integration/template_selection_test.rb @@ -33,9 +33,12 @@ sign_in @researcher - post plans_path(format: :js), {plan: {org_id: @template.org.id}} + post plans_path(format: :json), {plan: {org_id: @template.org.id}} assert_response :success - assert @response.body.include?("$(\"#plan_template_id\").val(\"#{original_id}\");") + json = JSON.parse(@response.body) + + assert_equal 1, json['templates'].size + assert_equal original_id, json['templates'][0]['id'] assert_equal original_id, Template.live(@template.dmptemplate_id).id # Version the template again @@ -43,9 +46,12 @@ template = version_template(template) # Make sure the published version is used - post plans_path(format: :js), {plan: {org_id: @template.org.id}} + post plans_path(format: :json), {plan: {org_id: @template.org.id}} assert_response :success - assert @response.body.include?("$(\"#plan_template_id\").val(\"#{original_id}\");") + json = JSON.parse(@response.body) + + assert_equal 1, json['templates'].size + assert_equal original_id, json['templates'][0]['id'] assert_equal original_id, Template.live(@template.dmptemplate_id).id # Update the template and make sure the published version stayed the same @@ -54,9 +60,12 @@ sign_in @researcher - post plans_path(format: :js), {plan: {org_id: @template.org.id}} + post plans_path(format: :json), {plan: {org_id: @template.org.id}} assert_response :success - assert @response.body.include?("$(\"#plan_template_id\").val(\"#{original_id}\");") + json = JSON.parse(@response.body) + + assert_equal 1, json['templates'].size + assert_equal original_id, json['templates'][0]['id'] assert_equal original_id, Template.live(@template.dmptemplate_id).id end @@ -67,36 +76,48 @@ sign_in @researcher - post plans_path(format: :js), {plan: {org_id: nil}} + post plans_path(format: :json), {plan: {org_id: nil}} assert_response :success - assert @response.body.include?("$(\"#plan_template_id\").val(\"#{@template.id}\");"), @response.body + json = JSON.parse(@response.body) + + assert_equal 1, json['templates'].size + assert_equal @template.id, json['templates'][0]['id'] end # ---------------------------------------------------------- test 'plan gets org template when no funder' do sign_in @researcher - post plans_path(format: :js), {plan: {org_id: @org.id, funder_id: nil}} + post plans_path(format: :json), {plan: {org_id: @org.id, funder_id: nil}} assert_response :success - assert @response.body.include?("$(\"#plan_template_id\").val(\"#{@org_template.id}\");"), @response.body + json = JSON.parse(@response.body) + + assert_equal 1, json['templates'].size + assert_equal @org_template.id, json['templates'][0]['id'] end # ---------------------------------------------------------- test 'plan gets funder template when no org' do sign_in @researcher - post plans_path(format: :js), {plan: {org_id: nil, funder_id: @funder.id}} + post plans_path(format: :json), {plan: {org_id: nil, funder_id: @funder.id}} assert_response :success - assert @response.body.include?("$(\"#plan_template_id\").val(\"#{@funder_template.id}\");"), @response.body + json = JSON.parse(@response.body) + + assert_equal 1, json['templates'].size + assert_equal @funder_template.id, json['templates'][0]['id'] end # ---------------------------------------------------------- test 'plan gets funder template when org has no customization' do sign_in @researcher - post plans_path(format: :js), {plan: {org_id: @org.id, funder_id: @funder.id}} + post plans_path(format: :json), {plan: {org_id: @org.id, funder_id: @funder.id}} assert_response :success - assert @response.body.include?("$(\"#plan_template_id\").val(\"#{@funder_template.id}\");"), @response.body + json = JSON.parse(@response.body) + + assert_equal 1, json['templates'].size + assert_equal @funder_template.id, json['templates'][0]['id'] end # ---------------------------------------------------------- @@ -109,9 +130,12 @@ sign_in @researcher - post plans_path(format: :js), {plan: {org_id: @org.id, funder_id: @funder.id}} + post plans_path(format: :json), {plan: {org_id: @org.id, funder_id: @funder.id}} assert_response :success - assert @response.body.include?("$(\"#plan_template_id\").val(\"#{customization.id}\");"), @response.body + json = JSON.parse(@response.body) + + assert_equal 1, json['templates'].size + assert_equal customization.id, json['templates'][0]['id'] end # ---------------------------------------------------------- @@ -123,11 +147,13 @@ sign_in @researcher - post plans_path(format: :js), {plan: {org_id: @org.id, funder_id: @funder.id}} + post plans_path(format: :json), {plan: {org_id: @org.id, funder_id: @funder.id}} assert_response :success - assert_select "option", 3, "expected a dropdown with 2 templates and a 'please select' option" - assert @response.body.include?("