diff --git a/Gemfile.lock b/Gemfile.lock index 6b1eadde7..104ea4b50 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -106,7 +106,7 @@ GEM latex-decode (~> 0.0) binding_of_caller (0.8.0) debug_inspector (>= 0.0.1) - bolognese (1.3.9) + bolognese (1.3.10) activesupport (>= 4.2.5, < 6) benchmark_methods (~> 0.7) bibtex-ruby (~> 4.1) @@ -134,7 +134,7 @@ GEM builder (3.2.3) byebug (11.0.1) cancancan (2.3.0) - capybara (3.27.0) + capybara (3.28.0) addressable mini_mime (>= 0.1.3) nokogiri (~> 1.8) diff --git a/app/controllers/dois_controller.rb b/app/controllers/dois_controller.rb index de162d33d..f04c3da02 100644 --- a/app/controllers/dois_controller.rb +++ b/app/controllers/dois_controller.rb @@ -487,9 +487,6 @@ def safe_params fail JSON::ParserError, "You need to provide a payload following the JSONAPI spec" unless params[:data].present? - # default values for attributes stored as JSON - defaults = { data: { titles: [], descriptions: [], types: {}, dates: [], rightsList: [], creators: [], contributors: [] }} - attributes = [ :doi, :confirmDoi, @@ -507,8 +504,8 @@ def safe_params { dates: [:date, :dateType, :dateInformation] }, :subjects, { subjects: [:subject, :subjectScheme, :schemeUri, :valueUri, :lang] }, - { - landingPage: [ + :landingPage, + { landingPage: [ :checked, :url, :status, @@ -567,6 +564,9 @@ def safe_params ] relationships = [{ client: [data: [:type, :id]] }] + # default values for attributes stored as JSON + defaults = { data: { titles: [], descriptions: [], types: {}, container: {}, dates: [], subjects: [], rightsList: [], creators: [], contributors: [], sizes: [], formats: [], contentUrl: [], identifiers: [], relatedIdentifiers: [], fundingReferences: [], geoLocations: [] }} + p = params.require(:data).permit(:type, :id, attributes: attributes, relationships: relationships).reverse_merge(defaults) client_id = p.dig("relationships", "client", "data", "id") || current_user.try(:client_id) p = p.fetch("attributes").merge(client_id: client_id) @@ -619,7 +619,7 @@ def safe_params # merge attributes from xml into regular attributes # make sure we don't accidentally set any attributes to nil read_attrs_keys.each do |attr| - p.merge!(attr.to_s.underscore => p[attr].presence || meta[attr.to_s.underscore].presence || p[attr]) if p.has_key?(attr) || meta.has_key?(attr.to_s.underscore) + p.merge!(attr.to_s.underscore => p[attr] || meta[attr.to_s.underscore] || p[attr]) if p.has_key?(attr) || meta.has_key?(attr.to_s.underscore) end p.merge!(version_info: p[:version] || meta["version_info"]) if p.has_key?(:version_info) || meta["version_info"].present? diff --git a/app/models/doi.rb b/app/models/doi.rb index c46ecbc69..8c64cb405 100644 --- a/app/models/doi.rb +++ b/app/models/doi.rb @@ -85,8 +85,18 @@ class Doi < ActiveRecord::Base validates :xml, presence: true, xml_schema: true, if: Proc.new { |doi| doi.validatable? } validate :check_dates, if: :dates? validate :check_rights_list, if: :rights_list? + validate :check_titles, if: :titles? validate :check_descriptions, if: :descriptions? + validate :check_types, if: :types? + validate :check_container, if: :container? validate :check_subjects, if: :subjects? + validate :check_creators, if: :creators? + validate :check_contributors, if: :contributors? + validate :check_landing_page, if: :landing_page? + validate :check_identifiers, if: :identifiers? + validate :check_related_identifiers, if: :related_identifiers? + validate :check_funding_references, if: :funding_references? + validate :check_geo_locations, if: :geo_locations? after_commit :update_url, on: [:create, :update] after_commit :update_media, on: [:create, :update] @@ -727,6 +737,18 @@ def doi=(value) write_attribute(:doi, value.upcase) if value.present? end + def formats=(value) + write_attribute(:formats, Array.wrap(value)) + end + + def sizes=(value) + write_attribute(:sizes, Array.wrap(value)) + end + + def content_url=(value) + write_attribute(:content_url, Array.wrap(value)) + end + def identifier normalize_doi(doi, sandbox: !Rails.env.production?) end @@ -787,8 +809,8 @@ def update_media media.delete_all - Array.wrap(content_url).each do |c| - media << Media.create(url: c, media_type: formats) + Array.wrap(content_url).each_with_index do |c, index| + media << Media.create(url: c, media_type: Array.wrap(formats)[index]) end end @@ -853,6 +875,12 @@ def check_rights_list end end + def check_titles + Array.wrap(titles).each do |t| + errors.add(:titles, "Title '#{t}' should be an object instead of a string.") unless t.is_a?(Hash) + end + end + def check_descriptions Array.wrap(descriptions).each do |d| errors.add(:descriptions, "Description '#{d}' should be an object instead of a string.") unless d.is_a?(Hash) @@ -865,6 +893,54 @@ def check_subjects end end + def check_creators + Array.wrap(creators).each do |c| + errors.add(:creators, "Creator '#{c}' should be an object instead of a string.") unless c.is_a?(Hash) + end + end + + def check_contributors + Array.wrap(contributors).each do |c| + errors.add(:contributors, "Contributor '#{c}' should be an object instead of a string.") unless c.is_a?(Hash) + end + end + + def check_identifiers + Array.wrap(identifiers).each do |i| + errors.add(:identifiers, "Identifier '#{i}' should be an object instead of a string.") unless i.is_a?(Hash) + end + end + + def check_related_identifiers + Array.wrap(related_identifiers).each do |r| + errors.add(:related_identifiers, "Related identifier '#{r}' should be an object instead of a string.") unless r.is_a?(Hash) + end + end + + def check_funding_references + Array.wrap(funding_references).each do |f| + errors.add(:funding_references, "Funding reference '#{f}' should be an object instead of a string.") unless f.is_a?(Hash) + end + end + + def check_geo_locations + Array.wrap(geo_locations).each do |g| + errors.add(:geo_locations, "Geolocation '#{g}' should be an object instead of a string.") unless g.is_a?(Hash) + end + end + + def check_landing_page + errors.add(:landing_page, "Landing page '#{landing_page}' should be an object instead of a string.") unless landing_page.is_a?(Hash) + end + + def check_types + errors.add(:types, "Types '#{types}' should be an object instead of a string.") unless types.is_a?(Hash) + end + + def check_container + errors.add(:container, "Container '#{container}' should be an object instead of a string.") unless container.is_a?(Hash) + end + # to be used after DOIs were transferred to another DOI RA def self.delete_dois_by_prefix(prefix, options={}) logger = Logger.new(STDOUT) diff --git a/app/models/media.rb b/app/models/media.rb index 1381139ea..3ffa6b227 100644 --- a/app/models/media.rb +++ b/app/models/media.rb @@ -45,7 +45,7 @@ def doi_id=(value) def set_defaults current_media = Media.where(dataset: dataset).order('media.created DESC').first self.version = current_media.present? ? current_media.version + 1 : 0 - self.media_type = "text/plain " if media_type.blank? + self.media_type = "text/plain" if media_type.blank? self.updated = Time.zone.now.utc.iso8601 end end diff --git a/spec/requests/dois_spec.rb b/spec/requests/dois_spec.rb index 6bbc262ad..b87c69fef 100644 --- a/spec/requests/dois_spec.rb +++ b/spec/requests/dois_spec.rb @@ -338,7 +338,7 @@ it 'updates the doi' do put "/dois/#{doi.doi}", valid_attributes, admin_headers - + expect(last_response.status).to eq(200) expect(json.dig('data', 'attributes', 'sizes')).to eq(sizes) end @@ -569,6 +569,64 @@ end end + context 'when the title is changed wrong format' do + let(:xml) { Base64.strict_encode64(file_fixture('datacite.xml').read) } + let(:titles) { "Submitted chemical data for InChIKey=YAPQBXQYLJRXSA-UHFFFAOYSA-N" } + let(:valid_attributes) do + { + "data" => { + "type" => "dois", + "attributes" => { + "url" => "http://www.bl.uk/pdf/pat.pdf", + "xml" => xml, + "titles" => titles, + "event" => "publish" + } + } + } + end + + it 'error' do + patch "/dois/#{doi.doi}", valid_attributes, headers + + expect(last_response.status).to eq(422) + expect(json['errors']).to eq([{"source"=>"titles", "title"=>"Title 'submitted chemical data for inchikey=yapqbxqyljrxsa-uhfffaoysa-n' should be an object instead of a string."}]) + end + end + + context 'when the description is changed to empty' do + let(:xml) { Base64.strict_encode64(file_fixture('datacite.xml').read) } + let(:descriptions) { [] } + let(:valid_attributes) do + { + "data" => { + "type" => "dois", + "attributes" => { + "url" => "http://www.bl.uk/pdf/pat.pdf", + "xml" => xml, + "descriptions" => descriptions, + "event" => "publish" + } + } + } + end + + it 'updates the record' do + patch "/dois/#{doi.doi}", valid_attributes, headers + puts last_response.body + expect(last_response.status).to eq(200) + expect(json.dig('data', 'attributes', 'url')).to eq("http://www.bl.uk/pdf/pat.pdf") + expect(json.dig('data', 'attributes', 'doi')).to eq(doi.doi.downcase) + expect(json.dig('data', 'attributes', 'descriptions')).to eq([]) + end + + it 'sets state to findable' do + patch "/dois/#{doi.doi}", valid_attributes, headers + + expect(json.dig('data', 'attributes', 'state')).to eq("findable") + end + end + context 'when a doi is created ignore reverting back' do let(:xml) { Base64.strict_encode64(file_fixture('datacite.xml').read) } let(:valid_attributes) do @@ -1041,7 +1099,7 @@ it 'updates the record' do patch "/dois/10.14454/8na3-9s47", valid_attributes, headers - + puts last_response.body expect(last_response.status).to eq(201) expect(json.dig('data', 'attributes', 'url')).to eq("https://ors.datacite.org/doi:/10.14454/8na3-9s47") expect(json.dig('data', 'attributes', 'doi')).to eq("10.14454/8na3-9s47") @@ -2174,6 +2232,7 @@ it 'updates the Doi' do patch "/dois/#{doi.doi}", update_attributes, headers + expect(last_response.status).to eq(200) expect(json.dig('data', 'attributes', 'contentUrl')).to eq(content_url) end end @@ -2330,7 +2389,7 @@ patch "/dois/#{doi.doi}", update_attributes_again, headers expect(json.dig('data', 'attributes', 'descriptions').size).to eq(1) - expect(json.dig('data', 'attributes', 'container')).to be_nil + expect(json.dig('data', 'attributes', 'container')).to be_empty end end