Refactor and improve tests (#17386)

* Change account and user fabricators to simplify and improve tests

- `Fabricate(:account)` implicitly fabricates an associated `user` if
  no `domain` attribute is given (an account with `domain: nil` is
  considered a local account, but no user record was created), unless
  `user: nil` is passed
- `Fabricate(:account, user: Fabricate(:user))` should still be possible
  but is discouraged.

* Fix and refactor tests

- avoid passing unneeded attributes to `Fabricate(:user)` or
  `Fabricate(:account)`
- avoid embedding `Fabricate(:user)` into a `Fabricate(:account)` or the other
  way around
- prefer `Fabricate(:user, account_attributes: …)` to
  `Fabricate(:user, account: Fabricate(:account, …)`
- also, some tests were using remote accounts with local user records, which is
  not representative of production code.

spec/controllers/admin/accounts_controller_spec.rb

@@ -61,7 +61,7 @@ RSpec.describe Admin::AccountsController, type: :controller do
 
   describe 'GET #show' do
     let(:current_user) { Fabricate(:user, admin: true) }
-    let(:account) { Fabricate(:account, username: 'bob') }
+    let(:account) { Fabricate(:account) }
 
     it 'returns http success' do
       get :show, params: { id: account.id }
@@ -73,7 +73,7 @@ RSpec.describe Admin::AccountsController, type: :controller do
     subject { post :memorialize, params: { id: account.id } }
 
     let(:current_user) { Fabricate(:user, admin: current_user_admin) }
-    let(:account) { Fabricate(:account, user: user) }
+    let(:account) { user.account }
     let(:user) { Fabricate(:user, admin: target_user_admin) }
 
     context 'when user is admin' do
@@ -125,7 +125,7 @@ RSpec.describe Admin::AccountsController, type: :controller do
     subject { post :enable, params: { id: account.id } }
 
     let(:current_user) { Fabricate(:user, admin: admin) }
-    let(:account) { Fabricate(:account, user: user) }
+    let(:account) { user.account }
     let(:user) { Fabricate(:user, disabled: true) }
 
     context 'when user is admin' do

spec/controllers/admin/change_email_controller_spec.rb

@@ -11,10 +11,9 @@ RSpec.describe Admin::ChangeEmailsController, type: :controller do
 
   describe "GET #show" do
     it "returns http success" do
-      account = Fabricate(:account)
-      user = Fabricate(:user, account: account)
+      user = Fabricate(:user)
 
-      get :show, params: { account_id: account.id }
+      get :show, params: { account_id: user.account.id }
 
       expect(response).to have_http_status(200)
     end
@@ -26,12 +25,11 @@ RSpec.describe Admin::ChangeEmailsController, type: :controller do
     end
 
     it "returns http success" do
-      account = Fabricate(:account)
-      user = Fabricate(:user, account: account)
+      user = Fabricate(:user)
 
       previous_email = user.email
 
-      post :update, params: { account_id: account.id, user: { unconfirmed_email: 'test@example.com' } }
+      post :update, params: { account_id: user.account.id, user: { unconfirmed_email: 'test@example.com' } }
 
       user.reload
 
@@ -41,7 +39,7 @@ RSpec.describe Admin::ChangeEmailsController, type: :controller do
 
       expect(UserMailer).to have_received(:confirmation_instructions).with(user, user.confirmation_token, { to: 'test@example.com' })
 
-      expect(response).to redirect_to(admin_account_path(account.id))
+      expect(response).to redirect_to(admin_account_path(user.account.id))
     end
   end
 end

spec/controllers/admin/confirmations_controller_spec.rb

@@ -9,9 +9,8 @@ RSpec.describe Admin::ConfirmationsController, type: :controller do
 
   describe 'POST #create' do
     it 'confirms the user' do
-      account = Fabricate(:account)
-      user = Fabricate(:user, confirmed_at: false, account: account)
-      post :create, params: { account_id: account.id }
+      user = Fabricate(:user, confirmed_at: false)
+      post :create, params: { account_id: user.account.id }
 
       expect(response).to redirect_to(admin_accounts_path)
       expect(user.reload).to be_confirmed
@@ -32,10 +31,9 @@ RSpec.describe Admin::ConfirmationsController, type: :controller do
   end
 
   describe 'POST #resernd' do
-    subject { post :resend, params: { account_id: account.id } }
+    subject { post :resend, params: { account_id: user.account.id } }
 
-    let(:account) { Fabricate(:account) }
-    let!(:user) { Fabricate(:user, confirmed_at: confirmed_at, account: account) }
+    let!(:user) { Fabricate(:user, confirmed_at: confirmed_at) }
 
     before do
       allow(UserMailer).to receive(:confirmation_instructions) { double(:email, deliver_later: nil) }

spec/controllers/admin/resets_controller_spec.rb

@@ -3,7 +3,7 @@ require 'rails_helper'
 describe Admin::ResetsController do
   render_views
 
-  let(:account) { Fabricate(:account, user: Fabricate(:user)) }
+  let(:account) { Fabricate(:account) }
   before do
     sign_in Fabricate(:user, admin: true), scope: :user
   end