Merge branch 'issue_220040_12' into 'master'

Fix Rails/SaveBang offenses

See merge request gitlab-org/gitlab!75974

.rubocop_todo/rails/save_bang.yml

@@ -2,9 +2,3 @@
 Rails/SaveBang:
   Exclude:
   - ee/spec/lib/analytics/merge_request_metrics_calculator_spec.rb
-  - spec/lib/backup/manager_spec.rb
-  - spec/lib/gitlab/alerting/alert_spec.rb
-  - spec/lib/gitlab/analytics/cycle_analytics/records_fetcher_spec.rb
-  - spec/lib/gitlab/auth/ldap/user_spec.rb
-  - spec/lib/gitlab/auth/o_auth/user_spec.rb
-  - spec/lib/gitlab/auth/saml/user_spec.rb

spec/lib/backup/manager_spec.rb

@@ -409,7 +409,7 @@ RSpec.describe Backup::Manager do
 
       # the Fog mock only knows about directories we create explicitly
       connection = ::Fog::Storage.new(Gitlab.config.backup.upload.connection.symbolize_keys)
-      connection.directories.create(key: Gitlab.config.backup.upload.remote_directory)
+      connection.directories.create(key: Gitlab.config.backup.upload.remote_directory) # rubocop:disable Rails/SaveBang
     end
 
     context 'target path' do
@@ -455,7 +455,7 @@ RSpec.describe Backup::Manager do
           }
         )
 
-        connection.directories.create(key: Gitlab.config.backup.upload.remote_directory)
+        connection.directories.create(key: Gitlab.config.backup.upload.remote_directory) # rubocop:disable Rails/SaveBang
       end
 
       context 'with SSE-S3 without using storage_options' do
@@ -521,7 +521,7 @@ RSpec.describe Backup::Manager do
         )
 
         connection = ::Fog::Storage.new(Gitlab.config.backup.upload.connection.symbolize_keys)
-        connection.directories.create(key: Gitlab.config.backup.upload.remote_directory)
+        connection.directories.create(key: Gitlab.config.backup.upload.remote_directory) # rubocop:disable Rails/SaveBang
       end
 
       it 'does not attempt to set ACL' do

spec/lib/gitlab/analytics/cycle_analytics/records_fetcher_spec.rb

@@ -43,8 +43,8 @@ RSpec.describe Gitlab::Analytics::CycleAnalytics::RecordsFetcher do
       end
 
       before do
-        issue1.metrics.update(first_added_to_board_at: 3.days.ago, first_mentioned_in_commit_at: 2.days.ago)
-        issue2.metrics.update(first_added_to_board_at: 3.days.ago, first_mentioned_in_commit_at: 2.days.ago)
+        issue1.metrics.update!(first_added_to_board_at: 3.days.ago, first_mentioned_in_commit_at: 2.days.ago)
+        issue2.metrics.update!(first_added_to_board_at: 3.days.ago, first_mentioned_in_commit_at: 2.days.ago)
       end
 
       context 'when records are loaded by guest' do
@@ -73,8 +73,8 @@ RSpec.describe Gitlab::Analytics::CycleAnalytics::RecordsFetcher do
       end
 
       before do
-        mr1.metrics.update(merged_at: 3.days.ago)
-        mr2.metrics.update(merged_at: 3.days.ago)
+        mr1.metrics.update!(merged_at: 3.days.ago)
+        mr2.metrics.update!(merged_at: 3.days.ago)
       end
 
       include_context 'when records are loaded by maintainer'
@@ -95,9 +95,9 @@ RSpec.describe Gitlab::Analytics::CycleAnalytics::RecordsFetcher do
     end
 
     before(:all) do
-      issue1.metrics.update(first_added_to_board_at: 3.days.ago, first_mentioned_in_commit_at: 2.days.ago)
-      issue2.metrics.update(first_added_to_board_at: 3.days.ago, first_mentioned_in_commit_at: 2.days.ago)
-      issue3.metrics.update(first_added_to_board_at: 3.days.ago, first_mentioned_in_commit_at: 2.days.ago)
+      issue1.metrics.update!(first_added_to_board_at: 3.days.ago, first_mentioned_in_commit_at: 2.days.ago)
+      issue2.metrics.update!(first_added_to_board_at: 3.days.ago, first_mentioned_in_commit_at: 2.days.ago)
+      issue3.metrics.update!(first_added_to_board_at: 3.days.ago, first_mentioned_in_commit_at: 2.days.ago)
     end
 
     before do

