Makes ORCid readonly. (#1218)

Closes #518.

- updates psu_identity to 0.4.0
- on login, an actor record gets it's ORCiD from directory-service

.rubocop.yml

@@ -27,6 +27,7 @@ Metrics/BlockLength:
 
 Metrics/CyclomaticComplexity:
   Exclude:
+    - 'app/models/user.rb'
     - 'app/services/doi_service.rb'
     - 'app/services/authorship_migration/work_version_creation_migration.rb'
 

Gemfile

@@ -30,7 +30,7 @@ gem 'omniauth-oauth2', '~> 1.7'
 gem 'omniauth-rails_csrf_protection', '~> 1.0'
 gem 'paper_trail'
 gem 'pg', '>= 0.18', '< 2.0'
-gem 'psu_identity', '~> 0.3'
+gem 'psu_identity', '0.4.0'
 gem 'puma', '~> 4.3'
 gem 'pundit'
 gem 'qa'

Gemfile.lock

@@ -298,9 +298,10 @@ GEM
       pry (~> 0.13.0)
     pry-rails (0.3.9)
       pry (>= 0.10.4)
-    psu_identity (0.3.0)
+    psu_identity (0.4.0)
       faraday (~> 1.0)
       json
+      oauth2
       rake (>= 12.0)
     public_suffix (4.0.6)
     puma (4.3.9)
@@ -550,7 +551,7 @@ DEPENDENCIES
   pg (>= 0.18, < 2.0)
   pry-byebug
   pry-rails
-  psu_identity (~> 0.3)
+  psu_identity (= 0.4.0)
   puma (~> 4.3)
   pundit
   qa

app/controllers/dashboard/profiles_controller.rb

@@ -27,7 +27,6 @@ def creator_params
             :surname,
             :display_name,
             :email,
-            :orcid,
             user_attributes: [
               :id,
               :admin_enabled,

app/models/user.rb

@@ -68,6 +68,7 @@ def self.from_omniauth(auth)
       actor.email = actor.email.presence || auth.info.email
       actor.given_name = actor.given_name.presence || auth.info.given_name
       actor.surname = actor.surname.presence || auth.info.surname || auth.info.family_name
+      actor.orcid = actor.orcid.presence || directory_orcid(auth.uid)
     end
 
     transaction do
@@ -103,4 +104,12 @@ def guest?
   def name
     actor.display_name
   end
+
+  def self.directory_orcid(uid)
+    linked_orcid = PsuIdentity::DirectoryService::Client.new.userid(uid).orc_id
+    OrcidId.new(linked_orcid).to_s
+  rescue StandardError => e
+    logger.error(e)
+    nil
+  end
 end

app/views/dashboard/profiles/edit.html.erb

@@ -19,7 +19,8 @@
         <%= render 'form_fields/text',
                    form: form,
                    attribute: :orcid,
-                   data: { controller: 'inputmask' } %>
+                   data: { controller: 'inputmask' },
+                   readonly: true %>
         <%= render 'user_fields', form: form %>
         <%= render 'admin_fields', form: form if @actor.user.admin_available? %>
       </div>

app/views/form_fields/_text.html.erb

@@ -6,6 +6,7 @@
                       placeholder: true,
                       required: local_assigns[:required].presence,
                       data: local_assigns[:data],
+                      readonly: local_assigns[:readonly].presence,
                       aria: { describedby: hint.dom_id } %>
   <%= form.label attribute %>
 

config/locales/en.yml

@@ -207,7 +207,7 @@ en:
         version_name: For datasets or software using semantic versioning (e.g., x.y.z)
         rights: The license you select determines how your work can be used by others. [Read More](https://docs.scholarsphere.psu.edu/guides/licenses/)
       actor:
-        orcid: "This is a 16-digit identifier in the format ####-####-####-####"
+        orcid: "[Click here to link your ORCiD](https://guides.libraries.psu.edu/orcid)"
         display_name: >
           When you create new works, this will be used as the initial value for the Creator field. For example, if your
           first and last name are Pat Researcher, you might wish to set this to "Dr. Pat Q. Researcher PhD."

spec/controllers/dashboard/profiles_controller_spec.rb

@@ -10,8 +10,7 @@
       'display_name' => "Dr. #{actor_attrs[:given_name]} #{actor_attrs[:surname]} PhD",
       'given_name' => actor_attrs[:given_name],
       'surname' => actor_attrs[:surname],
-      'email' => actor_attrs[:email],
-      'orcid' => actor_attrs[:orcid]
+      'email' => actor_attrs[:email]
     }
   }
 
@@ -61,7 +60,6 @@
           expect(actor.given_name).to eq actor_attrs[:given_name]
           expect(actor.surname).to eq actor_attrs[:surname]
           expect(actor.email).to eq actor_attrs[:email]
-          expect(actor.orcid).to eq actor_attrs[:orcid]
         end
 
         it "redirects to the user's dashboard" do

spec/features/dashboard/profile_spec.rb

@@ -9,6 +9,13 @@
   context 'with a standard user' do
     let(:user) { create(:user, opt_in_stats_email: true) }
 
+    it 'does not have an editable orcid field' do
+      visit edit_dashboard_profile_path
+      expect(page).to have_content('Edit Profile')
+      expect(page).to have_field('ORCiD', readonly: true)
+      expect(page).to have_field('Email', readonly: false)
+    end
+
     it 'displays and updates my profile information' do
       visit edit_dashboard_profile_path
       expect(page).to have_content('Edit Profile')
@@ -23,7 +30,6 @@
       user.actor.reload
       expect(user.actor.given_name).to eq(attributes[:given_name])
       expect(user.actor.surname).to eq(attributes[:surname])
-      expect(user.actor.orcid).to eq(attributes[:orcid])
       expect(user.actor.psu_id).to eq(user.access_id)
       user.reload
       expect(user.opt_in_stats_email).to be(false)

spec/models/actor_spec.rb

@@ -47,32 +47,6 @@
 
       it { is_expected.not_to be_valid }
     end
-
-    context 'when orcid is present but psu_id is not' do
-      subject { build(:actor, psu_id: nil) }
-
-      it { is_expected.to be_valid }
-    end
-
-    context 'when psu_id is present but orcid is not' do
-      subject { build(:actor, orcid: nil) }
-
-      it { is_expected.to be_valid }
-    end
-
-    context 'when the psu_id is present and the orcid is invalid' do
-      subject { build(:actor, orcid: 'asdf') }
-
-      it { is_expected.not_to be_valid }
-    end
-
-    context 'when the orcid is present and the psu_id is invalid' do
-      subject { build(:actor, psu_id: 'abc123') }
-
-      before { create(:actor, psu_id: 'abc123') }
-
-      it { is_expected.not_to be_valid }
-    end
   end
 
   describe 'after_save' do
@@ -159,22 +133,4 @@
     it_behaves_like 'a singlevalued field', :psu_id
     it_behaves_like 'a singlevalued field', :orcid
   end
-
-  describe '#orcid' do
-    context 'when set to valid value' do
-      subject { described_class.new(surname: 'Valid Orcid', orcid: FactoryBotHelpers.generate_orcid) }
-
-      it { is_expected.to be_valid }
-    end
-
-    context 'when set to an invalid value' do
-      subject { actor.errors.full_messages }
-
-      let(:actor) { described_class.new(orcid: Faker::Number.leading_zero_number(digits: 15)) }
-
-      before { actor.validate }
-
-      it { is_expected.to include('ORCiD must be valid') }
-    end
-  end
 end