🐛 Fix return value of #add_provider_to_user on User instance

  * Before, the method would return an instance of Authentication,
    instead of the original intention to return an instance of the
    current user.

  * Arguably the instance of the current user is not very useful,
    as it's already known 🤷‍♂, but at least it matches the current
    examples.

Author: mtomov

lib/sorcery/model/submodules/external.rb

@@ -17,16 +17,16 @@ module External
         def self.included(base)
           base.sorcery_config.class_eval do
             attr_accessor :authentications_class,
-                          :authentications_user_id_attribute_name,
-                          :provider_attribute_name,
-                          :provider_uid_attribute_name
+              :authentications_user_id_attribute_name,
+              :provider_attribute_name,
+              :provider_uid_attribute_name
           end
 
           base.sorcery_config.instance_eval do
-            @defaults.merge!(:@authentications_class                  => nil,
+            @defaults.merge!(:@authentications_class => nil,
                              :@authentications_user_id_attribute_name => :user_id,
-                             :@provider_attribute_name                => :provider,
-                             :@provider_uid_attribute_name            => :uid)
+                             :@provider_attribute_name => :provider,
+                             :@provider_uid_attribute_name => :uid)
 
             reset!
           end
@@ -96,14 +96,14 @@ def add_provider_to_user(provider, uid)
             authentications = sorcery_config.authentications_class.name.demodulize.underscore.pluralize
             # first check to see if user has a particular authentication already
             if sorcery_adapter.find_authentication_by_oauth_credentials(authentications, provider, uid).nil?
-              user = send(authentications).build(sorcery_config.provider_uid_attribute_name => uid,
-                                                 sorcery_config.provider_attribute_name => provider)
-              user.sorcery_adapter.save(validate: false)
+              authentication = send(authentications).build(sorcery_config.provider_uid_attribute_name => uid,
+                                                           sorcery_config.provider_attribute_name => provider)
+              authentication.sorcery_adapter.save(validate: false)
             else
-              user = false
+              return false
             end
 
-            user
+            self
           end
         end
       end

spec/shared_examples/user_oauth_shared_examples.rb

@@ -1,19 +1,19 @@
-shared_examples_for 'rails_3_oauth_model' do
+shared_examples_for "rails_3_oauth_model" do
   # ----------------- PLUGIN CONFIGURATION -----------------------
 
   let(:external_user) { create_new_external_user :twitter }
 
-  describe 'loaded plugin configuration' do
+  describe "loaded plugin configuration" do
     before(:all) do
       Authentication.sorcery_adapter.delete_all
       User.sorcery_adapter.delete_all
 
       sorcery_reload!([:external])
       sorcery_controller_property_set(:external_providers, [:twitter])
       sorcery_model_property_set(:authentications_class, Authentication)
-      sorcery_controller_external_property_set(:twitter, :key, 'eYVNBjBDi33aa9GkA3w')
-      sorcery_controller_external_property_set(:twitter, :secret, 'XpbeSdCoaKSmQGSeokz5qcUATClRW5u08QWNfv71N8')
-      sorcery_controller_external_property_set(:twitter, :callback_url, 'http://blabla.com')
+      sorcery_controller_external_property_set(:twitter, :key, "eYVNBjBDi33aa9GkA3w")
+      sorcery_controller_external_property_set(:twitter, :secret, "XpbeSdCoaKSmQGSeokz5qcUATClRW5u08QWNfv71N8")
+      sorcery_controller_external_property_set(:twitter, :callback_url, "http://blabla.com")
     end
 
     it "responds to 'load_from_provider'" do
@@ -29,5 +29,28 @@
       external_user
       expect(User.load_from_provider(:twitter, 980_342)).to be_nil
     end
+
+    describe "#add_provider_to_user" do
+      let!(:user) { create_new_user }
+
+      subject { user.add_provider_to_user(:twitter, 123) }
+
+      context "when a provider is successfully added" do
+        it "returns an instance of user" do
+          user = subject
+          expect(user).to be_kind_of(User)
+          expect(user.authentications.count).to eq 1
+        end
+      end
+
+      context "when a provider already exists" do
+        let(:user) { create_new_external_user :twitter }
+
+        it "does not create a new authentication and returns false" do
+          expect { subject }.not_to change(Authentication, :count)
+          expect(subject).to be false
+        end
+      end
+    end
   end
 end