From 7554156abe41bb8f1e0af43c2c2d942435db6948 Mon Sep 17 00:00:00 2001 From: Muhsin Keloth Date: Wed, 10 Sep 2025 14:12:22 +0530 Subject: [PATCH] chore: Account switching issue in newly added accounts (#12403) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The system determines a user’s active account by checking the `active_at` field in the `account_users` table and selecting the most recently active account: ```ruby def active_account_user account_users.order(active_at: :desc)&.first end ``` This works fine when all accounts have a valid active_at timestamp. **Problem** When a user is added to a new account, the `active_at` value is NULL (because the account has never been explicitly activated). Ordering by active_at DESC produces inconsistent results across databases, since handling of NULL values differs (sometimes treated as high, sometimes low). As a result: - Mobile apps (critical impact): `/profile` returns the wrong account. The UI keeps showing the old account even after switching, and restarting does not fix it. - Web app (accidentally works): Appears correct because the active account is inferred from the browser URL, but the backend API is still wrong. **Root Cause** - The ordering logic did not account for NULL `active_at`. - New accounts without active_at sometimes get incorrectly prioritized as the “active” account. **Solution** Explicitly ensure that accounts with NULL active_at are sorted after accounts with real timestamps by using NULLS LAST: ```ruby def active_account_user account_users.order(Arel.sql('active_at DESC NULLS LAST, id DESC'))&.first end ``` - Accounts with actual `active_at` values will always be prioritized. - New accounts (with NULL active_at) will be placed at the bottom until the user explicitly activates them. - Adding id DESC as a secondary ordering ensures consistent tie-breaking when multiple accounts have the same `active_at`. --- app/models/concerns/user_attribute_helpers.rb | 2 +- spec/models/user_spec.rb | 37 +++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/app/models/concerns/user_attribute_helpers.rb b/app/models/concerns/user_attribute_helpers.rb index 32ff026ac..442d166ef 100644 --- a/app/models/concerns/user_attribute_helpers.rb +++ b/app/models/concerns/user_attribute_helpers.rb @@ -18,7 +18,7 @@ module UserAttributeHelpers end def active_account_user - account_users.order(active_at: :desc)&.first + account_users.order(Arel.sql('active_at DESC NULLS LAST'))&.first end def current_account_user diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index c740c67fb..845ec0625 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -110,4 +110,41 @@ RSpec.describe User do expect(new_user.email).to eq('test123@test.com') end end + + describe '#active_account_user' do + let(:user) { create(:user) } + let(:account1) { create(:account) } + let(:account2) { create(:account) } + let(:account3) { create(:account) } + + before do + # Create account_users with different active_at values + create(:account_user, user: user, account: account1, active_at: 2.days.ago) + create(:account_user, user: user, account: account2, active_at: 1.day.ago) + create(:account_user, user: user, account: account3, active_at: nil) # New account with NULL active_at + end + + it 'returns the account_user with the most recent active_at, prioritizing timestamps over NULL values' do + # Should return account2 (most recent timestamp) even though account3 was created last with NULL active_at + expect(user.active_account_user.account_id).to eq(account2.id) + end + + it 'returns NULL active_at account only when no other accounts have active_at' do + # Remove active_at from all accounts + user.account_users.each { |au| au.update!(active_at: nil) } + + # Should return one of the accounts (behavior is undefined but consistent) + expect(user.active_account_user).to be_present + end + + context 'when multiple accounts have NULL active_at' do + before do + create(:account_user, user: user, account: create(:account), active_at: nil) + end + + it 'still prioritizes accounts with timestamps' do + expect(user.active_account_user.account_id).to eq(account2.id) + end + end + end end