From 48eeee9cbbb155fe7ea754ba808fa94f4b741159 Mon Sep 17 00:00:00 2001 From: Martin Tomov Date: Thu, 27 Feb 2020 23:58:41 +0000 Subject: [PATCH] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Fix=20variable=20name=20in?= =?UTF-8?q?=20#add=5Fprovider=5Fto=5Fuser?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * `#add_provider_to_user` is returning an Authentication instance using a `user` variable name. * This commit changes the variable name to match the return type, i.e. `user` -> `authentication` * Similarly amend a confusing test example --- lib/sorcery/model/submodules/external.rb | 14 +++++------ .../app/controllers/sorcery_controller.rb | 6 ++--- .../user_oauth_shared_examples.rb | 23 +++++++++++++++++++ 3 files changed, 32 insertions(+), 11 deletions(-) diff --git a/lib/sorcery/model/submodules/external.rb b/lib/sorcery/model/submodules/external.rb index 7c33d8a0..96268742 100644 --- a/lib/sorcery/model/submodules/external.rb +++ b/lib/sorcery/model/submodules/external.rb @@ -95,15 +95,13 @@ module InstanceMethods 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) - else - user = false - end + return false if sorcery_adapter.find_authentication_by_oauth_credentials(authentications, provider, uid) - user + authentication = send(authentications).build(sorcery_config.provider_uid_attribute_name => uid, + sorcery_config.provider_attribute_name => provider) + authentication.sorcery_adapter.save(validate: false) + + authentication end end end diff --git a/spec/rails_app/app/controllers/sorcery_controller.rb b/spec/rails_app/app/controllers/sorcery_controller.rb index 1f843c37..a4ffbcf0 100644 --- a/spec/rails_app/app/controllers/sorcery_controller.rb +++ b/spec/rails_app/app/controllers/sorcery_controller.rb @@ -445,10 +445,10 @@ def test_add_second_provider return unless logged_in? - if (@user = add_provider_to_user(provider)) - redirect_to 'bla', notice: 'Success!' + if (@authentication = add_provider_to_user(provider)) + redirect_to "bla", notice: "Success!" else - redirect_to 'blu', alert: 'Failed!' + redirect_to "blu", alert: "Failed!" end end diff --git a/spec/shared_examples/user_oauth_shared_examples.rb b/spec/shared_examples/user_oauth_shared_examples.rb index 3e6ec302..14ee34fc 100644 --- a/spec/shared_examples/user_oauth_shared_examples.rb +++ b/spec/shared_examples/user_oauth_shared_examples.rb @@ -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 authentication" do + expect { + expect(subject).to be_kind_of(Authentication) + }.to change(Authentication, :count).by(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