From 86806525ab0ad0af0649e7a4dd0b36b430ec0830 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Mon, 10 Aug 2026 09:29:10 +0200 Subject: [PATCH 1/4] refactor(invitations): replace attr_accessor hack with has_one association The User model used an attr_accessor for :invitation instead of a proper has_one association. This was a Rails 3 workaround needed because the inverse association wasn't reliably set in memory when building a user through Invitation#make_user. Modern Rails handles has_one/belongs_to inverses correctly, so the attr_accessor is no longer needed. Removing it also means the invitation no longer needs to pass itself as a user attribute; the association is wired automatically when self.user is assigned on the invitation side. Assisted-by: Claude Code --- app/models/invitation.rb | 2 +- app/models/user/invitations.rb | 7 ++----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/app/models/invitation.rb b/app/models/invitation.rb index 348fb37b68..9de9d0465e 100644 --- a/app/models/invitation.rb +++ b/app/models/invitation.rb @@ -24,7 +24,7 @@ class Invitation < ApplicationRecord # Build new user on information in this invitation. def make_user(params = {}) - self.user = account.users.build_with_fields params.reverse_merge(email: email, invitation: self) + self.user = account.users.build_with_fields params.reverse_merge(email: email) end def accepted? diff --git a/app/models/user/invitations.rb b/app/models/user/invitations.rb index 53a51611ee..97649c7c45 100644 --- a/app/models/user/invitations.rb +++ b/app/models/user/invitations.rb @@ -4,10 +4,7 @@ module User::Invitations included do after_commit :accept_invitation, :on => :create - attr_accessor :invitation - - # TODO: refactor to make this work removing above attribute. - # has_one :invitation + has_one :invitation before_destroy :destroy_invitation end @@ -18,7 +15,7 @@ def accept_invitation def destroy_invitation if account # some tests fail because of account being nil - invit = account.invitations.find_by_email(email) || account.invitations.find_by_user_id(id) # || eventually invitation + invit = account.invitations.find_by_email(email) || account.invitations.find_by_user_id(id) # halt the destruction if the destruction of invitation failed throw :abort if invit && invit.destroy == false end From 326bb70f63fb160395bdd0ec29816e3e4b9bdd31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Mon, 10 Aug 2026 10:55:38 +0200 Subject: [PATCH 2/4] test(invitations): assert acceptance on signup The invitation signup tests verified user creation and activation but never checked that the invitation itself was marked as accepted. This is the key side-effect of the after_commit :accept_invitation callback, and the behavior most likely to regress after replacing the attr_accessor hack with a real has_one association. Assisted-by: Claude Code --- .../developer_portal/accounts/invitee_signups_controller_test.rb | 1 + .../provider/invitee_signups_controller_integration_test.rb | 1 + test/unit/authentication/strategy/oauth2_test.rb | 1 + test/unit/authentication/strategy/provider_oauth2_test.rb | 1 + 4 files changed, 4 insertions(+) diff --git a/test/integration/developer_portal/accounts/invitee_signups_controller_test.rb b/test/integration/developer_portal/accounts/invitee_signups_controller_test.rb index 03c6a4fe71..5121e07258 100644 --- a/test/integration/developer_portal/accounts/invitee_signups_controller_test.rb +++ b/test/integration/developer_portal/accounts/invitee_signups_controller_test.rb @@ -46,6 +46,7 @@ def setup assert_equal I18n.t('developer_portal.accounts.invitee_signups.create.success'), flash[:notice] assert_redirected_to login_path + assert invitation.reload.accepted? end test 'create pushes webhook' do diff --git a/test/integration/provider/invitee_signups_controller_integration_test.rb b/test/integration/provider/invitee_signups_controller_integration_test.rb index e3943dea13..cba9d25ded 100644 --- a/test/integration/provider/invitee_signups_controller_integration_test.rb +++ b/test/integration/provider/invitee_signups_controller_integration_test.rb @@ -26,6 +26,7 @@ def setup assert_equal I18n.t('provider.invitee_signups.create.success'), flash[:success] assert_redirected_to provider_login_path + assert invitation.reload.accepted? end test 'do not set unpermitted attributes' do diff --git a/test/unit/authentication/strategy/oauth2_test.rb b/test/unit/authentication/strategy/oauth2_test.rb index 2815f444f3..d2ae3342ef 100644 --- a/test/unit/authentication/strategy/oauth2_test.rb +++ b/test/unit/authentication/strategy/oauth2_test.rb @@ -334,6 +334,7 @@ class SsoSignupTest < ActiveSupport::TestCase assert_equal result.username, user_data[:username] assert result.active? assert authentication_strategy.error_message.blank? + assert invitation.reload.accepted? end end end diff --git a/test/unit/authentication/strategy/provider_oauth2_test.rb b/test/unit/authentication/strategy/provider_oauth2_test.rb index 59e9bf422e..276db8e3fc 100644 --- a/test/unit/authentication/strategy/provider_oauth2_test.rb +++ b/test/unit/authentication/strategy/provider_oauth2_test.rb @@ -258,6 +258,7 @@ class SsoSignupTest < ActiveSupport::TestCase assert_equal result.username, user_data[:username] assert result.active? assert authentication_strategy.error_message.blank? + assert invitation.reload.accepted? end end end From 02eb1e6fc4f37ad5a7a462b02ce0c25025c2374a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Mon, 10 Aug 2026 12:07:33 +0200 Subject: [PATCH 3/4] style(invitations): replace dynamic find_by_* finders Dynamic finder methods (find_by_email, find_by_user_id) are deprecated in favour of find_by(attr:) syntax. Also caches account.invitations into a local variable to avoid calling the same method twice, addressing the reek DuplicateMethodCall warning flagged in code review. Assisted-by: Claude Code --- app/models/user/invitations.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/user/invitations.rb b/app/models/user/invitations.rb index 97649c7c45..354bbf002e 100644 --- a/app/models/user/invitations.rb +++ b/app/models/user/invitations.rb @@ -15,7 +15,8 @@ def accept_invitation def destroy_invitation if account # some tests fail because of account being nil - invit = account.invitations.find_by_email(email) || account.invitations.find_by_user_id(id) + invitations = account.invitations + invit = invitations.find_by(email: email) || invitations.find_by(user_id: id) # halt the destruction if the destruction of invitation failed throw :abort if invit && invit.destroy == false end From e1dd3b7020fb605699577c381bd91b5f01fe59cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Mon, 10 Aug 2026 12:07:47 +0200 Subject: [PATCH 4/4] style(invitations): suppress HasManyOrHasOneDependent cop rubocop requires a :dependent option on has_one/has_many associations. In this case the before_destroy callback already handles invitation destruction (with an abort guard if destruction fails), so adding a Rails-managed dependent option would conflict. Suppressing the cop with an inline comment explains the intentional design. Assisted-by: Claude Code --- app/models/user/invitations.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/user/invitations.rb b/app/models/user/invitations.rb index 354bbf002e..fb7904fd2e 100644 --- a/app/models/user/invitations.rb +++ b/app/models/user/invitations.rb @@ -4,7 +4,7 @@ module User::Invitations included do after_commit :accept_invitation, :on => :create - has_one :invitation + has_one :invitation # rubocop:disable Rails/HasManyOrHasOneDependent -- before_destroy :destroy_invitation handles it before_destroy :destroy_invitation end