From 4bb80e972841921fa77bd3cea01979bf1f500467 Mon Sep 17 00:00:00 2001 From: briri Date: Fri, 30 Jul 2021 11:47:00 -0700 Subject: [PATCH] Added maDMP features to the project details page fix for rubocop updated db schema rubocop appeasement fixed issue with db schema eslinter fixes fix weird rspec error fixed issue with schema fixed issue with deserialization service --- app/controllers/plans_controller.rb | 6 +- app/javascript/src/plans/editDetails.js | 233 ++++++++++-------- app/javascript/src/utils/conditionalFields.js | 43 ++++ app/models/concerns/identifiable.rb | 2 +- app/models/plan.rb | 58 +++-- app/models/research_domain.rb | 32 +++ app/models/research_output.rb | 28 --- app/presenters/api/v1/funding_presenter.rb | 13 +- app/presenters/api/v1/plan_presenter.rb | 2 + .../api/v1/contextual_error_service.rb | 2 +- app/services/api/v1/deserialization/plan.rb | 6 + .../api/v1/deserialization_service.rb | 2 +- app/views/api/v1/datasets/_show.json.jbuilder | 7 + app/views/api/v1/plans/_funding.json.jbuilder | 21 +- app/views/api/v1/plans/_show.json.jbuilder | 7 +- app/views/plans/_edit_details.html.erb | 3 +- app/views/plans/_project_details.html.erb | 128 +++++++--- app/views/shared/export/_plan_coversheet.erb | 4 +- config/initializers/_dmproadmap.rb | 6 + .../20210729204238_create_research_domains.rb | 12 + ...ical_issues_and_funding_status_to_plans.rb | 9 + db/migrate/20210729204611_madmp_cleanup.rb | 34 +++ db/schema.rb | 41 ++- lib/tasks/utils/external_api.rake | 101 ++++++++ spec/factories/plans.rb | 29 +-- spec/factories/research_domains.rb | 28 +++ spec/factories/research_outputs.rb | 8 - spec/models/api_client_spec.rb | 1 - spec/models/mime_type_spec.rb | 38 --- spec/models/plan_spec.rb | 35 ++- spec/models/research_domain_spec.rb | 12 + spec/models/research_output_spec.rb | 72 ------ .../api/v1/funding_presenter_spec.rb | 37 ++- spec/presenters/api/v1/plan_presenter_spec.rb | 8 +- .../api/v1/contextual_error_service_spec.rb | 18 +- spec/support/helpers/identifier_helper.rb | 42 ++++ .../v1/plans/_funding.json.jbuilder_spec.rb | 17 +- .../api/v1/plans/_show.json.jbuilder_spec.rb | 10 + 38 files changed, 756 insertions(+), 399 deletions(-) create mode 100644 app/javascript/src/utils/conditionalFields.js create mode 100644 app/models/research_domain.rb create mode 100644 db/migrate/20210729204238_create_research_domains.rb create mode 100644 db/migrate/20210729204412_add_ethical_issues_and_funding_status_to_plans.rb create mode 100644 db/migrate/20210729204611_madmp_cleanup.rb create mode 100644 lib/tasks/utils/external_api.rake create mode 100644 spec/factories/research_domains.rb delete mode 100644 spec/models/mime_type_spec.rb create mode 100644 spec/models/research_domain_spec.rb create mode 100644 spec/support/helpers/identifier_helper.rb diff --git a/app/controllers/plans_controller.rb b/app/controllers/plans_controller.rb index e6061c984b..53874b74fc 100644 --- a/app/controllers/plans_controller.rb +++ b/app/controllers/plans_controller.rb @@ -209,6 +209,8 @@ def show Template.where(family_id: @plan.template.customization_of).first end + @research_domains = ResearchDomain.all.order(:label) + respond_to :html end # rubocop:enable Metrics/AbcSize, Metrics/MethodLength @@ -260,7 +262,7 @@ def update # TODO: For some reason the `fields_for` isn't adding the # appropriate namespace, so org_id represents our funder funder = org_from_params(params_in: attrs, allow_create: true) - @plan.funder_id = funder.id if funder.present? + @plan.funder_id = funder&.id process_grant(grant_params: plan_params[:grant]) attrs.delete(:grant) attrs = remove_org_selection_params(params_in: attrs) @@ -467,6 +469,8 @@ def plan_params params.require(:plan) .permit(:template_id, :title, :visibility, :description, :identifier, :start_date, :end_date, :org_id, :org_name, :org_crosswalk, + :ethical_issues, :ethical_issues_description, :ethical_issues_report, + :research_domain_id, :funding_status, grant: %i[name value], org: %i[id org_id org_name org_sources org_crosswalk], funder: %i[id org_id org_name org_sources org_crosswalk]) diff --git a/app/javascript/src/plans/editDetails.js b/app/javascript/src/plans/editDetails.js index 2debed7832..f32b817240 100644 --- a/app/javascript/src/plans/editDetails.js +++ b/app/javascript/src/plans/editDetails.js @@ -1,127 +1,154 @@ import { initAutocomplete, scrubOrgSelectionParamsOnSubmit } from '../utils/autoComplete'; import { Tinymce } from '../utils/tinymce.js.erb'; +import toggleConditionalFields from '../utils/conditionalFields'; import getConstant from '../utils/constants'; $(() => { const grantIdField = $('.grant-id-typeahead'); const grantIdHidden = $('input#plan_grant_value'); - Tinymce.init(); - $('#is_test').click((e) => { - $('#plan_visibility').val($(e.target).is(':checked') ? 'is_test' : 'privately_visible'); - }); - - // Toggle the disabled flags - const toggleCheckboxes = (selections) => { - $('#priority-guidance-orgs, #other-guidance-orgs').find('input[type="checkbox"]').each((i, el) => { - const checkbox = $(el); - if (selections.length >= getConstant('MAX_NUMBER_GUIDANCE_SELECTIONS')) { - if (checkbox.is(':checked')) { - checkbox.removeAttr('disabled'); - } else { - checkbox.prop('disabled', true); - } - } else { - checkbox.prop('disabled', false); - } - }); - }; - - // Keep the modal window's guidance selections in line with selections on the main page - const syncGuidance = (ctx) => { - const currentList = $(ctx); - const otherList = (currentList.attr('id') === 'priority-guidance-orgs' ? $('#other-guidance-orgs') : $('#priority-guidance-orgs')); - const selections = currentList.find('input[type="checkbox"]:checked').map((i, el) => $(el).val()).get(); - otherList.find('input[type="checkbox"]').each((i, el) => { - const checkbox = $(el); - // Toggle the checked flag to match the current guidance list - if (selections.indexOf(checkbox.val()) >= 0) { - checkbox.prop('checked', true); - } else { - checkbox.prop('checked', false); - } + const form = $('form.edit_plan'); + + if (form.length > 0) { + Tinymce.init({ selector: '#plan_description' }); + Tinymce.init({ selector: '#plan_ethical_issues_description' }); + + $('#is_test').click((e) => { + $('#plan_visibility').val($(e.target).is(':checked') ? 'is_test' : 'privately_visible'); }); - toggleCheckboxes(selections); - }; - const grantNumberInfo = (grantId) => `Grant number: ${grantId}`; + const ethicalIssues = $('#plan_ethical_issues'); + const funderId = $('#plan_org_id'); + + if (ethicalIssues.length > 0) { + // If the user checks the ethical_issues field then display the other ethics fields + ethicalIssues.on('change', () => { + toggleConditionalFields(ethicalIssues, ethicalIssues.prop('checked')); + }).change(); - const setInitialGrantProjectName = () => { - const grantId = grantIdHidden.val(); - const researchProjects = window.researchProjects; - const researchProject = researchProjects.find((datum) => datum.grant_id === grantId); - if (researchProject) { - grantIdField.val(researchProject.description); + toggleConditionalFields(ethicalIssues, ethicalIssues.prop('checked')); } - }; - - const setUpTypeahead = () => { - if ($('.edit_plan').length) { - // TODO: Convert this over so that it just loads in the controller? - // Follow this pattern: - // if ($('#org-details-org-controls').length > 0) { - // initAutocomplete('#org-details-org-controls .autocomplete'); - // } - - $.get('/research_projects.json', (data) => { - window.researchProjects = data; - const descriptionData = $.map((dataIn, datum) => datum.description); - grantIdField.typeahead({ source: descriptionData }); - }).then(() => { setInitialGrantProjectName(); }); - - grantIdField.on('change', () => { - const current = grantIdField.typeahead('getActive'); - if (current) { - // match or partial match found - const currentResearchProject = window.researchProjects.find((datum) => { - const fixString = (string) => String(string).toLowerCase(); - return fixString(datum.description) === fixString(current); - }); - if (currentResearchProject) { - const grantId = currentResearchProject.grant_id; - $('#grant_number_info').html(grantNumberInfo(grantId)); - if (grantId.length > 0) { - grantIdHidden.val(grantId); - } else { - grantIdHidden.val(grantIdField.val()); - } + if (funderId.length > 0) { + // If the plan has a funder defined then display the other funder fields + funderId.on('change', () => { + toggleConditionalFields(funderId, (funderId.val() !== '{"name":""}' && funderId.val() !== '')); + }).change(); + + toggleConditionalFields(funderId, (funderId.val() !== '{"name":""}' && funderId.val() !== '')); + } + + // Toggle the disabled flags + const toggleCheckboxes = (selections) => { + $('#priority-guidance-orgs, #other-guidance-orgs').find('input[type="checkbox"]').each((i, el) => { + const checkbox = $(el); + if (selections.length >= getConstant('MAX_NUMBER_GUIDANCE_SELECTIONS')) { + if (checkbox.is(':checked')) { + checkbox.removeAttr('disabled'); + } else { + checkbox.prop('disabled', true); } } else { - $('#grant_number_info').html(grantNumberInfo('')); - grantIdHidden.val(grantIdField.val()); + checkbox.prop('disabled', false); } }); - } - }; - - $('#other-guidance-orgs').find('input[type="checkbox"]').click((e) => { - const checkbox = $(e.target); - // Since this is the modal window, copy any selections over to the priority list - if (checkbox.is(':checked')) { - const priorityList = $('#priority-guidance-orgs'); - if (priorityList.find(`input[value="${checkbox.val()}"]`).length <= 0) { - const li = checkbox.closest('li'); - // If its a subgroup copy the whole group otherwise just copy the line - if (li.children('.sublist').length > 0) { - priorityList.append(li.closest('ul').parent().clone()); + }; + + // Keep the modal window's guidance selections in line with selections on the main page + const syncGuidance = (ctx) => { + const currentList = $(ctx); + const otherList = (currentList.attr('id') === 'priority-guidance-orgs' ? $('#other-guidance-orgs') : $('#priority-guidance-orgs')); + const selections = currentList.find('input[type="checkbox"]:checked').map((i, el) => $(el).val()).get(); + otherList.find('input[type="checkbox"]').each((i, el) => { + const checkbox = $(el); + // Toggle the checked flag to match the current guidance list + if (selections.indexOf(checkbox.val()) >= 0) { + checkbox.prop('checked', true); } else { - priorityList.append(li.clone()); + checkbox.prop('checked', false); + } + }); + toggleCheckboxes(selections); + }; + + const grantNumberInfo = (grantId) => `Grant number: ${grantId}`; + + const setInitialGrantProjectName = () => { + const grantId = grantIdHidden.val(); + const researchProjects = window.researchProjects; + const researchProject = researchProjects.find((datum) => datum.grant_id === grantId); + if (researchProject) { + grantIdField.val(researchProject.description); + } + }; + + const setUpTypeahead = () => { + if ($('.edit_plan').length) { + // TODO: Convert this over so that it just loads in the controller? + // Follow this pattern: + // if ($('#org-details-org-controls').length > 0) { + // initAutocomplete('#org-details-org-controls .autocomplete'); + // } + + $.get('/research_projects.json', (data) => { + window.researchProjects = data; + const descriptionData = $.map((dataIn, datum) => datum.description); + grantIdField.typeahead({ source: descriptionData }); + }).then(() => { setInitialGrantProjectName(); }); + + grantIdField.on('change', () => { + const current = grantIdField.typeahead('getActive'); + if (current) { + // match or partial match found + const currentResearchProject = window.researchProjects.find((datum) => { + const fixString = (string) => String(string).toLowerCase(); + return fixString(datum.description) === fixString(current); + }); + if (currentResearchProject) { + const grantId = currentResearchProject.grant_id; + $('#grant_number_info').html(grantNumberInfo(grantId)); + if (grantId.length > 0) { + grantIdHidden.val(grantId); + } else { + grantIdHidden.val(grantIdField.val()); + } + } + } else { + $('#grant_number_info').html(grantNumberInfo('')); + grantIdHidden.val(grantIdField.val()); + } + }); + } + }; + + $('#other-guidance-orgs').find('input[type="checkbox"]').click((e) => { + const checkbox = $(e.target); + // Since this is the modal window, copy any selections over to the priority list + if (checkbox.is(':checked')) { + const priorityList = $('#priority-guidance-orgs'); + if (priorityList.find(`input[value="${checkbox.val()}"]`).length <= 0) { + const li = checkbox.closest('li'); + // If its a subgroup copy the whole group otherwise just copy the line + if (li.children('.sublist').length > 0) { + priorityList.append(li.closest('ul').parent().clone()); + } else { + priorityList.append(li.clone()); + } } } - } - syncGuidance(checkbox.closest('ul[id]')); - }); + syncGuidance(checkbox.closest('ul[id]')); + }); - $('#priority-guidance-orgs').find('input[type="checkbox"]').click((e) => { - syncGuidance($(e.target).closest('ul[id]')); - }); + $('#priority-guidance-orgs').find('input[type="checkbox"]').click((e) => { + syncGuidance($(e.target).closest('ul[id]')); + }); - initAutocomplete('#funder-org-controls .autocomplete'); - // Scrub out the large arrays of data used for the Org Selector JS so that they - // are not a part of the form submissiomn - scrubOrgSelectionParamsOnSubmit('form.edit_plan'); + initAutocomplete('#funder-org-controls .autocomplete'); + // Scrub out the large arrays of data used for the Org Selector JS so that they + // are not a part of the form submissiomn + scrubOrgSelectionParamsOnSubmit('form.edit_plan'); - toggleCheckboxes($('#priority-guidance-orgs input[type="checkbox"]:checked').map((i, el) => $(el).val()).get()); + toggleCheckboxes($('#priority-guidance-orgs input[type="checkbox"]:checked').map((i, el) => $(el).val()).get()); - setUpTypeahead(); + setUpTypeahead(); + } }); diff --git a/app/javascript/src/utils/conditionalFields.js b/app/javascript/src/utils/conditionalFields.js new file mode 100644 index 0000000000..df4a2af42f --- /dev/null +++ b/app/javascript/src/utils/conditionalFields.js @@ -0,0 +1,43 @@ +// Logic to hide/dsiplay a set of fields based on whether or not a related checkbox is clicked +// +// Expecting the checkbox and the corresponding fields to be wrapped in +// a element +// +// For example see: app/views/plans/_edit_details.html.erb +// app/javascript/src/plans/editDetails.js +// +import { Tinymce } from './tinymce.js.erb'; + +// Expecting `context` to be the field that triggers the hide/show of the corresponding fields +export default function toggleConditionalFields(context, showThem) { + const container = $(context).closest('conditional'); + + if (container.length > 0) { + if (showThem === true) { + container.find('.toggleable-field').show(); + + // Resize any TinyMCE editors + container.find('.toggleable-field').find('.tinymce').each((_idx, el) => { + const tinymceEditor = Tinymce.findEditorById($(el).attr('id')); + if (tinymceEditor) { + $(tinymceEditor.iframeElement).height(tinymceEditor.settings.autoresize_min_height); + } + }); + } else { + // Clear the contents of any textarea select boxes or input fields + container.find('.toggleable-field').find('input, textarea, select').val('').change(); + + // TODO: clear check boxes and radio buttons as needed + + // Clear the contents of any TinyMCE editors + container.find('.toggleable-field').find('.tinymce').each((_idx, el) => { + const tinymceEditor = Tinymce.findEditorById($(el).attr('id')); + if (tinymceEditor) { + tinymceEditor.setContent(''); + } + }); + + container.find('.toggleable-field').hide(); + } + } +} diff --git a/app/models/concerns/identifiable.rb b/app/models/concerns/identifiable.rb index 3593d8a061..3aea3fc227 100644 --- a/app/models/concerns/identifiable.rb +++ b/app/models/concerns/identifiable.rb @@ -53,7 +53,7 @@ def self.from_identifiers(array:) # gets the identifier for the scheme def identifier_for_scheme(scheme:) scheme = IdentifierScheme.by_name(scheme.downcase).first if scheme.is_a?(String) - identifiers.select { |id| id.identifier_scheme == scheme }.first + identifiers.select { |id| id.identifier_scheme == scheme }.last end # Combines the existing identifiers with the new ones diff --git a/app/models/plan.rb b/app/models/plan.rb index 8ac2ed1a32..a5fe102007 100644 --- a/app/models/plan.rb +++ b/app/models/plan.rb @@ -9,18 +9,9 @@ # # id :integer not null, primary key # complete :boolean default(FALSE) -# data_contact :string -# data_contact_email :string -# data_contact_phone :string # description :text # feedback_requested :boolean default(FALSE) -# funder_name :string -# grant_number :string # identifier :string -# principal_investigator :string -# principal_investigator_email :string -# principal_investigator_identifier :string -# principal_investigator_phone :string # title :string # visibility :integer default(3), not null # created_at :datetime @@ -29,7 +20,11 @@ # org_id :integer # funder_id :integer # grant_id :integer -# api_client_id :integer +# research_domain_id :bigint +# funding_status :integer +# ethical_issues :boolean +# ethical_issues_description :text +# ethical_issues_report :string # # Indexes # @@ -42,10 +37,9 @@ # # fk_rails_... (template_id => templates.id) # fk_rails_... (org_id => orgs.id) +# fk_rails_... (research_domain_id => research_domains.id) # -# TODO: Drop the funder_name and grant_number columns once the funder_id has -# been back filled and we're removing the is_other org stuff class Plan < ApplicationRecord include ConditionalUserMailer @@ -74,6 +68,8 @@ class Plan < ApplicationRecord enum visibility: %i[organisationally_visible publicly_visible is_test privately_visible] + enum funding_status: %i[planned funded denied] + alias_attribute :name, :title attribute :visibility, :integer, default: 3 @@ -88,6 +84,8 @@ class Plan < ApplicationRecord belongs_to :funder, class_name: "Org", optional: true + belongs_to :research_domain, optional: true + has_many :phases, through: :template has_many :sections, through: :phases @@ -117,10 +115,6 @@ class Plan < ApplicationRecord has_many :contributors, dependent: :destroy - has_one :grant, as: :identifiable, dependent: :destroy, class_name: "Identifier" - - belongs_to :api_client, optional: true - # ===================== # = Nested Attributes = # ===================== @@ -172,10 +166,6 @@ class Plan < ApplicationRecord ) } - # TODO: Add in a left join here so we can search contributors as well when - # we move to Rails 5: - # OR lower(contributors.name) LIKE lower(:search_pattern) - # OR lower(identifiers.value) LIKE lower(:search_pattern)", scope :search, lambda { |term| if date_range?(term: term) joins(:template, roles: [user: :org]) @@ -184,13 +174,14 @@ class Plan < ApplicationRecord else search_pattern = "%#{term}%" joins(:template, roles: [user: :org]) + .left_outer_joins(:identifiers, :contributors) .where(Role.creator_condition) .where("lower(plans.title) LIKE lower(:search_pattern) OR lower(orgs.name) LIKE lower (:search_pattern) OR lower(orgs.abbreviation) LIKE lower (:search_pattern) OR lower(templates.title) LIKE lower(:search_pattern) - OR lower(plans.principal_investigator) LIKE lower(:search_pattern) - OR lower(plans.principal_investigator_identifier) LIKE lower(:search_pattern)", + OR lower(contributors.name) LIKE lower(:search_pattern) + OR lower(identifiers.value) LIKE lower(:search_pattern)", search_pattern: search_pattern) end } @@ -568,6 +559,29 @@ def landing_page identifiers.select { |i| %w[doi ark].include?(i.identifier_format) }.first end + # Since the Grant is not a normal AR association, override the getter and setter + def grant + Identifier.find_by(id: grant_id) + end + + # Helper method to convert the grant id value entered by the user into an Identifier + # works with both controller params or an instance of Identifier + def grant=(params) + val = params.present? ? params[:value] : nil + current = grant + + # Remove it if it was blanked out by the user + current.destroy if current.present? && !val.present? + return unless val.present? + + # Create the Identifier if it doesn't exist and then set the id + current.update(value: val) if current.present? && current.value != val + return if current.present? + + current = Identifier.create(identifiable: self, value: val) + self.grant_id = current.id + end + private # Validation to prevent end date from coming before the start date diff --git a/app/models/research_domain.rb b/app/models/research_domain.rb new file mode 100644 index 0000000000..be049807c9 --- /dev/null +++ b/app/models/research_domain.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +# == Schema Information +# +# Table name: research_domains +# +# id :bigint not null, primary key +# identifier :string not null +# label :string not null +# created_at :datetime not null +# updated_at :datetime not null +# parent_id :bigint +# +# Indexes +# +# index_research_domains_on_parent_id (parent_id) +# +# Foreign Keys +# +# fk_rails_... (parent_id => research_domains.id) +# +class ResearchDomain < ApplicationRecord + + # ================ + # = Associations = + # ================ + + # Self join + has_many :sub_fields, class_name: "ResearchDomain", foreign_key: "parent_id" + belongs_to :parent, class_name: "ResearchDomain", optional: true + +end diff --git a/app/models/research_output.rb b/app/models/research_output.rb index a180615207..858ef06d10 100644 --- a/app/models/research_output.rb +++ b/app/models/research_output.rb @@ -8,13 +8,9 @@ # abbreviation :string # access :integer default(0), not null # byte_size :bigint -# coverage_end :datetime -# coverage_region :string -# coverage_start :datetime # description :text # display_order :integer # is_default :boolean default("false") -# mandatory_attribution :text # output_type :integer default(3), not null # output_type_description :string # personal_data :boolean @@ -47,7 +43,6 @@ class ResearchOutput < ApplicationRecord # ================ belongs_to :plan, optional: true - belongs_to :mime_type, optional: true # =============== # = Validations = @@ -58,24 +53,11 @@ class ResearchOutput < ApplicationRecord # Ensure presence of the :output_type_description if the user selected 'other' validates_presence_of :output_type_description, if: -> { other? }, message: PRESENCE_MESSAGE - # Ensure that :coverage_start comes before :coverage_end - validate :end_date_after_start_date # ==================== # = Instance methods = # ==================== - # :mime_type is only applicable for certain :output_types - # This method returns the applicable :mime_types - def available_mime_types - cat = %w[audio video] if audiovisual? || sound? - cat = %w[image] if image? - cat = %w[model] if model_representation? - cat = %w[text] if data_paper? || dataset? || text? - - cat.present? ? MimeType.where(category: cat).order(:description) : [] - end - # TODO: placeholders for once the License, Repository, Metadata Standard and # Resource Type Lookups feature is built. # @@ -109,14 +91,4 @@ def resource_types [] end - private - - # Validation to prevent end date from coming before the start date - def end_date_after_start_date - # allow nil values - return true if coverage_end.blank? || coverage_start.blank? - - errors.add(:coverage_end, _("must be after the start date")) if coverage_end < coverage_start - end - end diff --git a/app/presenters/api/v1/funding_presenter.rb b/app/presenters/api/v1/funding_presenter.rb index ccd1fa0157..2216ace28c 100644 --- a/app/presenters/api/v1/funding_presenter.rb +++ b/app/presenters/api/v1/funding_presenter.rb @@ -11,9 +11,16 @@ class << self # If the plan has a grant number then it has been awarded/granted # otherwise it is 'planned' def status(plan:) - return "planned" unless plan.present? && plan.grant_number.present? - - "granted" + return "planned" unless plan.present? + + case plan.funding_status + when "funded" + "granted" + when "denied" + "rejected" + else + "planned" + end end end diff --git a/app/presenters/api/v1/plan_presenter.rb b/app/presenters/api/v1/plan_presenter.rb index 406a6caf66..a8e2510390 100644 --- a/app/presenters/api/v1/plan_presenter.rb +++ b/app/presenters/api/v1/plan_presenter.rb @@ -16,6 +16,8 @@ def initialize(plan:) @plan = plan + @data_contact = @plan.owner + # Attach the first data_curation role as the data_contact, otherwise # add the contributor to the contributors array @plan.contributors.each do |contributor| diff --git a/app/services/api/v1/contextual_error_service.rb b/app/services/api/v1/contextual_error_service.rb index a6a94494b9..7dc67ad464 100644 --- a/app/services/api/v1/contextual_error_service.rb +++ b/app/services/api/v1/contextual_error_service.rb @@ -54,7 +54,7 @@ def contextualize(errors:, context: "DMP") def valid_plan?(plan:) plan.valid? && (plan.funder.blank? || plan.funder.valid?) && - (plan.grant.blank? || plan.grant.valid?) + (plan.grant.blank? || plan.grant.value.present?) end end diff --git a/app/services/api/v1/deserialization/plan.rb b/app/services/api/v1/deserialization/plan.rb index 5a4492f47b..5fbd2101bc 100644 --- a/app/services/api/v1/deserialization/plan.rb +++ b/app/services/api/v1/deserialization/plan.rb @@ -58,6 +58,12 @@ def deserialize(json: {}) return nil unless plan.present? && plan.template.present? plan.description = json[:description] if json[:description].present? + issues = Api::V1::ConversionService.yes_no_unknown_to_boolean( + json[:ethical_issues_exist] + ) + plan.ethical_issues = issues + plan.ethical_issues_description = json[:ethical_issues_description] + plan.ethical_issues_report = json[:ethical_issues_report] # TODO: Handle ethical issues when the Question is in place diff --git a/app/services/api/v1/deserialization_service.rb b/app/services/api/v1/deserialization_service.rb index 73128217cf..2291781a27 100644 --- a/app/services/api/v1/deserialization_service.rb +++ b/app/services/api/v1/deserialization_service.rb @@ -52,7 +52,7 @@ def translate_role(role:) role = role.gsub("#{url}/", "").downcase if role.include?(url) # Return the role if its a valid one otherwise defualt - return role if ::Contributor.new.respond_to?(role.downcase.to_sym) + return role if ::Contributor.new.all_roles.include?(role.downcase.to_sym) default end diff --git a/app/views/api/v1/datasets/_show.json.jbuilder b/app/views/api/v1/datasets/_show.json.jbuilder index e70fe44546..3a3bb0b3b3 100644 --- a/app/views/api/v1/datasets/_show.json.jbuilder +++ b/app/views/api/v1/datasets/_show.json.jbuilder @@ -20,3 +20,10 @@ json.distribution [plan] do |distribution| json.array! ["application/pdf"] end end + +if plan.research_domain_id.present? + research_domain = ResearchDomain.find_by(id: plan.research_domain_id) + if research_domain.present? + json.keyword [research_domain.label, "#{research_domain.identifier} - #{research_domain.label}"] + end +end diff --git a/app/views/api/v1/plans/_funding.json.jbuilder b/app/views/api/v1/plans/_funding.json.jbuilder index bd3a6cc280..725cda1358 100644 --- a/app/views/api/v1/plans/_funding.json.jbuilder +++ b/app/views/api/v1/plans/_funding.json.jbuilder @@ -19,4 +19,23 @@ if plan.grant_id.present? && plan.grant.present? json.partial! "api/v1/identifiers/show", identifier: plan.grant end end -json.funding_status plan.grant.present? ? "granted" : "planned" + +json.funding_status Api::V1::FundingPresenter.status(plan: plan) + +# DMPTool extensions to the RDA common metadata standard +# ------------------------------------------------------ + +# We collect a user entered ID on the form, so this is a way to convey it to other systems +# The ID would typically be something relevant to the funder or research organization +if plan.identifier.present? + json.dmproadmap_funding_opportunity_id do + json.partial! "api/v1/identifiers/show", identifier: Identifier.new(identifiable: plan, + value: plan.identifier) + end +end + +# Since the Plan owner (aka contact) and contributor orgs could be different than the +# one associated with the Plan, we add it here. +json.dmproadmap_funded_affiliations [plan.org] do |funded_org| + json.partial! "api/v1/orgs/show", org: funded_org +end diff --git a/app/views/api/v1/plans/_show.json.jbuilder b/app/views/api/v1/plans/_show.json.jbuilder index d0311aeb0c..6f8d226f5f 100644 --- a/app/views/api/v1/plans/_show.json.jbuilder +++ b/app/views/api/v1/plans/_show.json.jbuilder @@ -15,10 +15,9 @@ json.language Api::V1::LanguagePresenter.three_char_code( json.created plan.created_at.to_formatted_s(:iso8601) json.modified plan.updated_at.to_formatted_s(:iso8601) -# TODO: Update this to pull from the appropriate question once the work is complete -json.ethical_issues_exist "unknown" -# json.ethical_issues_description "" -# json.ethical_issues_report "" +json.ethical_issues_exist Api::V1::ConversionService.boolean_to_yes_no_unknown(plan.ethical_issues) +json.ethical_issues_description plan.ethical_issues_description +json.ethical_issues_report plan.ethical_issues_report id = presenter.identifier if id.present? diff --git a/app/views/plans/_edit_details.html.erb b/app/views/plans/_edit_details.html.erb index c2bbcc43dd..e6cc4a747a 100644 --- a/app/views/plans/_edit_details.html.erb +++ b/app/views/plans/_edit_details.html.erb @@ -5,7 +5,8 @@
<%= render partial: "plans/project_details", - locals: { form: f, plan: plan, orgs: orgs, org_partial: org_partial } %> + locals: { form: f, plan: plan, orgs: orgs, org_partial: org_partial, + research_domains: @research_domains } %> <%= f.button(_('Save'), class: "btn btn-default", type: "submit") %>
diff --git a/app/views/plans/_project_details.html.erb b/app/views/plans/_project_details.html.erb index 3e1a9c0ca7..eba8192601 100644 --- a/app/views/plans/_project_details.html.erb +++ b/app/views/plans/_project_details.html.erb @@ -1,8 +1,13 @@ -<%# locals: form, plan %> +<%# locals: form, plan, research_domains %> -<% project_title_tooltip = _('If applying for funding, state the name exactly as in the grant proposal.') %> -<% project_abstract_tooltip = _("Briefly summarise your research project to help others understand the purposes for which the data are being collected or created.") %> -<% id_tooltip = _('A pertinent ID as determined by the funder and/or organisation.') %> +<% +project_title_tooltip = _('If applying for funding, state the name exactly as in the grant proposal.') +project_abstract_tooltip = _("Briefly summarise your research project to help others understand the purposes for which the data are being collected or created.") +id_tooltip = _('A pertinent ID as determined by the funder and/or organisation.') +ethics_tooltip = _("Whether there are any potential ethical issues related to data that this DMP describes") +ethics_description_tooltip = _("Description of the ethical issues") +ethics_report_tooltip = _("Link to a protocol from a meeting with an ethics commitee") +%>
@@ -36,6 +41,22 @@
+<% if Rails.configuration.x.madmp.enable_research_domain %> +
+
+ <%= form.label(:research_domain_id, _("Research domain"), class: "control-label") %> + + <% options = research_domains.map { |rd| [rd.label, rd.id] } %> + <%= form.select :research_domain_id, options_for_select(options, form.object.research_domain_id), + { + include_blank: _("- Please select one -"), + selected: form.object.research_domain_id + }, + { class: "form-control" } %> +
+
+<% end %> +
<%= form.label(:start_date, _("Project Start"), class: "control-label") %> @@ -81,18 +102,44 @@
<% end %> -
- <% if plan.template.org == plan.funder %> - <%# If the plan's funder is the same as the template owner then just - display the identifiers %> -
- <%= form.label(:funder_name, _('Funder'), class: 'control-label') %> +<% if Rails.configuration.x.madmp.enable_ethical_issues %> + +
+
+
+ <%= form.label(:ethical_issues, class: 'control-label', title: ethics_tooltip) do %> + <%= form.check_box(:ethical_issues) %> + <%= _('Research outputs may have ethical concerns') %> + <% end %> +
+
-
- <%= plan.funder.name %>: +
+
+ <%= form.label(:ethical_issues_description, _('Describe any ethical concerns'), class: 'control-label') %> +
+
+ <%= ethics_description_tooltip %> + <%= form.text_area :ethical_issues_description, rows: 6, class: 'form-control tinymce', + "aria-required": false %> +
- <% else %> - <%# Otherwise display the Org typeahead for funders %> +
+
+ <%= form.label(:ethical_issues_report, _('Ethical protocols'), class: 'control-label') %> +
+
+ <%= ethics_report_tooltip %> + <%= form.url_field(:ethical_issues_report, class: "form-control", "aria-required": false, + 'data-toggle': 'tooltip', + title: ethics_report_tooltip) %> +
+
+ +<% end %> + + +
<%= fields_for :funder, plan.funder do |funder_fields| %> <%= render partial: org_partial, @@ -106,28 +153,41 @@ } %> <% end %>
- <% end %> -
+
-
- <%= form.fields_for :grant, @plan.grant do |grant_fields| %> +
- <%= grant_fields.label(:value, _("Grant number/url"), class: "control-label") %> + <%= form.label(:funding_status, _("Funding status"), class: "control-label") %>
-
- <%# If the OpenAire grant typeahead if enabled use it %> - <% if Rails.configuration.x.open_aire.active %> - - <%= grant_fields.text_field :name, value: @plan.grant&.value, class: "form-control grant-id-typeahead", - autocomplete: "off", aria: { required: false } %> - <%= grant_fields.hidden_field :value %> - Grant number: <%= @plan.grant_number %> - <% else %> - <%= grant_fields.text_field(:value, class: "form-control", - data: { toggle: "tooltip" }, - title: _("Provide a URL to the award's landing page if possible, if not please provide the award/grant number.")) %> - <% end %> + <% funding_statuses = Plan.funding_statuses.map { |status| [status[0].capitalize, status[0]] } %> + <%= form.select :funding_status, options_for_select(funding_statuses, form.object.funding_status), + { + include_blank: _("- Please select one -"), + selected: form.object.funding_status + }, + { class: "form-control" } %>
- <% end %> -
+ + <%= form.fields_for :grant, plan.grant do |grant_fields| %> +
+ <%= grant_fields.label(:value, _("Grant number/url"), class: "control-label") %> +
+ +
+ <%# If the OpenAire grant typeahead if enabled use it %> + <% if Rails.configuration.x.open_aire.active %> + + <%= grant_fields.text_field :name, value: plan.grant&.value, class: "form-control grant-id-typeahead", + autocomplete: "off", aria: { required: false } %> + <%= grant_fields.hidden_field :value %> + Grant number: <%= plan.grant&.value %> + <% else %> + <%= grant_fields.text_field(:value, class: "form-control", + data: { toggle: "tooltip" }, + title: _("Provide a URL to the award's landing page if possible, if not please provide the award/grant number.")) %> + <% end %> +
+ <% end %> +
+ diff --git a/app/views/shared/export/_plan_coversheet.erb b/app/views/shared/export/_plan_coversheet.erb index 3a50c75dcd..df16331646 100644 --- a/app/views/shared/export/_plan_coversheet.erb +++ b/app/views/shared/export/_plan_coversheet.erb @@ -41,8 +41,8 @@

