Skip to content

Commit 48eeee9

Browse files
committed
♻️ Fix variable name in #add_provider_to_user
* `#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
1 parent c30cefa commit 48eeee9

File tree

3 files changed

+32
-11
lines changed

3 files changed

+32
-11
lines changed

lib/sorcery/model/submodules/external.rb

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -95,15 +95,13 @@ module InstanceMethods
9595
def add_provider_to_user(provider, uid)
9696
authentications = sorcery_config.authentications_class.name.demodulize.underscore.pluralize
9797
# first check to see if user has a particular authentication already
98-
if sorcery_adapter.find_authentication_by_oauth_credentials(authentications, provider, uid).nil?
99-
user = send(authentications).build(sorcery_config.provider_uid_attribute_name => uid,
100-
sorcery_config.provider_attribute_name => provider)
101-
user.sorcery_adapter.save(validate: false)
102-
else
103-
user = false
104-
end
98+
return false if sorcery_adapter.find_authentication_by_oauth_credentials(authentications, provider, uid)
10599

106-
user
100+
authentication = send(authentications).build(sorcery_config.provider_uid_attribute_name => uid,
101+
sorcery_config.provider_attribute_name => provider)
102+
authentication.sorcery_adapter.save(validate: false)
103+
104+
authentication
107105
end
108106
end
109107
end

spec/rails_app/app/controllers/sorcery_controller.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -445,10 +445,10 @@ def test_add_second_provider
445445

446446
return unless logged_in?
447447

448-
if (@user = add_provider_to_user(provider))
449-
redirect_to 'bla', notice: 'Success!'
448+
if (@authentication = add_provider_to_user(provider))
449+
redirect_to "bla", notice: "Success!"
450450
else
451-
redirect_to 'blu', alert: 'Failed!'
451+
redirect_to "blu", alert: "Failed!"
452452
end
453453
end
454454

spec/shared_examples/user_oauth_shared_examples.rb

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,5 +29,28 @@
2929
external_user
3030
expect(User.load_from_provider(:twitter, 980_342)).to be_nil
3131
end
32+
33+
describe "#add_provider_to_user" do
34+
let!(:user) { create_new_user }
35+
36+
subject { user.add_provider_to_user(:twitter, 123) }
37+
38+
context "when a provider is successfully added" do
39+
it "returns an instance of authentication" do
40+
expect {
41+
expect(subject).to be_kind_of(Authentication)
42+
}.to change(Authentication, :count).by(1)
43+
end
44+
end
45+
46+
context "when a provider already exists" do
47+
let(:user) { create_new_external_user :twitter }
48+
49+
it "does not create a new authentication and returns false" do
50+
expect { subject }.not_to change(Authentication, :count)
51+
expect(subject).to be false
52+
end
53+
end
54+
end
3255
end
3356
end

0 commit comments

Comments
 (0)