From 476fe5e339593c7ec3e2706fbfb3ccfac06d0f96 Mon Sep 17 00:00:00 2001 From: "Michael J. Giarlo" Date: Fri, 6 Oct 2017 09:18:37 -0700 Subject: [PATCH] Robustify ORCiD validation Now handles * http and https URIs * trailing slashes * identifiers with more than 19 characters Also changes default form to be the HTTPS URI rather than the HTTP URI Fixes https://github.com/curationexperts/nurax/issues/105 --- app/models/concerns/hyrax/user.rb | 6 ++--- app/models/hyrax/orcid_validator.rb | 7 +++++- .../dashboard/profiles_controller_spec.rb | 2 +- spec/models/user_spec.rb | 25 +++++++++++++++---- 4 files changed, 30 insertions(+), 10 deletions(-) diff --git a/app/models/concerns/hyrax/user.rb b/app/models/concerns/hyrax/user.rb index a1ff94ebc9..ba91cc2798 100644 --- a/app/models/concerns/hyrax/user.rb +++ b/app/models/concerns/hyrax/user.rb @@ -96,9 +96,9 @@ def normalize_orcid # 1. validation has already flagged the ORCID as invalid # 2. the orcid field is blank # 3. the orcid is already in its normalized form - return if errors[:orcid].first.present? || orcid.blank? || orcid.starts_with?('http://orcid.org/') - bare_orcid = Hyrax::OrcidValidator.match(orcid).string - self.orcid = "http://orcid.org/#{bare_orcid}" + return if errors[:orcid].first.present? || orcid.blank? || orcid.starts_with?('https://orcid.org/') + bare_orcid = Hyrax::OrcidValidator.match(orcid) + self.orcid = "https://orcid.org/#{bare_orcid}" end # Format the json for select2 which requires just an id and a field called text. diff --git a/app/models/hyrax/orcid_validator.rb b/app/models/hyrax/orcid_validator.rb index 0db355e8ce..6335ab9df3 100644 --- a/app/models/hyrax/orcid_validator.rb +++ b/app/models/hyrax/orcid_validator.rb @@ -6,7 +6,12 @@ def validate(record) end def self.match(string) - /\d{4}-\d{4}-\d{4}-\d{3}[\dX]/.match(string) + Regexp.new(orcid_regex).match(string) { |m| m[:orcid] } end + + def self.orcid_regex + '^(?https?://orcid.org/)?(?\d{4}-\d{4}-\d{4}-\d{3}[\dX])/?$' + end + private_class_method :orcid_regex end end diff --git a/spec/controllers/hyrax/dashboard/profiles_controller_spec.rb b/spec/controllers/hyrax/dashboard/profiles_controller_spec.rb index 8b03407ace..cdb6bbee2c 100644 --- a/spec/controllers/hyrax/dashboard/profiles_controller_spec.rb +++ b/spec/controllers/hyrax/dashboard/profiles_controller_spec.rb @@ -127,7 +127,7 @@ expect(u.facebook_handle).to eq 'face' expect(u.googleplus_handle).to eq 'goo' expect(u.linkedin_handle).to eq 'link' - expect(u.orcid).to eq 'http://orcid.org/0000-0000-1111-2222' + expect(u.orcid).to eq 'https://orcid.org/0000-0000-1111-2222' end it 'displays a flash when invalid ORCID is entered' do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index a4bb10eb9f..708a07b5d9 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -96,21 +96,36 @@ expect(user).to be_valid expect(user.save).to be true end - it 'saves when a valid ORCID URI is supplied' do - user.orcid = 'http://orcid.org/0000-0000-1111-2222' + it 'saves when a valid ORCID HTTP URI w/ trailing slash is supplied' do + user.orcid = 'http://orcid.org/0000-0000-1111-2222/' expect(user).to be_valid expect(user.save).to be true end - it 'normalizes bare ORCIDs to URIs' do + it 'saves when a valid ORCID HTTPS URI is supplied' do + user.orcid = 'https://orcid.org/0000-0000-1111-2222' + expect(user).to be_valid + expect(user.save).to be true + end + it 'normalizes bare ORCIDs to HTTPS URIs' do user.orcid = '0000-0000-1111-2222' user.save - expect(user.orcid).to eq 'http://orcid.org/0000-0000-1111-2222' + expect(user.orcid).to eq 'https://orcid.org/0000-0000-1111-2222' + end + it 'normalizes HTTP ORCIDs to HTTPS URIs' do + user.orcid = 'http://orcid.org/0000-0000-1111-2222' + user.save + expect(user.orcid).to eq 'https://orcid.org/0000-0000-1111-2222' end - it 'marks bad ORCIDs as invalid' do + it 'marks short ORCIDs as invalid' do user.orcid = '000-000-111-222' expect(user).not_to be_valid expect(user.save).to be false end + it 'marks long ORCIDs as invalid' do + user.orcid = '0000-0000-1111-222222' + expect(user).not_to be_valid + expect(user.save).to be false + end end describe "#to_param" do