spec/lib/gitlab/auth/ldap/user_spec.rb

@@ -53,12 +53,12 @@ RSpec.describe Gitlab::Auth::Ldap::User do
     it "finds the user if already existing" do
       create(:omniauth_user, extern_uid: 'uid=john smith,ou=people,dc=example,dc=com', provider: 'ldapmain')
 
-      expect { ldap_user.save }.not_to change { User.count }
+      expect { ldap_user.save }.not_to change { User.count } # rubocop:disable Rails/SaveBang
     end
 
     it "connects to existing non-ldap user if the email matches" do
       existing_user = create(:omniauth_user, email: 'john@example.com', provider: "twitter")
-      expect { ldap_user.save }.not_to change { User.count }
+      expect { ldap_user.save }.not_to change { User.count } # rubocop:disable Rails/SaveBang
 
       existing_user.reload
       expect(existing_user.ldap_identity.extern_uid).to eql 'uid=john smith,ou=people,dc=example,dc=com'
@@ -67,7 +67,7 @@ RSpec.describe Gitlab::Auth::Ldap::User do
 
     it 'connects to existing ldap user if the extern_uid changes' do
       existing_user = create(:omniauth_user, email: 'john@example.com', extern_uid: 'old-uid', provider: 'ldapmain')
-      expect { ldap_user.save }.not_to change { User.count }
+      expect { ldap_user.save }.not_to change { User.count } # rubocop:disable Rails/SaveBang
 
       existing_user.reload
       expect(existing_user.ldap_identity.extern_uid).to eql 'uid=john smith,ou=people,dc=example,dc=com'
@@ -77,7 +77,7 @@ RSpec.describe Gitlab::Auth::Ldap::User do
 
     it 'connects to existing ldap user if the extern_uid changes and email address has upper case characters' do
       existing_user = create(:omniauth_user, email: 'john@example.com', extern_uid: 'old-uid', provider: 'ldapmain')
-      expect { ldap_user_upper_case.save }.not_to change { User.count }
+      expect { ldap_user_upper_case.save }.not_to change { User.count } # rubocop:disable Rails/SaveBang
 
       existing_user.reload
       expect(existing_user.ldap_identity.extern_uid).to eql 'uid=john smith,ou=people,dc=example,dc=com'
@@ -89,7 +89,7 @@ RSpec.describe Gitlab::Auth::Ldap::User do
       existing_user = create(:omniauth_user, email: 'john@example.com', provider: 'twitter')
       expect(existing_user.identities.count).to be(1)
 
-      ldap_user.save
+      ldap_user.save # rubocop:disable Rails/SaveBang
       expect(ldap_user.gl_user.identities.count).to be(2)
 
       # Expect that find_by provider only returns a single instance of an identity and not an Enumerable
@@ -98,7 +98,7 @@ RSpec.describe Gitlab::Auth::Ldap::User do
     end
 
     it "creates a new user if not found" do
-      expect { ldap_user.save }.to change { User.count }.by(1)
+      expect { ldap_user.save }.to change { User.count }.by(1) # rubocop:disable Rails/SaveBang
     end
 
     context 'when signup is disabled' do
@@ -107,7 +107,7 @@ RSpec.describe Gitlab::Auth::Ldap::User do
       end
 
       it 'creates the user' do
-        ldap_user.save
+        ldap_user.save # rubocop:disable Rails/SaveBang
 
         expect(gl_user).to be_persisted
       end
@@ -119,7 +119,7 @@ RSpec.describe Gitlab::Auth::Ldap::User do
       end
 
       it 'creates and confirms the user anyway' do
-        ldap_user.save
+        ldap_user.save # rubocop:disable Rails/SaveBang
 
         expect(gl_user).to be_persisted
         expect(gl_user).to be_confirmed
@@ -132,7 +132,7 @@ RSpec.describe Gitlab::Auth::Ldap::User do
       end
 
       it 'creates the user' do
-        ldap_user.save
+        ldap_user.save # rubocop:disable Rails/SaveBang
 
         expect(gl_user).to be_persisted
       end
@@ -189,7 +189,7 @@ RSpec.describe Gitlab::Auth::Ldap::User do
         end
 
         it do
-          ldap_user.save
+          ldap_user.save # rubocop:disable Rails/SaveBang
           expect(gl_user).to be_valid
           expect(gl_user).not_to be_blocked
         end
@@ -201,7 +201,7 @@ RSpec.describe Gitlab::Auth::Ldap::User do
         end
 
         it do