<%= _("Last modified: ") %><%= l(@plan.updated_at.to_date, formats: :short) %>


- <% if @plan.grant_number.present? %> -

<%= _("Grant number / URL: ") %><%= @plan.grant_number %>


+ <% if @plan.grant.present? %> +

<%= _("Grant number / URL: ") %><%= @plan.grant.value %>


<% end %> <% if @public_plan %> diff --git a/config/initializers/_dmproadmap.rb b/config/initializers/_dmproadmap.rb index 1b1fff6ed9..0d44e72c5f 100644 --- a/config/initializers/_dmproadmap.rb +++ b/config/initializers/_dmproadmap.rb @@ -204,6 +204,12 @@ class Application < Rails::Application # ------------------------------------------------------------------------ # config.x.recaptcha.enabled = false + # --------------------------------------------------- # + # Machine Actionable / Networked DMP Features (maDMP) # + # --------------------------------------------------- # + config.x.madmp.enable_ethical_issues = false + config.x.madmp.enable_research_domain = false + end end diff --git a/db/migrate/20210729204238_create_research_domains.rb b/db/migrate/20210729204238_create_research_domains.rb new file mode 100644 index 0000000000..3cb6414b0c --- /dev/null +++ b/db/migrate/20210729204238_create_research_domains.rb @@ -0,0 +1,12 @@ +class CreateFos < ActiveRecord::Migration[5.2] + def change + create_table :research_domains do |t| + t.string :identifier, null: false + t.string :label, null: false + t.references :parent, foreign_key: { to_table: :research_domains } + t.timestamps + end + + add_reference :plans, :research_domain, index: true + end +end diff --git a/db/migrate/20210729204412_add_ethical_issues_and_funding_status_to_plans.rb b/db/migrate/20210729204412_add_ethical_issues_and_funding_status_to_plans.rb new file mode 100644 index 0000000000..a1432822e0 --- /dev/null +++ b/db/migrate/20210729204412_add_ethical_issues_and_funding_status_to_plans.rb @@ -0,0 +1,9 @@ +class AddEthicalIssuesAndFundingStatusToPlans < ActiveRecord::Migration[5.2] + def change + add_column :plans, :ethical_issues, :boolean + add_column :plans, :ethical_issues_description, :text + add_column :plans, :ethical_issues_report, :string + + add_column :plans, :funding_status, :integer, null: true + end +end diff --git a/db/migrate/20210729204611_madmp_cleanup.rb b/db/migrate/20210729204611_madmp_cleanup.rb new file mode 100644 index 0000000000..bfea82c298 --- /dev/null +++ b/db/migrate/20210729204611_madmp_cleanup.rb @@ -0,0 +1,34 @@ +class MadmpCleanup < ActiveRecord::Migration[5.2] + def change + + # Decided not to ask users for the mime type when defining research outputs + drop_table :mime_types + remove_column :research_outputs, :mime_type_id + + # Remove attributes found in the RDA common standard that we decided not to use + remove_column :research_outputs, :mandatory_attribution + remove_column :research_outputs, :coverage_region + remove_column :research_outputs, :coverage_start + remove_column :research_outputs, :coverage_end + + # We're going to move towards a different solution allowing multiple api_clients + # to have an interest in a plan + remove_column :plans, :api_client_id + + # Remove the old principal_investigator and data_contact fields since they now + # live in the contributors table + remove_column :plans, :data_contact + remove_column :plans, :data_contact_email + remove_column :plans, :data_contact_phone + + remove_column :plans, :principal_investigator + remove_column :plans, :principal_investigator_email + remove_column :plans, :principal_investigator_identifier + remove_column :plans, :principal_investigator_phone + + + # Remove the old funder and grant fields since they have been replaced by associations + remove_column :plans, :funder_name + remove_column :plans, :grant_number + end +end diff --git a/db/schema.rb b/db/schema.rb index afeebf74be..e36701b4ce 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2020_12_16_140226) do +ActiveRecord::Schema.define(version: 2021_07_29_204611) do create_table "annotations", id: :integer, force: :cascade do |t| t.integer "question_id" @@ -157,15 +157,6 @@ t.boolean "default_language" end - create_table "mime_types", force: :cascade do |t| - t.string "description", null: false - t.string "category", null: false - t.string "value", null: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.index ["value"], name: "index_mime_types_on_value" - end - create_table "notes", id: :integer, force: :cascade do |t| t.integer "user_id" t.text "text" @@ -257,18 +248,9 @@ t.integer "template_id" t.datetime "created_at" t.datetime "updated_at" - t.string "grant_number" t.string "identifier" t.text "description" - t.string "principal_investigator" - t.string "principal_investigator_identifier" - t.string "data_contact" - t.string "funder_name" t.integer "visibility", default: 3, null: false - t.string "data_contact_email" - t.string "data_contact_phone" - t.string "principal_investigator_email" - t.string "principal_investigator_phone" t.boolean "feedback_requested", default: false t.boolean "complete", default: false t.integer "org_id" @@ -276,11 +258,16 @@ t.integer "grant_id" t.datetime "start_date" t.datetime "end_date" - t.integer "api_client_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 ["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 ["template_id"], name: "index_plans_on_template_id" + t.index ["research_domain_id"], name: "index_plans_on_research_domain_id" end create_table "plans_guidance_groups", id: :integer, force: :cascade do |t| @@ -355,6 +342,15 @@ t.integer "super_region_id" end + create_table "research_domains", force: :cascade do |t| + t.string "identifier", null: false + t.string "label", null: false + t.bigint "parent_id" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["parent_id"], name: "index_research_domains_on_parent_id" + end + create_table "research_outputs", force: :cascade do |t| t.integer "plan_id" t.integer "output_type", default: 3, null: false @@ -364,16 +360,11 @@ t.integer "display_order" t.boolean "is_default" t.text "description" - t.integer "mime_type_id" t.integer "access", default: 0, null: false t.datetime "release_date" t.boolean "personal_data" t.boolean "sensitive_data" t.bigint "byte_size" - t.text "mandatory_attribution" - t.datetime "coverage_start" - t.datetime "coverage_end" - t.string "coverage_region" t.datetime "created_at", null: false t.datetime "updated_at", null: false t.index ["output_type"], name: "index_research_outputs_on_output_type" diff --git a/lib/tasks/utils/external_api.rake b/lib/tasks/utils/external_api.rake new file mode 100644 index 0000000000..7b363831f5 --- /dev/null +++ b/lib/tasks/utils/external_api.rake @@ -0,0 +1,101 @@ +# frozen_string_literal: true + +# rubocop:disable Metrics/BlockLength, Layout/LineLength +namespace :external_api do + desc "Seed the Research Domain table with Field of Science categories" + task add_field_of_science_to_research_domains: :environment do + # TODO: If we can identify an external API authority for this information we should switch + # to fetch the list from there instead of the hard-coded list below which was derived from: + # https://www.oecd.org/science/inno/38235147.pdf + [ + { + identifier: "1", + label: "Natural sciences", + children: [ + { identifier: "1.1", label: "Mathematics" }, + { identifier: "1.2", label: "Computer and information sciences" }, + { identifier: "1.3", label: "Physical sciences" }, + { identifier: "1.4", label: "Chemical sciences" }, + { identifier: "1.5", label: "Earth and related environmental sciences" }, + { identifier: "1.6", label: "Biological sciences" }, + { identifier: "1.7", label: "Other natural sciences" } + ] + }, + { + identifier: "2", + label: "Engineering and Technology", + children: [ + { identifier: "2.1", label: "Civil engineering" }, + { identifier: "2.2", label: "Electrical engineering, electronic engineering, information engineering" }, + { identifier: "2.3", label: "Mechanical engineering" }, + { identifier: "2.4", label: "Chemical engineering" }, + { identifier: "2.5", label: "Materials engineering" }, + { identifier: "2.6", label: "Medical engineering" }, + { identifier: "2.7", label: "Environmental engineering" }, + { identifier: "2.8", label: "Environmental biotechnology" }, + { identifier: "2.9", label: "Industrial Biotechnology" }, + { identifier: "2.10", label: "Nano-technology" }, + { identifier: "2.11", label: "Other engineering and technologies" } + ] + }, + { + identifier: "3", + label: "Medical and Health Sciences", + children: [ + { identifier: "3.1", label: "Basic medicine" }, + { identifier: "3.2", label: "Clinical medicine" }, + { identifier: "3.3", label: "Health sciences" }, + { identifier: "3.4", label: "Health biotechnology" }, + { identifier: "3.5", label: "Other medical sciences" } + ] + }, + { + identifier: "4", + label: "Agricultural Sciences", + children: [ + { identifier: "4.1", label: "Agriculture, forestry, and fisheries" }, + { identifier: "4.2", label: "Animal and dairy science" }, + { identifier: "4.3", label: "Veterinary science" }, + { identifier: "4.4", label: "Agricultural biotechnology" }, + { identifier: "4.5", label: "Other agricultural sciences" } + ] + }, + { + identifier: "5", + label: "Social Sciences", + children: [ + { identifier: "5.1", label: "Psychology" }, + { identifier: "5.2", label: "Economics and business" }, + { identifier: "5.3", label: "Educational sciences" }, + { identifier: "5.4", label: "Sociology" }, + { identifier: "5.5", label: "Law" }, + { identifier: "5.6", label: "Political science" }, + { identifier: "5.7", label: "Social and economic geography" }, + { identifier: "5.8", label: "Media and communications" }, + { identifier: "5.7", label: "Other social sciences" } + ] + }, + { + identifier: "6", + label: "Humanities", + children: [ + { identifier: "6.1", label: "History and archaeology" }, + { identifier: "6.2", label: "Languages and literature" }, + { identifier: "6.3", label: "Philosophy, ethics and religion" }, + { identifier: "6.4", label: "Art (arts, history of arts, performing arts, music)" }, + { identifier: "6.5", label: "Other humanities" } + ] + } + ].each do |fos| + p "#{fos[:identifier]} - #{fos[:label]}" + parent = ResearchDomain.find_or_create_by(identifier: fos[:identifier], label: fos[:label]) + + fos[:children].each do |child| + child[:parent_id] = parent.id + p " #{child[:identifier]} - #{child[:label]}" + ResearchDomain.find_or_create_by(child) + end + end + end +end +# rubocop:enable Metrics/BlockLength, Layout/LineLength diff --git a/spec/factories/plans.rb b/spec/factories/plans.rb index ebd56acf16..20bfbdd9a3 100644 --- a/spec/factories/plans.rb +++ b/spec/factories/plans.rb @@ -6,18 +6,13 @@ # # id :integer not null, primary key # complete :boolean default(FALSE) -# data_contact :string -# data_contact_email :string -# data_contact_phone :string # description :text +# ethical_issues :boolean +# ethical_issues_description :text +# ethical_issues_report :string # feedback_requested :boolean default(FALSE) -# funder_name :string -# grant_number :string +# funding_status :integer # identifier :string -# principal_investigator :string -# principal_investigator_email :string -# principal_investigator_identifier :string -# principal_investigator_phone :string # title :string # visibility :integer default(3), not null # created_at :datetime @@ -26,7 +21,7 @@ # org_id :integer # funder_id :integer # grant_id :integer -# api_client_id :integer +# research_domain_id :bigint # # Indexes # @@ -39,6 +34,7 @@ # # fk_rails_... (template_id => templates.id) # fk_rails_... (org_id => orgs.id) +# fk_rails_... (research_domain_id => research_domains.id) # FactoryBot.define do @@ -46,21 +42,16 @@ title { Faker::Company.bs } template org - # TODO: Drop this column once the funder_id has been back filled - # and we're removing the is_other org stuff - grant_number { SecureRandom.rand(1_000) } identifier { SecureRandom.hex } description { Faker::Lorem.paragraph } - principal_investigator { Faker::Name.name } - # TODO: Drop this column once the funder_id has been back filled - # and we're removing the is_other org stuff - funder_name { Faker::Company.name } - data_contact_email { Faker::Internet.safe_email } - principal_investigator_email { Faker::Internet.safe_email } feedback_requested { false } complete { false } start_date { Time.now } end_date { start_date + 2.years } + ethical_issues { [true, false].sample } + ethical_issues_description { Faker::Lorem.paragraph } + ethical_issues_report { Faker::Internet.url } + funding_status { Plan.funding_statuses.keys.sample } transient do phases { 0 } diff --git a/spec/factories/research_domains.rb b/spec/factories/research_domains.rb new file mode 100644 index 0000000000..7d4017f924 --- /dev/null +++ b/spec/factories/research_domains.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +# == Schema Information +# +# Table name: research_domains +# +# id :bigint(8) not null, primary key +# identifier :string(255) not null +# label :string(255) not null +# created_at :datetime not null +# updated_at :datetime not null +# parent_id :bigint(8) +# +# Indexes +# +# index_research_domain_on_parent_id (parent_id) +# +# Foreign Keys +# +# fk_rails_... (parent_id => research_domains.id) +# +FactoryBot.define do + factory :research_domain do + identifier { SecureRandom.uuid } + label { Faker::Lorem.unique.word } + uri { Faker::Internet.url } + end +end diff --git a/spec/factories/research_outputs.rb b/spec/factories/research_outputs.rb index ccc7204d83..807ecd8ad4 100644 --- a/spec/factories/research_outputs.rb +++ b/spec/factories/research_outputs.rb @@ -8,13 +8,9 @@ # abbreviation :string # access :integer default(0), not null # byte_size :bigint -# coverage_end :datetime -# coverage_region :string -# coverage_start :datetime # description :text # display_order :integer # is_default :boolean default("false") -# mandatory_attribution :text # output_type :integer default(3), not null # output_type_description :string # personal_data :boolean @@ -36,11 +32,8 @@ abbreviation { Faker::Lorem.unique.word } access { ResearchOutput.accesses.keys.sample } byte_size { Faker::Number.number } - coverage_end { Time.now + 2.years } - coverage_start { Time.now + 1.month } description { Faker::Lorem.paragraph } is_default { [nil, true, false].sample } - mandatory_attribution { Faker::Music::PearlJam.musician } display_order { Faker::Number.between(from: 1, to: 20) } output_type { ResearchOutput.output_types.keys.sample } output_type_description { Faker::Lorem.sentence } @@ -51,7 +44,6 @@ trait :complete do after(:create) do |research_output| - research_output.mime_type = create(:mime_type) # add a license identifier # add a repository identifier # add a metadata_standard identifier diff --git a/spec/models/api_client_spec.rb b/spec/models/api_client_spec.rb index b33c87d4ad..68fe7089f2 100644 --- a/spec/models/api_client_spec.rb +++ b/spec/models/api_client_spec.rb @@ -34,7 +34,6 @@ context "Associations" do it { is_expected.to belong_to(:org).optional } - it { is_expected.to have_many(:plans) } end context "Instance Methods" do diff --git a/spec/models/mime_type_spec.rb b/spec/models/mime_type_spec.rb deleted file mode 100644 index c6c826c0f1..0000000000 --- a/spec/models/mime_type_spec.rb +++ /dev/null @@ -1,38 +0,0 @@ -# frozen_string_literal: true - -require "rails_helper" - -RSpec.describe MimeType, type: :model do - - context "associations" do - it { is_expected.to have_many :research_outputs } - end - - context "validations" do - it { is_expected.to validate_presence_of(:category) } - it { is_expected.to validate_presence_of(:description) } - it { is_expected.to validate_presence_of(:value) } - end - - it "factory builds a valid model" do - expect(build(:mime_type).valid?).to eql(true) - end - - context "scopes" do - describe ":categories" do - before(:each) do - @categories = [Faker::Lorem.unique.word, Faker::Lorem.unique.word] - @categories.each { |category| 2.times { create(:mime_type, category: category) } } - @results = described_class.categories - end - - it "returns a unique list of categories" do - expect(@results.first).not_to eql(@results.last) - end - it "returns a sorted list of categories" do - expect(@results).to eql(@categories.sort { |a, b| a <=> b }) - end - end - end - -end diff --git a/spec/models/plan_spec.rb b/spec/models/plan_spec.rb index aedf054f2c..2f8f38c2de 100644 --- a/spec/models/plan_spec.rb +++ b/spec/models/plan_spec.rb @@ -4,6 +4,7 @@ describe Plan do + include IdentifierHelper include RolesHelper include TemplateHelper @@ -50,6 +51,8 @@ it { is_expected.to belong_to :org } + it { is_expected.to belong_to(:research_domain).optional } + it { is_expected.to belong_to(:funder).optional } it { is_expected.to have_many :phases } @@ -529,12 +532,11 @@ end - # TODO: Add this one in once we are able to easily do LEFT JOINs in Rails 5 context "when Contributor name matches term" do let!(:plan) { create(:plan, :creator, description: "foolike desc") } let!(:contributor) { create(:contributor, plan: plan, name: "Dr. Foo Bar") } - xit "returns contributor name" do + it "returns contributor name" do expect(subject).to include(plan) end end @@ -1542,7 +1544,6 @@ plan.grant = id plan.save plan2 = create(:plan, grant: id) - expect(plan2.grant).to eql(plan.grant) expect(plan2.grant.value).to eql(plan.grant.value) # Make sure that deleting the plan does not delete the shared grant! plan.destroy @@ -1550,4 +1551,32 @@ end end + describe "#grant association sanity checks" do + let!(:plan) { create(:plan, :creator) } + + it "allows a grant identifier to be associated" do + plan.grant = { value: build(:identifier, identifier_scheme: nil).value } + plan.save + expect(plan.grant.new_record?).to eql(false) + end + it "allows a grant identifier to be deleted" do + plan.grant = { value: build(:identifier, identifier_scheme: nil).value } + plan.save + plan.grant = { value: nil } + plan.save + expect(plan.grant).to eql(nil) + expect(Identifier.last).to eql(nil) + end + it "does not allow multiple grants on a single plan" do + plan.grant = { value: build(:identifier, identifier_scheme: nil).value } + plan.save + val = SecureRandom.uuid + plan.grant = { value: build(:identifier, identifier_scheme: nil, value: val).value } + plan.save + expect(plan.grant.new_record?).to eql(false) + expect(plan.grant.value).to eql(val) + expect(Identifier.all.length).to eql(1) + end + end + end diff --git a/spec/models/research_domain_spec.rb b/spec/models/research_domain_spec.rb new file mode 100644 index 0000000000..a9294fa69c --- /dev/null +++ b/spec/models/research_domain_spec.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require "rails_helper" + +describe ResearchDomain do + + context "associations" do + it { is_expected.to have_many :sub_fields } + it { is_expected.to belong_to(:parent).optional } + end + +end diff --git a/spec/models/research_output_spec.rb b/spec/models/research_output_spec.rb index 221f974fd5..d948f6c206 100644 --- a/spec/models/research_output_spec.rb +++ b/spec/models/research_output_spec.rb @@ -6,7 +6,6 @@ context "associations" do it { is_expected.to belong_to(:plan).optional } - it { is_expected.to belong_to(:mime_type).optional } end # rubocop:disable Layout/LineLength @@ -32,22 +31,6 @@ @subject.dataset! expect(@subject).not_to validate_presence_of(:output_type_description) end - - describe ":coverage_start and :coverage_end" do - it "allows coverage_start to be nil" do - @subject.coverage_start = nil - expect(@subject.valid?).to eql(true) - end - it "allows end_date to be nil" do - @subject.coverage_end = nil - expect(@subject.valid?).to eql(true) - end - it "does not allow end_date to come before start_date" do - @subject.coverage_end = Time.now - @subject.coverage_start = Time.now + 2.days - expect(@subject.valid?).to eql(false) - end - end end # rubocop:enable Layout/LineLength @@ -63,64 +46,9 @@ model.destroy expect(Plan.last).to eql(plan) end - it "does not delete associated mime_type" do - model = create(:research_output, :complete, plan: create(:plan)) - mime_type = model.mime_type - model.destroy - expect(MimeType.last).to eql(mime_type) - end end context "instance methods" do - describe ":available_mime_types" do - before(:each) do - @audiovisuals = %w[audio video].map do |cat| - create(:mime_type, category: cat) - end - @audiovisuals = @audiovisuals.sort { |a, b| a.description <=> b.description } - @images = [create(:mime_type, category: "image")] - @texts = [create(:mime_type, category: "text")] - @models = [create(:mime_type, category: "model")] - @subject = build(:research_output) - end - it "returns an empty array if no :output_type is present" do - @subject.output_type = nil - expect(@subject.available_mime_types.to_a).to eql([]) - end - it "returns an empty array if :output_type has no mime_types defined" do - @subject.physical_object! - expect(@subject.available_mime_types.to_a).to eql([]) - end - it "returns the correct mime_types for :output_type == :audiovisual" do - @subject.audiovisual! - expect(@subject.available_mime_types.to_a).to eql(@audiovisuals) - end - it "returns the correct mime_types for :output_type == :sound" do - @subject.sound! - expect(@subject.available_mime_types.to_a).to eql(@audiovisuals) - end - it "returns the correct mime_types for :output_type == :image" do - @subject.image! - expect(@subject.available_mime_types.to_a).to eql(@images) - end - it "returns the correct mime_types for :output_type == :data_paper" do - @subject.data_paper! - expect(@subject.available_mime_types.to_a).to eql(@texts) - end - it "returns the correct mime_types for :output_type == :dataset" do - @subject.dataset! - expect(@subject.available_mime_types.to_a).to eql(@texts) - end - it "returns the correct mime_types for :output_type == :text" do - @subject.text! - expect(@subject.available_mime_types.to_a).to eql(@texts) - end - it "returns the correct mime_types for :output_type == :model_representation" do - @subject.model_representation! - expect(@subject.available_mime_types.to_a).to eql(@models) - end - end - xit "licenses should have tests once implemented" do true end diff --git a/spec/presenters/api/v1/funding_presenter_spec.rb b/spec/presenters/api/v1/funding_presenter_spec.rb index feb6dde3d6..2216ace28c 100644 --- a/spec/presenters/api/v1/funding_presenter_spec.rb +++ b/spec/presenters/api/v1/funding_presenter_spec.rb @@ -1,21 +1,32 @@ # frozen_string_literal: true -require "rails_helper" +module Api -RSpec.describe Api::V1::FundingPresenter do + module V1 + + class FundingPresenter + + class << self + + # If the plan has a grant number then it has been awarded/granted + # otherwise it is 'planned' + def status(plan:) + return "planned" unless plan.present? + + case plan.funding_status + when "funded" + "granted" + when "denied" + "rejected" + else + "planned" + end + end + + end - describe "#status(plan:)" do - it "returns `planned` if the plan is nil" do - expect(described_class.status(plan: nil)).to eql("planned") - end - it "returns `planned` if the plan's grant_number is nil" do - plan = build(:plan, grant_number: nil) - expect(described_class.status(plan: plan)).to eql("planned") - end - it "returns `granted` if the plan has a grant_number" do - plan = build(:plan, grant_number: Faker::Lorem.word) - expect(described_class.status(plan: plan)).to eql("granted") end + end end diff --git a/spec/presenters/api/v1/plan_presenter_spec.rb b/spec/presenters/api/v1/plan_presenter_spec.rb index ea5fb9071e..83ffb43180 100644 --- a/spec/presenters/api/v1/plan_presenter_spec.rb +++ b/spec/presenters/api/v1/plan_presenter_spec.rb @@ -25,7 +25,13 @@ expect(presenter.data_contact).to eql(nil) expect(presenter.contributors).to eql([]) end - it "sets data_contact" do + it "sets data_contact to the owner" do + # Plan.owner stupidly requires that the record be persisted, so need to create here + plan = create(:plan, :creator) + presenter = described_class.new(plan: plan) + expect(presenter.data_contact).to eql(plan.owner) + end + it "sets data_contact to the data_curation contributor if there is no owner" do expect(@presenter.data_contact).to eql(@data_contact) end it "sets other contributors (including the data_contact)" do diff --git a/spec/services/api/v1/contextual_error_service_spec.rb b/spec/services/api/v1/contextual_error_service_spec.rb index 235135557f..529b9f28c3 100644 --- a/spec/services/api/v1/contextual_error_service_spec.rb +++ b/spec/services/api/v1/contextual_error_service_spec.rb @@ -10,7 +10,7 @@ @plan.contributors << build(:contributor, org: build(:org), investigation: true) @plan.contributors.first.identifiers << build(:identifier) @plan.funder = build(:org) - @plan.grant = build(:identifier) + @plan.grant = { value: build(:identifier).value } end describe "process_plan_errors(plan:)" do @@ -38,9 +38,6 @@ it "contextualizes the :plan funder errors" do expect(@results.include?("Funding name can't be blank")).to eql(true) end - it "contextualizes the :plan grant errors" do - expect(@results.include?("Grant value can't be blank")).to eql(true) - end end describe "contextualize(errors:)" do @@ -106,13 +103,6 @@ expect(result.length).to eql(1) expect(result.first.start_with?("Funding name ")).to eql(true) end - it "returns errors if a Grant is invalid" do - @plan.grant.value = nil - @plan.grant.valid? - result = described_class.contextualize(errors: @plan.grant.errors, context: "Grant") - expect(result.length).to eql(1) - expect(result.first.start_with?("Grant value ")).to eql(true) - end end describe "valid_plan?(plan:)" do @@ -124,13 +114,9 @@ @plan.funder.name = nil expect(described_class.valid_plan?(plan: @plan)).to eql(false) end - it "return false if :plan grant is not valid" do - @plan.grant.value = nil - expect(described_class.valid_plan?(plan: @plan)).to eql(false) - end it "does not require :plan funder and grant to be present" do @plan.funder = nil - @plan.grant = nil + @plan.grant = {} expect(described_class.valid_plan?(plan: @plan)).to eql(true) end it "returns true when everything is valid" do diff --git a/spec/support/helpers/identifier_helper.rb b/spec/support/helpers/identifier_helper.rb new file mode 100644 index 0000000000..8a199a8a48 --- /dev/null +++ b/spec/support/helpers/identifier_helper.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +module IdentifierHelper + + def create_orcid(user:, val: SecureRandom.uuid) + scheme = orcid_scheme + val = append_prefix(scheme: scheme, val: val) + create(:identifier, identifiable: user, identifier_scheme: scheme, value: val) + end + + def orcid_scheme + name = Rails.configuration.x.orcid.name || "orcid" + landing_page = Rails.configuration.x.orcid.landing_page_url || "https://orcid.org/" + scheme = IdentifierScheme.find_by(name: name) + scheme.update(identifier_prefix: landing_page) if scheme.present? + return scheme if scheme.present? + + create(:identifier_scheme, :for_identification, :for_users, name: name, + identifier_prefix: landing_page) + end + + def append_prefix(scheme:, val:) + val = val.start_with?("/") ? val[1..val.length] : val + url = landing_page_for(scheme: scheme) + val.start_with?(url) ? val : "#{url}#{val}" + end + + def remove_prefix(scheme:, val:) + val.gsub(landing_page_for(scheme: scheme), "") + end + + def landing_page_for(scheme:) + url = scheme.identifier_prefix + unless url.present? + url = Rails.configuration.x.send(:"#{scheme.name.downcase}")&.landing_page_url + end + return nil unless url.present? + + %w[/ : & ?].include?(url.last) ? url : "#{url}/" + end + +end diff --git a/spec/views/api/v1/plans/_funding.json.jbuilder_spec.rb b/spec/views/api/v1/plans/_funding.json.jbuilder_spec.rb index 3f200f6274..69dba26ac9 100644 --- a/spec/views/api/v1/plans/_funding.json.jbuilder_spec.rb +++ b/spec/views/api/v1/plans/_funding.json.jbuilder_spec.rb @@ -9,7 +9,9 @@ create(:identifier, identifiable: @funder, identifier_scheme: create(:identifier_scheme, name: "fundref")) @funder.reload - @plan = create(:plan, funder: @funder) + @plan = create(:plan, funder: @funder, org: create(:org), identifier: SecureRandom.uuid) + create(:identifier, identifiable: @plan.org, + identifier_scheme: create(:identifier_scheme, name: "ror")) @grant = create(:identifier, identifiable: @plan) @plan.update(grant_id: @grant.id) @plan.reload @@ -31,10 +33,23 @@ expect(@json[:funder_id][:type]).to eql(id.identifier_format) expect(@json[:funder_id][:identifier]).to eql(id.value) end + it "includes :dmproadmap_funding_opportunity_identifier" do + identifier = @plan.identifier + expect(@json[:dmproadmap_funding_opportunity_id][:type]).to eql("other") + expect(@json[:dmproadmap_funding_opportunity_id][:identifier]).to eql(identifier) + end it "includes :grant_ids" do expect(@json[:grant_id][:type]).to eql(@grant.identifier_format) expect(@json[:grant_id][:identifier]).to eql(@grant.value) end + it "includes :dmproadmap_funded_affiliations" do + org = @plan.org + expect(@json[:dmproadmap_funded_affiliations].any?).to eql(true) + affil = @json[:dmproadmap_funded_affiliations].last + expect(affil[:name]).to eql(org.name) + expect(affil[:affiliation_id][:type]).to eql(org.identifiers.last.identifier_format) + expect(affil[:affiliation_id][:identifier]).to eql(org.identifiers.last.value) + end end end diff --git a/spec/views/api/v1/plans/_show.json.jbuilder_spec.rb b/spec/views/api/v1/plans/_show.json.jbuilder_spec.rb index 2bf3068212..7f800c9d91 100644 --- a/spec/views/api/v1/plans/_show.json.jbuilder_spec.rb +++ b/spec/views/api/v1/plans/_show.json.jbuilder_spec.rb @@ -38,6 +38,16 @@ it "includes the :modified" do expect(@json[:modified]).to eql(@plan.updated_at.to_formatted_s(:iso8601)) end + it "includes :ethical_issues" do + expected = Api::V1::ConversionService.boolean_to_yes_no_unknown(@plan.ethical_issues) + expect(@json[:ethical_issues_exist]).to eql(expected) + end + it "includes :ethical_issues_description" do + expect(@json[:ethical_issues_description]).to eql(@plan.ethical_issues_description) + end + it "includes :ethical_issues_report" do + expect(@json[:ethical_issues_report]).to eql(@plan.ethical_issues_report) + end it "returns the URL of the plan as the :dmp_id if no DOI is defined" do expected = Rails.application.routes.url_helpers.api_v1_plan_url(@plan)