diff --git a/app/controllers/join_codes_controller.rb b/app/controllers/join_codes_controller.rb index 5b0977cd5..971b3c6f8 100644 --- a/app/controllers/join_codes_controller.rb +++ b/app/controllers/join_codes_controller.rb @@ -12,7 +12,7 @@ class JoinCodesController < ApplicationController def create identity = Identity.find_or_create_by!(email_address: params.expect(:email_address)) - @join_code.redeem { |account| identity.join(account) } unless identity.member_of?(@join_code.account) + @join_code.redeem_if { |account| identity.join(account) } user = User.active.find_by!(account: @join_code.account, identity: identity) if identity == Current.identity && user.setup? diff --git a/app/models/account/join_code.rb b/app/models/account/join_code.rb index 49fe9af22..82283e4f6 100644 --- a/app/models/account/join_code.rb +++ b/app/models/account/join_code.rb @@ -7,10 +7,9 @@ class Account::JoinCode < ApplicationRecord before_create :generate_code, if: -> { code.blank? } - def redeem + def redeem_if(&block) transaction do - increment!(:usage_count) - yield account if block_given? + increment!(:usage_count) if block.call(account) end end diff --git a/app/models/identity/joinable.rb b/app/models/identity/joinable.rb index 4a6eabcd4..52afb339a 100644 --- a/app/models/identity/joinable.rb +++ b/app/models/identity/joinable.rb @@ -5,11 +5,9 @@ module Identity::Joinable attributes[:name] ||= email_address transaction do - account.users.create!(**attributes, identity: self) + account.users.find_or_create_by!(identity: self) do |user| + user.assign_attributes(attributes) + end.previously_new_record? end end - - def member_of?(account) - account.users.exists?(identity: self) - end end diff --git a/test/controllers/join_codes_controller_test.rb b/test/controllers/join_codes_controller_test.rb index 74613c724..fe8c820d9 100644 --- a/test/controllers/join_codes_controller_test.rb +++ b/test/controllers/join_codes_controller_test.rb @@ -43,7 +43,7 @@ class JoinCodesControllerTest < ActionDispatch::IntegrationTest identity = identities(:jz) sign_in_as :jz - assert identity.member_of?(@account), "JZ should be a member of 37s for this test" + assert identity.users.exists?(account: @account), "JZ should be a member of 37s for this test" assert identity.users.find_by!(account: @account).setup?, "JZ's user should be setup for this test" assert_no_difference -> { Identity.count } do @@ -59,7 +59,7 @@ class JoinCodesControllerTest < ActionDispatch::IntegrationTest identity = identities(:mike) sign_in_as :mike - assert_not identity.member_of?(@account), "Mike should not be a member of 37s for this test" + assert_not identity.users.exists?(account: @account), "Mike should not be a member of 37s for this test" assert_no_difference -> { Identity.count } do assert_difference -> { User.count }, 1 do diff --git a/test/models/account/join_code_test.rb b/test/models/account/join_code_test.rb index 4247ce7ec..bebf07ee1 100644 --- a/test/models/account/join_code_test.rb +++ b/test/models/account/join_code_test.rb @@ -10,11 +10,19 @@ class Account::JoinCodeTest < ActiveSupport::TestCase assert_equal 3, parts.count end - test "redeem" do + test "redeem_if increments usage_count when block returns true" do join_code = account_join_codes(:"37s") - assert_difference -> { join_code.reload.usage_count }, 1 do - join_code.redeem + assert_difference -> { join_code.reload.usage_count }, +1 do + join_code.redeem_if { true } + end + end + + test "redeem_if does not increment usage_count when block returns false" do + join_code = account_join_codes(:"37s") + + assert_no_difference -> { join_code.reload.usage_count } do + join_code.redeem_if { false } end end diff --git a/test/models/identity/joinable_test.rb b/test/models/identity/joinable_test.rb index 8a8d40e1c..95ee0e04b 100644 --- a/test/models/identity/joinable_test.rb +++ b/test/models/identity/joinable_test.rb @@ -1,25 +1,37 @@ require "test_helper" class Identity::JoinableTest < ActiveSupport::TestCase - test "join" do + test "join creates a new user and returns true" do identity = identities(:david) - user = identity.join(accounts(:initech)) - assert_kind_of User, user - assert_equal accounts(:initech), user.account - assert_equal identity.email_address, user.name + assert_difference -> { User.count }, 1 do + result = identity.join(accounts(:initech)) + assert result, "join should return true when creating a new user" + end + user = identity.users.find_by!(account: accounts(:initech)) + assert_equal identity.email_address, user.name + end + + test "join with custom attributes" do identity = identities(:mike) - user = identity.join(accounts("37s"), name: "Mike") - assert_kind_of User, user - assert_equal accounts("37s"), user.account + result = identity.join(accounts("37s"), name: "Mike") + assert result + + user = identity.users.find_by!(account: accounts("37s")) assert_equal "Mike", user.name end - test "member_of?" do + test "join returns false if user already exists" do identity = identities(:david) - assert identity.member_of?(accounts("37s")) - assert_not identity.member_of?(accounts(:initech)) + account = accounts("37s") + + assert identity.users.exists?(account: account), "David should already be a member of 37s" + + assert_no_difference -> { User.count } do + result = identity.join(account) + assert_not result, "join should return false when user already exists" + end end end