Authenticating ORCID should de-authenticate existing ORCID users

instead of throwing error

Author: fbacall

app/models/profile.rb

@@ -50,14 +50,18 @@ def merge(*others)
 
   def authenticate_orcid(orcid)
     existing = Profile.where(orcid: orcid, orcid_authenticated: true)
-    if existing.any?
-      errors.add(:orcid, :orcid_taken)
-      false
-    else
-      self.orcid = orcid
-      self.orcid_authenticated = true
-      self.save
+    self.orcid = orcid
+    self.orcid_authenticated = true
+    out = self.save
+
+    if out
+      existing.each do |profile|
+        next if profile == self
+        profile.update_column(:orcid_authenticated, false)
+      end
     end
+
+    out
   end
 
   private

test/controllers/orcid_controller_test.rb

@@ -47,20 +47,23 @@ class OrcidControllerTest < ActionController::TestCase
     assert_redirected_to user
   end
 
-  test 'handle callback but do not assign orcid if already used' do
+  test 'handle callback and assign orcid even if already used' do
     mock_images
     user = users(:regular_user)
+    existing_orcid_user = users(:trainer_user)
+    assert existing_orcid_user.profile.orcid_authenticated?
     sign_in user
 
     VCR.use_cassette('orcid/get_token_existing_orcid') do
       get :callback, params: { code: '123xyz' }
     end
 
     profile = user.profile.reload
-    assert profile.orcid.blank?
-    refute profile.orcid_authenticated?
+    assert_equal '0000-0002-1825-0097', profile.orcid
+    assert profile.orcid_authenticated?
+    refute existing_orcid_user.reload.profile.orcid_authenticated?
     assert_redirected_to user
-    assert_includes flash[:error], 'ORCID has already been'
+    assert flash[:error].blank?
   end
 
   test 'do not handle callback if not logged-in' do
@@ -92,7 +95,7 @@ class OrcidControllerTest < ActionController::TestCase
     user = users(:regular_user)
     sign_in user
 
-    VCR.use_cassette('orcid/get_token_orcid_missing') do
+    VCR.use_cassette('orcid/error_500') do
       get :callback, params: { code: '123xyz' }
     end
 

test/models/profile_test.rb

@@ -101,21 +101,23 @@ class ProfileTest < ActiveSupport::TestCase
     assert profile.orcid_authenticated
   end
 
-  test 'does not assign authenticated orcid if already taken' do
+  test 'assigns authenticated orcid and de-authenticates other profiles that use it' do
     existing_profile = profiles(:trainer_one_profile)
     profile = users(:regular_user).profile
     assert existing_profile.orcid.present?
     assert existing_profile.orcid_authenticated
     refute profile.orcid.present?
     refute profile.orcid_authenticated
+    existing_orcid = existing_profile.orcid
 
-    profile.authenticate_orcid(existing_profile.orcid)
+    profile.authenticate_orcid(existing_orcid)
 
-    assert profile.errors.added?(:orcid, :orcid_taken)
-    assert existing_profile.orcid.present?
-    assert existing_profile.orcid_authenticated
-    refute profile.orcid.present?
-    refute profile.orcid_authenticated
+    assert_empty profile.errors
+    assert_equal existing_orcid, profile.orcid
+    assert profile.orcid_authenticated
+    existing_profile.reload
+    assert_equal existing_orcid, existing_profile.orcid
+    refute existing_profile.orcid_authenticated
   end
 
   test 'assigns authenticated orcid if existing orcid is not authenticated' do