-          ldap_user.save
+          ldap_user.save # rubocop:disable Rails/SaveBang
           expect(gl_user).to be_valid
           expect(gl_user).to be_blocked
         end
@@ -210,7 +210,7 @@ RSpec.describe Gitlab::Auth::Ldap::User do
 
     context 'sign-in' do
       before do
-        ldap_user.save
+        ldap_user.save # rubocop:disable Rails/SaveBang
         ldap_user.gl_user.activate
       end
 
@@ -220,7 +220,7 @@ RSpec.describe Gitlab::Auth::Ldap::User do
         end
 
         it do
-          ldap_user.save
+          ldap_user.save # rubocop:disable Rails/SaveBang
           expect(gl_user).to be_valid
           expect(gl_user).not_to be_blocked
         end
@@ -232,7 +232,7 @@ RSpec.describe Gitlab::Auth::Ldap::User do
         end
 
         it do
-          ldap_user.save
+          ldap_user.save # rubocop:disable Rails/SaveBang
           expect(gl_user).to be_valid
           expect(gl_user).not_to be_blocked
         end

spec/lib/gitlab/auth/saml/user_spec.rb

@@ -36,7 +36,7 @@ RSpec.describe Gitlab::Auth::Saml::User do
 
       context 'and should bind with SAML' do
         it 'adds the SAML identity to the existing user' do
-          saml_user.save
+          saml_user.save # rubocop:disable Rails/SaveBang
           expect(gl_user).to be_valid
           expect(gl_user).to eq existing_user
           identity = gl_user.identities.first
@@ -49,7 +49,7 @@ RSpec.describe Gitlab::Auth::Saml::User do
         context 'are defined' do
           it 'marks the user as external' do
             stub_saml_group_config(%w(Freelancers))
-            saml_user.save
+            saml_user.save # rubocop:disable Rails/SaveBang
             expect(gl_user).to be_valid
             expect(gl_user.external).to be_truthy
           end
@@ -61,7 +61,7 @@ RSpec.describe Gitlab::Auth::Saml::User do
 
         context 'are defined but the user does not belong there' do
           it 'does not mark the user as external' do
-            saml_user.save
+            saml_user.save # rubocop:disable Rails/SaveBang
             expect(gl_user).to be_valid
             expect(gl_user.external).to be_falsey
           end
@@ -70,7 +70,7 @@ RSpec.describe Gitlab::Auth::Saml::User do
         context 'user was external, now should not be' do
           it 'makes user internal' do
             existing_user.update_attribute('external', true)
-            saml_user.save
+            saml_user.save # rubocop:disable Rails/SaveBang
             expect(gl_user).to be_valid
             expect(gl_user.external).to be_falsey
           end
@@ -86,7 +86,7 @@ RSpec.describe Gitlab::Auth::Saml::User do
           end
 
           it 'creates a user from SAML' do
-            saml_user.save
+            saml_user.save # rubocop:disable Rails/SaveBang
 
             expect(gl_user).to be_valid
             identity = gl_user.identities.first
@@ -101,7 +101,7 @@ RSpec.describe Gitlab::Auth::Saml::User do
           end
 
           it 'does not throw an error' do
-            expect { saml_user.save }.not_to raise_error
+            expect { saml_user.save }.not_to raise_error # rubocop:disable Rails/SaveBang
           end
         end
 
@@ -111,7 +111,7 @@ RSpec.describe Gitlab::Auth::Saml::User do
           end
 
           it 'throws an error' do
-            expect { saml_user.save }.to raise_error StandardError
+            expect { saml_user.save }.to raise_error StandardError # rubocop:disable Rails/SaveBang
           end
         end
       end
@@ -120,7 +120,7 @@ RSpec.describe Gitlab::Auth::Saml::User do
         context 'are defined' do
           it 'marks the user as external' do
             stub_saml_group_config(%w(Freelancers))
-            saml_user.save
+            saml_user.save # rubocop:disable Rails/SaveBang
             expect(gl_user).to be_valid
             expect(gl_user.external).to be_truthy
           end
@@ -129,7 +129,7 @@ RSpec.describe Gitlab::Auth::Saml::User do
         context 'are defined but the user does not belong there' do
           it 'does not mark the user as external' do
             stub_saml_group_config(%w(Interns))
-            saml_user.save
+            saml_user.save # rubocop:disable Rails/SaveBang
             expect(gl_user).to be_valid
             expect(gl_user.external).to be_falsey
           end
