Auth::User classes refactor adds should_save?

4 files changed, 10 insertions, 18 deletions

diff --git a/lib/gitlab/auth/ldap/user.rb b/lib/gitlab/auth/ldap/user.rb
index 6487f32f7b6..922d0567d99 100644
--- a/lib/gitlab/auth/ldap/user.rb
+++ b/lib/gitlab/auth/ldap/user.rb
@@ -31,15 +31,11 @@ module Gitlab
           self.class.find_by_uid_and_provider(auth_hash.uid, auth_hash.provider)
         end
 
-        def changed?
+        override :should_save?
+        def should_save?
           gl_user.changed? || gl_user.identities.any?(&:changed?)
         end
 
-        override :omniauth_should_save?
-        def omniauth_should_save?
-          changed? && super
-        end
-
         def block_after_signup?
           ldap_config.block_auto_created_users
         end
diff --git a/lib/gitlab/auth/o_auth/user.rb b/lib/gitlab/auth/o_auth/user.rb
index f2923c29163..6c5d0788a0a 100644
--- a/lib/gitlab/auth/o_auth/user.rb
+++ b/lib/gitlab/auth/o_auth/user.rb
@@ -69,14 +69,14 @@ module Gitlab
         end
 
         def find_and_update!
-          save if omniauth_should_save?
+          save if should_save?
 
           gl_user
         end
 
         protected
 
-        def omniauth_should_save?
+        def should_save?
           true
         end
 
diff --git a/lib/gitlab/auth/saml/user.rb b/lib/gitlab/auth/saml/user.rb
index 557e6aa21a4..cb01cd8004c 100644
--- a/lib/gitlab/auth/saml/user.rb
+++ b/lib/gitlab/auth/saml/user.rb
@@ -29,17 +29,13 @@ module Gitlab
           user
         end
 
-        def changed?
+        override :should_save?
+        def should_save?
           return true unless gl_user
 
           gl_user.changed? || gl_user.identities.any?(&:changed?)
         end
 
-        override :omniauth_should_save?
-        def omniauth_should_save?
-          changed? && super
-        end
-
         protected
 
         def saml_config
diff --git a/spec/lib/gitlab/auth/ldap/user_spec.rb b/spec/lib/gitlab/auth/ldap/user_spec.rb
index cab2169593a..653c19942ea 100644
--- a/spec/lib/gitlab/auth/ldap/user_spec.rb
+++ b/spec/lib/gitlab/auth/ldap/user_spec.rb
@@ -25,20 +25,20 @@ describe Gitlab::Auth::LDAP::User do
     OmniAuth::AuthHash.new(uid: 'uid=John Smith,ou=People,dc=example,dc=com', provider: 'ldapmain', info: info_upper_case)
   end
 
-  describe '#changed?' do
+  describe '#should_save?' do
     it "marks existing ldap user as changed" do
       create(:omniauth_user, extern_uid: 'uid=John Smith,ou=People,dc=example,dc=com', provider: 'ldapmain')
-      expect(ldap_user.changed?).to be_truthy
+      expect(ldap_user.should_save?).to be_truthy
     end
 
     it "marks existing non-ldap user if the email matches as changed" do
       create(:user, email: 'john@example.com')
-      expect(ldap_user.changed?).to be_truthy
+      expect(ldap_user.should_save?).to be_truthy
     end
 
     it "does not mark existing ldap user as changed" do
       create(:omniauth_user, email: 'john@example.com', extern_uid: 'uid=john smith,ou=people,dc=example,dc=com', provider: 'ldapmain')
-      expect(ldap_user.changed?).to be_falsey
+      expect(ldap_user.should_save?).to be_falsey
     end
   end