Merge pull request #925 from amatsuda/legacy_to_identity_on_login

Migrate from legacy provider/uid to Identity record on lookup

Author: amatsuda

app/models/user.rb

@@ -41,8 +41,14 @@ def self.from_omniauth(auth, invitation_email = nil)
     return identity.user if identity
 
     # Fall back to legacy lookup in users table
-    user = find_by(provider: auth.provider, uid: auth.uid)
-    return user if user
+    if (user = find_by(provider: auth.provider, uid: auth.uid))
+      # Migrate legacy user to identities table
+      transaction do
+        user.identities.create!(provider: auth.provider, uid: auth.uid)
+        user.update_columns(provider: nil, uid: nil)
+      end
+      return user
+    end
 
     # Create new user with identity
     create_from_omniauth!(auth, invitation_email)

spec/models/user_spec.rb

@@ -11,15 +11,25 @@
     let(:email) { '[email protected]' }
     let(:account_name) { 'testuser' }
 
-    context "User already exists" do
-      let!(:user) { create(:user, email: email, name: OmniAuth.config.mock_auth[:github].info.name, uid: OmniAuth.config.mock_auth[:github].uid, provider: "github") }
+    context 'User already exists with legacy provider/uid' do
+      let!(:user) { create(:user, email: email, name: OmniAuth.config.mock_auth[:github].info.name, uid: OmniAuth.config.mock_auth[:github].uid, provider: 'github') }
 
-      it "doesn't create a new user from github auth hash when user exists" do
+      it 'does not create a new user' do
         expect {
           User.from_omniauth(github_auth_hash)
         }.to_not change { User.count }
       end
 
+      it 'migrates legacy user to identities table' do
+        expect {
+          User.from_omniauth(github_auth_hash)
+        }.to change { Identity.count }.by(1)
+
+        user.reload
+        expect(user.identities.find_by(provider: 'github', uid: github_auth_hash.uid)).to be_present
+        expect(user.provider).to be_nil
+        expect(user.uid).to be_nil
+      end
     end
 
     context "User doesn't yet exist" do