Skip to content

Commit ac9fd85

Browse files
committed
🐛 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.
1 parent c30cefa commit ac9fd85

File tree

2 files changed

+28
-7
lines changed

2 files changed

+28
-7
lines changed

lib/sorcery/model/submodules/external.rb

Lines changed: 5 additions & 7 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,
98+
return false if sorcery_adapter.find_authentication_by_oauth_credentials(authentications, provider, uid)
99+
100+
authentication = send(authentications).build(sorcery_config.provider_uid_attribute_name => uid,
100101
sorcery_config.provider_attribute_name => provider)
101-
user.sorcery_adapter.save(validate: false)
102-
else
103-
user = false
104-
end
102+
authentication.sorcery_adapter.save(validate: false)
105103

106-
user
104+
self
107105
end
108106
end
109107
end

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 user" do
40+
user = subject
41+
expect(user).to be_kind_of(User)
42+
expect(user.authentications.count).to eq 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)