From cec47222d9f28bba98398d0888f7283d4aeba5d2 Mon Sep 17 00:00:00 2001 From: Dylan Date: Sun, 7 Dec 2025 20:56:11 +0800 Subject: [PATCH 1/2] Fix Identity#destroy callback ordering and enable user reactivation Add prepend: true to before_destroy callback so deactivation runs before dependent: :nullify clears identity_id. Preserve identity link in User#deactivate to enable reactivation when rejoining via join code. Also improves Identity::Joinable#join to properly reactivate inactive users and grant board access. --- app/models/identity/joinable.rb | 26 +++++++++++++++++++++++--- app/models/user.rb | 14 +++++++++----- 2 files changed, 32 insertions(+), 8 deletions(-) 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 20ee1c40cb..d5542697a9 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -16,12 +16,16 @@ class User < ApplicationRecord scope :with_avatars, -> { preload(:account, :avatar_attachment) } + # Deactivates the user within this account while preserving the identity link. + # + # The identity reference is intentionally kept so that: + # 1. Identity#destroy triggers this via `dependent: :nullify` (which severs the link) + # 2. UsersController#destroy preserves the link, allowing reactivation if the + # user rejoins via join code (see Identity::Joinable#join) 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? From 208da8d4a68c482aba34633992f22efb339a59ae Mon Sep 17 00:00:00 2001 From: Dylan Date: Wed, 10 Dec 2025 04:01:58 +0800 Subject: [PATCH 2/2] Add tests for user reactivation feature --- .../controllers/join_codes_controller_test.rb | 21 ++++++++++++++++++ test/models/identity/joinable_test.rb | 22 +++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/test/controllers/join_codes_controller_test.rb b/test/controllers/join_codes_controller_test.rb index 415bde3e50..8786d37d40 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