diff --git a/app/models/identity/joinable.rb b/app/models/identity/joinable.rb index 52afb339a0..1d0c4cd05f 100644 --- a/app/models/identity/joinable.rb +++ b/app/models/identity/joinable.rb @@ -1,13 +1,33 @@ module Identity::Joinable extend ActiveSupport::Concern + # Enables user reactivation when rejoining an account after deactivation. + # + # Relies on User#deactivate preserving the identity link (no `identity: nil`). + # When a deactivated user rejoins, find_or_create_by! finds their existing + # record and the reactivation block restores them to active status. + # + # Return value semantics for JoinCode#redeem_if: + # - true: new user created (increments usage count) + # - false: existing user found (active or reactivated, no increment) + def join(account, **attributes) attributes[:name] ||= email_address transaction do - account.users.find_or_create_by!(identity: self) do |user| - user.assign_attributes(attributes) - end.previously_new_record? + user = account.users.find_or_create_by!(identity: self) do |u| + u.assign_attributes(attributes) + end + + # Reactivate if found but inactive. New users get board access via + # after_create_commit callback, but reactivated users need it manually + # since update! doesn't trigger create callbacks. + unless user.previously_new_record? || user.active? + user.update!(active: true) + user.send(:grant_access_to_boards) + end + + user.previously_new_record? end end end diff --git a/app/models/user.rb b/app/models/user.rb index e16d70670a..dfbf462aa3 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -17,11 +17,9 @@ class User < ApplicationRecord has_many :exports, class_name: "Account::Export", dependent: :destroy def deactivate - transaction do - accesses.destroy_all - update! active: false, identity: nil - close_remote_connections - end + accesses.destroy_all + update! active: false + close_remote_connections end def setup? diff --git a/test/controllers/join_codes_controller_test.rb b/test/controllers/join_codes_controller_test.rb index 81a4ea6eae..95b3f353c7 100644 --- a/test/controllers/join_codes_controller_test.rb +++ b/test/controllers/join_codes_controller_test.rb @@ -55,6 +55,27 @@ class JoinCodesControllerTest < ActionDispatch::IntegrationTest assert_redirected_to landing_url(script_name: @account.slug) end + test "create for existing identity with deactivated user" do + identity = identities(:kevin) + user = users(:kevin) + + sign_in_as :kevin + + assert user.setup?, "Kevin should be setup for this test" + user.deactivate + + assert_not user.reload.active?, "user should be inactive after deactivation" + assert_equal identity.id, user.identity_id, "identity link should be preserved after deactivation" + + assert_no_difference -> { User.count } do + post join_path(code: @join_code.code, script_name: @account.slug), params: { email_address: identity.email_address } + end + + user.reload + assert user.active?, "user should be reactivated after joining" + assert_redirected_to landing_url(script_name: @account.slug) + end + test "create for signed-in identity without a user in the account redirects to verification" do identity = identities(:mike) sign_in_as :mike diff --git a/test/models/identity/joinable_test.rb b/test/models/identity/joinable_test.rb index 95ee0e04b6..03569e9297 100644 --- a/test/models/identity/joinable_test.rb +++ b/test/models/identity/joinable_test.rb @@ -34,4 +34,26 @@ class Identity::JoinableTest < ActiveSupport::TestCase assert_not result, "join should return false when user already exists" end end + + test "join reactivates a deactivated user" do + identity = identities(:kevin) + account = accounts("37s") + user = users(:kevin) + + user.stubs(:close_remote_connections) + user.deactivate + + assert_not user.reload.active?, "user should be inactive after deactivation" + assert_empty user.accesses, "user should have no accesses after deactivation" + assert_equal identity.id, user.identity_id, "identity link should be preserved after deactivation" + + assert_no_difference -> { User.count } do + result = identity.join(account) + assert_not result, "join should return false for reactivation" + end + + user.reload + assert user.active?, "user should be active after reactivation" + assert user.accesses.any?, "user should have accesses restored after reactivation" + end end