diff --git a/.rubocop_todo/rails/save_bang.yml b/.rubocop_todo/rails/save_bang.yml index f83bb1fddef0194b2e8674b41f936025cda219a2..e3d97a45afdba373c5c6dab9cd8e54146381da14 100644 --- a/.rubocop_todo/rails/save_bang.yml +++ b/.rubocop_todo/rails/save_bang.yml @@ -2,15 +2,6 @@ Rails/SaveBang: Exclude: - ee/spec/lib/analytics/merge_request_metrics_calculator_spec.rb - - ee/spec/lib/gitlab/auth/ldap/access_spec.rb - - ee/spec/lib/gitlab/auth/o_auth/user_spec.rb - - ee/spec/lib/gitlab/auth/saml/user_spec.rb - - ee/spec/lib/gitlab/elastic/search_results_spec.rb - - ee/spec/lib/gitlab/email/handler/ee/service_desk_handler_spec.rb - - ee/spec/lib/gitlab/geo_spec.rb - - ee/spec/lib/gitlab/git_access_spec.rb - - ee/spec/lib/gitlab/import_export/group/relation_factory_spec.rb - - ee/spec/lib/gitlab/mirror_spec.rb - ee/spec/models/application_setting_spec.rb - ee/spec/models/approval_merge_request_rule_spec.rb - ee/spec/models/approval_project_rule_spec.rb diff --git a/ee/spec/lib/gitlab/auth/ldap/access_spec.rb b/ee/spec/lib/gitlab/auth/ldap/access_spec.rb index 6520e00aef934b747ac2bcff7259ec9a5b600a2c..feabdcf5ef1fc52ae77474b5e3fc9da14dedc902 100644 --- a/ee/spec/lib/gitlab/auth/ldap/access_spec.rb +++ b/ee/spec/lib/gitlab/auth/ldap/access_spec.rb @@ -262,7 +262,7 @@ it 'does not performs the membership update for existing users' do user.created_at = Time.now - 10.minutes user.last_credential_check_at = Time.now - user.save + user.save! expect(LdapGroupLink).not_to receive(:where) expect(LdapGroupSyncWorker).not_to receive(:perform_async) @@ -321,7 +321,7 @@ context 'user has at least one LDAPKey' do before do - user.keys.ldap.create key: ssh_key, title: 'to be removed' + user.keys.ldap.create! key: ssh_key, title: 'to be removed' end it 'removes a SSH key if it is no longer in LDAP' do @@ -356,7 +356,7 @@ it 'updates existing Kerberos identity in GitLab if Active Directory has a different one' do allow_any_instance_of(EE::Gitlab::Auth::Ldap::Person).to receive_messages(kerberos_principal: 'otherlogin@BAR.COM') - user.identities.build(provider: 'kerberos', extern_uid: 'mylogin@FOO.COM').save + user.identities.build(provider: 'kerberos', extern_uid: 'mylogin@FOO.COM').save! expect { access.update_user }.not_to change(user.identities.where(provider: 'kerberos'), :count) expect(user.identities.where(provider: 'kerberos').last.extern_uid).to eq('otherlogin@BAR.COM') @@ -364,7 +364,7 @@ it 'does not remove Kerberos identities from GitLab if they are none in the LDAP provider' do allow_any_instance_of(EE::Gitlab::Auth::Ldap::Person).to receive_messages(kerberos_principal: nil) - user.identities.build(provider: 'kerberos', extern_uid: 'otherlogin@BAR.COM').save + user.identities.build(provider: 'kerberos', extern_uid: 'otherlogin@BAR.COM').save! expect { access.update_user }.not_to change(user.identities.where(provider: 'kerberos'), :count) expect(user.identities.where(provider: 'kerberos').last.extern_uid).to eq('otherlogin@BAR.COM') diff --git a/ee/spec/lib/gitlab/auth/o_auth/user_spec.rb b/ee/spec/lib/gitlab/auth/o_auth/user_spec.rb index 9934436f9162aa7ba0ee3b9716a4700049d4e062..608eaeea9b4bb3da483919cc4c448bc85a556b7f 100644 --- a/ee/spec/lib/gitlab/auth/o_auth/user_spec.rb +++ b/ee/spec/lib/gitlab/auth/o_auth/user_spec.rb @@ -27,7 +27,7 @@ stub_ldap_person_find_by_uid(uid, ldap_entry) - oauth_user.save + oauth_user.save # rubocop:disable Rails/SaveBang end it 'links the LDAP person to the GitLab user' do diff --git a/ee/spec/lib/gitlab/auth/saml/user_spec.rb b/ee/spec/lib/gitlab/auth/saml/user_spec.rb index f46798ea4f3edd6b31f5be201c7cdcc22dc9346a..4fac2fc01beab6ef5c0c7750c462b5eec89a4a2b 100644 --- a/ee/spec/lib/gitlab/auth/saml/user_spec.rb +++ b/ee/spec/lib/gitlab/auth/saml/user_spec.rb @@ -50,7 +50,7 @@ def stub_saml_group_config(type, groups) %w(admin auditor).each do |group_type| it "marks the user as #{group_type} when the user is in the configured group" do stub_saml_group_config(group_type, %w(Developers)) - saml_user.save + saml_user.save # rubocop:disable Rails/SaveBang expect(gl_user).to be_valid expect(gl_user.public_send(group_type)).to be_truthy @@ -58,7 +58,7 @@ def stub_saml_group_config(type, groups) it "does not mark the user as #{group_type} when the user is not in the configured group" do stub_saml_group_config(group_type, %w(Admin)) - saml_user.save + saml_user.save # rubocop:disable Rails/SaveBang expect(gl_user).to be_valid expect(gl_user.public_send(group_type)).to be_falsey @@ -67,7 +67,7 @@ def stub_saml_group_config(type, groups) it "demotes from #{group_type} if not in the configured group" do create(:user, email: 'john@mail.com', username: 'john').update_attribute(group_type, true) stub_saml_group_config(group_type, %w(Admin)) - saml_user.save + saml_user.save # rubocop:disable Rails/SaveBang expect(gl_user).to be_valid expect(gl_user.public_send(group_type)).to be_falsey @@ -76,14 +76,14 @@ def stub_saml_group_config(type, groups) it "does not demote from #{group_type} if not configured" do create(:user, email: 'john@mail.com', username: 'john').update_attribute(group_type, true) stub_saml_group_config(group_type, []) - saml_user.save + saml_user.save # rubocop:disable Rails/SaveBang expect(gl_user).to be_valid expect(gl_user.public_send(group_type)).to be_truthy end it "skips #{group_type} if not configured" do - saml_user.save + saml_user.save # rubocop:disable Rails/SaveBang expect(gl_user).to be_valid expect(gl_user.public_send(group_type)).to be_falsey @@ -96,7 +96,7 @@ def stub_saml_group_config(type, groups) context 'required groups' do context 'not defined' do it 'lets anyone in' do - saml_user.save + saml_user.save # rubocop:disable Rails/SaveBang expect(gl_user).to be_valid end @@ -109,21 +109,21 @@ def stub_saml_group_config(type, groups) it 'lets members in' do stub_saml_required_group_config(%w(Developers)) - saml_user.save + saml_user.save # rubocop:disable Rails/SaveBang expect(gl_user).to be_valid end it 'unblocks already blocked members' do stub_saml_required_group_config(%w(Developers)) - saml_user.save.ldap_block + saml_user.save.ldap_block # rubocop:disable Rails/SaveBang expect(saml_user.find_user).to be_active end it 'does not unblock manually blocked members' do stub_saml_required_group_config(%w(Developers)) - saml_user.save.block! + saml_user.save.block! # rubocop:disable Rails/SaveBang expect(saml_user.find_user).not_to be_active end @@ -131,14 +131,14 @@ def stub_saml_group_config(type, groups) it 'does not allow non-members' do stub_saml_required_group_config(%w(ArchitectureAstronauts)) - expect { saml_user.save }.to raise_error Gitlab::Auth::OAuth::User::SignupDisabledError + expect { saml_user.save }.to raise_error Gitlab::Auth::OAuth::User::SignupDisabledError # rubocop:disable Rails/SaveBang end it 'blocks non-members' do orig_groups = auth_hash.extra.raw_info["groups"] auth_hash.extra.raw_info.add("groups", "ArchitectureAstronauts") stub_saml_required_group_config(%w(ArchitectureAstronauts)) - saml_user.save + saml_user.save # rubocop:disable Rails/SaveBang auth_hash.extra.raw_info.set("groups", orig_groups) expect(saml_user.find_user).to be_ldap_blocked diff --git a/ee/spec/lib/gitlab/elastic/search_results_spec.rb b/ee/spec/lib/gitlab/elastic/search_results_spec.rb index 24da1531848547d8244d7c830adb96cf3163abbe..f5813c8b78ec7612aa02ed51cceb80af21d4f722 100644 --- a/ee/spec/lib/gitlab/elastic/search_results_spec.rb +++ b/ee/spec/lib/gitlab/elastic/search_results_spec.rb @@ -1126,7 +1126,7 @@ def self.ruby_method_123(ruby_another_method_arg) let(:limit_project_ids) { [private_project2.id] } before do - private_project2.project_members.create(user: user, access_level: ProjectMember::DEVELOPER) + private_project2.project_members.create!(user: user, access_level: ProjectMember::DEVELOPER) end context 'issues' do @@ -1190,7 +1190,7 @@ def self.ruby_method_123(ruby_another_method_arg) context 'when user is admin' do context 'when admin mode enabled', :enable_admin_mode do it 'returns right set of milestones' do - user.update(admin: true) + user.update!(admin: true) public_project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) public_project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) internal_project.project_feature.update!(issues_access_level: ProjectFeature::DISABLED) diff --git a/ee/spec/lib/gitlab/geo_spec.rb b/ee/spec/lib/gitlab/geo_spec.rb index 82c0094e946a63c4b1f016e8bbe682013035ef9a..90a82aef93878c2c917cebe2cd80ed417385de60 100644 --- a/ee/spec/lib/gitlab/geo_spec.rb +++ b/ee/spec/lib/gitlab/geo_spec.rb @@ -73,7 +73,7 @@ end it 'returns false when primary does not exist' do - primary_node.destroy + primary_node.destroy! expect(described_class.primary_node_configured?).to be_falsey end diff --git a/ee/spec/lib/gitlab/git_access_spec.rb b/ee/spec/lib/gitlab/git_access_spec.rb index 3b905c09a5742e3d0494927e318b47bf3c803738..00a0ff1fea88de33b14a15d38f4e84353c1775ea 100644 --- a/ee/spec/lib/gitlab/git_access_spec.rb +++ b/ee/spec/lib/gitlab/git_access_spec.rb @@ -480,7 +480,7 @@ def self.run_permission_checks(permissions_matrix) project.add_role(user, role) end - protected_branch.save + protected_branch.save! aggregate_failures do matrix.each do |action, allowed| @@ -506,7 +506,7 @@ def self.run_group_permission_checks(permissions_matrix) create(:project_group_link, role, group: group, project: project) - protected_branch.save + protected_branch.save! aggregate_failures do matrix.each do |action, allowed| @@ -880,7 +880,7 @@ def stub_redis let(:actor) { deploy_key } before do - deploy_key.deploy_keys_projects.create(project: project, can_push: true) + deploy_key.deploy_keys_projects.create!(project: project, can_push: true) end it 'allows push and pull access' do diff --git a/ee/spec/lib/gitlab/import_export/group/relation_factory_spec.rb b/ee/spec/lib/gitlab/import_export/group/relation_factory_spec.rb index 0f7a31f33ce47d5ea7b55981c09e67ccd6b5cd92..5dc4a56957ccde249b4a3e5c8a29403ac09134fd 100644 --- a/ee/spec/lib/gitlab/import_export/group/relation_factory_spec.rb +++ b/ee/spec/lib/gitlab/import_export/group/relation_factory_spec.rb @@ -9,7 +9,7 @@ let(:importer_user) { admin } let(:excluded_keys) { [] } let(:created_object) do - described_class.create( + described_class.create( # rubocop:disable Rails/SaveBang relation_sym: relation_sym, relation_hash: relation_hash, relation_index: 1, diff --git a/ee/spec/lib/gitlab/mirror_spec.rb b/ee/spec/lib/gitlab/mirror_spec.rb index 810358dc4f0fd3c41240b487832f69f357f79135..a9f807add39964e69bc60311a50f86a87238f2da 100644 --- a/ee/spec/lib/gitlab/mirror_spec.rb +++ b/ee/spec/lib/gitlab/mirror_spec.rb @@ -28,7 +28,7 @@ describe 'without jobs already running' do before do - Sidekiq::Cron::Job.find("update_all_mirrors_worker")&.destroy + Sidekiq::Cron::Job.find("update_all_mirrors_worker")&.destroy # rubocop:disable Rails/SaveBang end it 'creates update_all_mirrors_worker' do