@@ -170,7 +170,7 @@ RSpec.describe Gitlab::Auth::Saml::User do
 
             context 'and no account for the LDAP user' do
               it 'creates a user with dual LDAP and SAML identities' do
-                saml_user.save
+                saml_user.save # rubocop:disable Rails/SaveBang
 
                 expect(gl_user).to be_valid
                 expect(gl_user.username).to eql uid
@@ -230,7 +230,7 @@ RSpec.describe Gitlab::Auth::Saml::User do
                     { provider: id.provider, extern_uid: id.extern_uid }
                   end
 
-                  saml_user.save
+                  saml_user.save # rubocop:disable Rails/SaveBang
 
                   expect(gl_user).to be_valid
                   expect(gl_user.username).to eql 'john'
@@ -259,7 +259,7 @@ RSpec.describe Gitlab::Auth::Saml::User do
               end
 
               it 'adds the omniauth identity to the LDAP account' do
-                saml_user.save
+                saml_user.save # rubocop:disable Rails/SaveBang
 
                 expect(gl_user).to be_valid
                 expect(gl_user.username).to eql 'john'
@@ -271,9 +271,9 @@ RSpec.describe Gitlab::Auth::Saml::User do
               end
 
               it 'saves successfully on subsequent tries, when both identities are present' do
-                saml_user.save
+                saml_user.save # rubocop:disable Rails/SaveBang
                 local_saml_user = described_class.new(auth_hash)
-                local_saml_user.save
+                local_saml_user.save # rubocop:disable Rails/SaveBang
 
                 expect(local_saml_user.gl_user).to be_valid
                 expect(local_saml_user.gl_user).to be_persisted
@@ -289,7 +289,7 @@ RSpec.describe Gitlab::Auth::Saml::User do
                 local_hash = OmniAuth::AuthHash.new(uid: dn, provider: provider, info: info_hash)
                 local_saml_user = described_class.new(local_hash)
 
-                local_saml_user.save
+                local_saml_user.save # rubocop:disable Rails/SaveBang
                 local_gl_user = local_saml_user.gl_user
 
                 expect(local_gl_user).to be_valid
@@ -309,7 +309,7 @@ RSpec.describe Gitlab::Auth::Saml::User do
         end
 
         it 'creates the user' do
-          saml_user.save
+          saml_user.save # rubocop:disable Rails/SaveBang
 
           expect(gl_user).to be_persisted
         end
@@ -321,7 +321,7 @@ RSpec.describe Gitlab::Auth::Saml::User do
         end
 
         it 'creates and confirms the user anyway' do
-          saml_user.save
+          saml_user.save # rubocop:disable Rails/SaveBang
 
           expect(gl_user).to be_persisted
           expect(gl_user).to be_confirmed
@@ -334,7 +334,7 @@ RSpec.describe Gitlab::Auth::Saml::User do
         end
 
         it 'creates the user' do
-          saml_user.save
+          saml_user.save # rubocop:disable Rails/SaveBang
 
           expect(gl_user).to be_persisted
         end
@@ -353,7 +353,7 @@ RSpec.describe Gitlab::Auth::Saml::User do
           end
 
           it 'does not block the user' do
-            saml_user.save
+            saml_user.save # rubocop:disable Rails/SaveBang
             expect(gl_user).to be_valid
             expect(gl_user).not_to be_blocked
           end
@@ -365,7 +365,7 @@ RSpec.describe Gitlab::Auth::Saml::User do
           end
 
           it 'blocks user' do
-            saml_user.save
+            saml_user.save # rubocop:disable Rails/SaveBang
             expect(gl_user).to be_valid
             expect(gl_user).to be_blocked
           end
@@ -374,7 +374,7 @@ RSpec.describe Gitlab::Auth::Saml::User do
 
       context 'sign-in' do
         before do
-          saml_user.save
+          saml_user.save # rubocop:disable Rails/SaveBang
           saml_user.gl_user.activate
         end
 
@@ -384,7 +384,7 @@ RSpec.describe Gitlab::Auth::Saml::User do
           end
 
           it do
-            saml_user.save
+            saml_user.save # rubocop:disable Rails/SaveBang
             expect(gl_user).to be_valid
             expect(gl_user).not_to be_blocked
           end
@@ -396,7 +396,7 @@ RSpec.describe Gitlab::Auth::Saml::User do
           end
 
           it do
-            saml_user.save
+            saml_user.save # rubocop:disable Rails/SaveBang
             expect(gl_user).to be_valid
             expect(gl_user).not_to be_blocked
           end