Commit de123aba authored by Siddharth Asthana's avatar Siddharth Asthana

Fix Rails/SaveBang offenses

Changelog: other
EE: true
parent 98a3a026
...@@ -2,15 +2,6 @@ ...@@ -2,15 +2,6 @@
Rails/SaveBang: Rails/SaveBang:
Exclude: Exclude:
- ee/spec/lib/analytics/merge_request_metrics_calculator_spec.rb - 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/application_setting_spec.rb
- ee/spec/models/approval_merge_request_rule_spec.rb - ee/spec/models/approval_merge_request_rule_spec.rb
- ee/spec/models/approval_project_rule_spec.rb - ee/spec/models/approval_project_rule_spec.rb
......
...@@ -262,7 +262,7 @@ RSpec.describe Gitlab::Auth::Ldap::Access do ...@@ -262,7 +262,7 @@ RSpec.describe Gitlab::Auth::Ldap::Access do
it 'does not performs the membership update for existing users' do it 'does not performs the membership update for existing users' do
user.created_at = Time.now - 10.minutes user.created_at = Time.now - 10.minutes
user.last_credential_check_at = Time.now user.last_credential_check_at = Time.now
user.save user.save!
expect(LdapGroupLink).not_to receive(:where) expect(LdapGroupLink).not_to receive(:where)
expect(LdapGroupSyncWorker).not_to receive(:perform_async) expect(LdapGroupSyncWorker).not_to receive(:perform_async)
...@@ -321,7 +321,7 @@ RSpec.describe Gitlab::Auth::Ldap::Access do ...@@ -321,7 +321,7 @@ RSpec.describe Gitlab::Auth::Ldap::Access do
context 'user has at least one LDAPKey' do context 'user has at least one LDAPKey' do
before 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 end
it 'removes a SSH key if it is no longer in LDAP' do it 'removes a SSH key if it is no longer in LDAP' do
...@@ -356,7 +356,7 @@ RSpec.describe Gitlab::Auth::Ldap::Access do ...@@ -356,7 +356,7 @@ RSpec.describe Gitlab::Auth::Ldap::Access do
it 'updates existing Kerberos identity in GitLab if Active Directory has a different one' do 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') 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 { 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') expect(user.identities.where(provider: 'kerberos').last.extern_uid).to eq('otherlogin@BAR.COM')
...@@ -364,7 +364,7 @@ RSpec.describe Gitlab::Auth::Ldap::Access do ...@@ -364,7 +364,7 @@ RSpec.describe Gitlab::Auth::Ldap::Access do
it 'does not remove Kerberos identities from GitLab if they are none in the LDAP provider' do 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) 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 { 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') expect(user.identities.where(provider: 'kerberos').last.extern_uid).to eq('otherlogin@BAR.COM')
......
...@@ -27,7 +27,7 @@ RSpec.describe Gitlab::Auth::OAuth::User do ...@@ -27,7 +27,7 @@ RSpec.describe Gitlab::Auth::OAuth::User do
stub_ldap_person_find_by_uid(uid, ldap_entry) stub_ldap_person_find_by_uid(uid, ldap_entry)
oauth_user.save oauth_user.save # rubocop:disable Rails/SaveBang
end end
it 'links the LDAP person to the GitLab user' do it 'links the LDAP person to the GitLab user' do
......
...@@ -50,7 +50,7 @@ RSpec.describe Gitlab::Auth::Saml::User do ...@@ -50,7 +50,7 @@ RSpec.describe Gitlab::Auth::Saml::User do
%w(admin auditor).each do |group_type| %w(admin auditor).each do |group_type|
it "marks the user as #{group_type} when the user is in the configured group" do it "marks the user as #{group_type} when the user is in the configured group" do
stub_saml_group_config(group_type, %w(Developers)) 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).to be_valid
expect(gl_user.public_send(group_type)).to be_truthy expect(gl_user.public_send(group_type)).to be_truthy
...@@ -58,7 +58,7 @@ RSpec.describe Gitlab::Auth::Saml::User do ...@@ -58,7 +58,7 @@ RSpec.describe Gitlab::Auth::Saml::User do
it "does not mark the user as #{group_type} when the user is not in the configured group" do 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)) 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).to be_valid
expect(gl_user.public_send(group_type)).to be_falsey expect(gl_user.public_send(group_type)).to be_falsey
...@@ -67,7 +67,7 @@ RSpec.describe Gitlab::Auth::Saml::User do ...@@ -67,7 +67,7 @@ RSpec.describe Gitlab::Auth::Saml::User do
it "demotes from #{group_type} if not in the configured group" do 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) create(:user, email: 'john@mail.com', username: 'john').update_attribute(group_type, true)
stub_saml_group_config(group_type, %w(Admin)) 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).to be_valid
expect(gl_user.public_send(group_type)).to be_falsey expect(gl_user.public_send(group_type)).to be_falsey
...@@ -76,14 +76,14 @@ RSpec.describe Gitlab::Auth::Saml::User do ...@@ -76,14 +76,14 @@ RSpec.describe Gitlab::Auth::Saml::User do
it "does not demote from #{group_type} if not configured" do it "does not demote from #{group_type} if not configured" do
create(:user, email: 'john@mail.com', username: 'john').update_attribute(group_type, true) create(:user, email: 'john@mail.com', username: 'john').update_attribute(group_type, true)
stub_saml_group_config(group_type, []) 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).to be_valid
expect(gl_user.public_send(group_type)).to be_truthy expect(gl_user.public_send(group_type)).to be_truthy
end end
it "skips #{group_type} if not configured" do 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).to be_valid
expect(gl_user.public_send(group_type)).to be_falsey expect(gl_user.public_send(group_type)).to be_falsey
...@@ -96,7 +96,7 @@ RSpec.describe Gitlab::Auth::Saml::User do ...@@ -96,7 +96,7 @@ RSpec.describe Gitlab::Auth::Saml::User do
context 'required groups' do context 'required groups' do
context 'not defined' do context 'not defined' do
it 'lets anyone in' do it 'lets anyone in' do
saml_user.save saml_user.save # rubocop:disable Rails/SaveBang
expect(gl_user).to be_valid expect(gl_user).to be_valid
end end
...@@ -109,21 +109,21 @@ RSpec.describe Gitlab::Auth::Saml::User do ...@@ -109,21 +109,21 @@ RSpec.describe Gitlab::Auth::Saml::User do
it 'lets members in' do it 'lets members in' do
stub_saml_required_group_config(%w(Developers)) stub_saml_required_group_config(%w(Developers))
saml_user.save saml_user.save # rubocop:disable Rails/SaveBang
expect(gl_user).to be_valid expect(gl_user).to be_valid
end end
it 'unblocks already blocked members' do it 'unblocks already blocked members' do
stub_saml_required_group_config(%w(Developers)) 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 expect(saml_user.find_user).to be_active
end end
it 'does not unblock manually blocked members' do it 'does not unblock manually blocked members' do
stub_saml_required_group_config(%w(Developers)) 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 expect(saml_user.find_user).not_to be_active
end end
...@@ -131,14 +131,14 @@ RSpec.describe Gitlab::Auth::Saml::User do ...@@ -131,14 +131,14 @@ RSpec.describe Gitlab::Auth::Saml::User do
it 'does not allow non-members' do it 'does not allow non-members' do
stub_saml_required_group_config(%w(ArchitectureAstronauts)) 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 end
it 'blocks non-members' do it 'blocks non-members' do
orig_groups = auth_hash.extra.raw_info["groups"] orig_groups = auth_hash.extra.raw_info["groups"]
auth_hash.extra.raw_info.add("groups", "ArchitectureAstronauts") auth_hash.extra.raw_info.add("groups", "ArchitectureAstronauts")
stub_saml_required_group_config(%w(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) auth_hash.extra.raw_info.set("groups", orig_groups)
expect(saml_user.find_user).to be_ldap_blocked expect(saml_user.find_user).to be_ldap_blocked
......
...@@ -1126,7 +1126,7 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :clean_gitlab_redis_sha ...@@ -1126,7 +1126,7 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :clean_gitlab_redis_sha
let(:limit_project_ids) { [private_project2.id] } let(:limit_project_ids) { [private_project2.id] }
before do 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 end
context 'issues' do context 'issues' do
...@@ -1190,7 +1190,7 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :clean_gitlab_redis_sha ...@@ -1190,7 +1190,7 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :clean_gitlab_redis_sha
context 'when user is admin' do context 'when user is admin' do
context 'when admin mode enabled', :enable_admin_mode do context 'when admin mode enabled', :enable_admin_mode do
it 'returns right set of milestones' 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!(merge_requests_access_level: ProjectFeature::PRIVATE)
public_project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) public_project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE)
internal_project.project_feature.update!(issues_access_level: ProjectFeature::DISABLED) internal_project.project_feature.update!(issues_access_level: ProjectFeature::DISABLED)
......
...@@ -73,7 +73,7 @@ RSpec.describe Gitlab::Geo, :geo, :request_store do ...@@ -73,7 +73,7 @@ RSpec.describe Gitlab::Geo, :geo, :request_store do
end end
it 'returns false when primary does not exist' do it 'returns false when primary does not exist' do
primary_node.destroy primary_node.destroy!
expect(described_class.primary_node_configured?).to be_falsey expect(described_class.primary_node_configured?).to be_falsey
end end
......
...@@ -480,7 +480,7 @@ RSpec.describe Gitlab::GitAccess do ...@@ -480,7 +480,7 @@ RSpec.describe Gitlab::GitAccess do
project.add_role(user, role) project.add_role(user, role)
end end
protected_branch.save protected_branch.save!
aggregate_failures do aggregate_failures do
matrix.each do |action, allowed| matrix.each do |action, allowed|
...@@ -506,7 +506,7 @@ RSpec.describe Gitlab::GitAccess do ...@@ -506,7 +506,7 @@ RSpec.describe Gitlab::GitAccess do
create(:project_group_link, role, group: group, create(:project_group_link, role, group: group,
project: project) project: project)
protected_branch.save protected_branch.save!
aggregate_failures do aggregate_failures do
matrix.each do |action, allowed| matrix.each do |action, allowed|
...@@ -880,7 +880,7 @@ RSpec.describe Gitlab::GitAccess do ...@@ -880,7 +880,7 @@ RSpec.describe Gitlab::GitAccess do
let(:actor) { deploy_key } let(:actor) { deploy_key }
before do 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 end
it 'allows push and pull access' do it 'allows push and pull access' do
......
...@@ -9,7 +9,7 @@ RSpec.describe Gitlab::ImportExport::Group::RelationFactory do ...@@ -9,7 +9,7 @@ RSpec.describe Gitlab::ImportExport::Group::RelationFactory do
let(:importer_user) { admin } let(:importer_user) { admin }
let(:excluded_keys) { [] } let(:excluded_keys) { [] }
let(:created_object) do let(:created_object) do
described_class.create( described_class.create( # rubocop:disable Rails/SaveBang
relation_sym: relation_sym, relation_sym: relation_sym,
relation_hash: relation_hash, relation_hash: relation_hash,
relation_index: 1, relation_index: 1,
......
...@@ -28,7 +28,7 @@ RSpec.describe Gitlab::Mirror do ...@@ -28,7 +28,7 @@ RSpec.describe Gitlab::Mirror do
describe 'without jobs already running' do describe 'without jobs already running' do
before 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 end
it 'creates update_all_mirrors_worker' do it 'creates update_all_mirrors_worker' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment