Commit 11dd30f1 authored by Siddharth Asthana's avatar Siddharth Asthana

Fix Rails/SaveBang offenses

Changelog: other
EE: true
parent e5e3d9fd
......@@ -18,16 +18,6 @@ Rails/SaveBang:
- ee/spec/models/visible_approvable_spec.rb
- ee/spec/models/vulnerabilities/feedback_spec.rb
- ee/spec/models/vulnerabilities/issue_link_spec.rb
- ee/spec/services/ee/merge_requests/update_service_spec.rb
- ee/spec/services/ee/notes/quick_actions_service_spec.rb
- ee/spec/services/ee/notification_service_spec.rb
- ee/spec/services/epic_links/create_service_spec.rb
- ee/spec/services/epics/close_service_spec.rb
- ee/spec/services/epics/issue_promote_service_spec.rb
- ee/spec/services/epics/reopen_service_spec.rb
- ee/spec/services/epics/tree_reorder_service_spec.rb
- ee/spec/services/epics/update_dates_service_spec.rb
- ee/spec/services/epics/update_service_spec.rb
- ee/spec/services/geo/blob_verification_secondary_service_spec.rb
- ee/spec/services/geo/files_expire_service_spec.rb
- ee/spec/services/geo/metrics_update_service_spec.rb
......
......@@ -80,7 +80,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
let(:project) { create(:project, :repository, approvals_before_merge: project_value) }
it "does not update" do
merge_request.update(approvals_before_merge: mr_before_value)
merge_request.update!(approvals_before_merge: mr_before_value)
rule = create(:approval_merge_request_rule, merge_request: merge_request)
update_merge_request(approvals_before_merge: mr_after_value)
......@@ -95,7 +95,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
context 'when not approved' do
before do
merge_request.update(approvals_before_merge: 1)
merge_request.update!(approvals_before_merge: 1)
perform_enqueued_jobs do
update_merge_request(opts)
......@@ -109,8 +109,8 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
context 'when approved' do
before do
merge_request.update(approvals_before_merge: 1)
merge_request.approvals.create(user: user)
merge_request.update!(approvals_before_merge: 1)
merge_request.approvals.create!(user: user)
perform_enqueued_jobs do
update_merge_request(opts)
......@@ -193,8 +193,8 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
update_merge_request(approver_ids: "#{existing_approver.id},#{new_approver.id}")
end
merge_request.target_project.update(reset_approvals_on_push: true)
merge_request.approvals.create(user_id: existing_approver.id)
merge_request.target_project.update!(reset_approvals_on_push: true)
merge_request.approvals.create!(user_id: existing_approver.id)
end
it 'resets approvals when target_branch is changed' do
......
......@@ -72,6 +72,7 @@ RSpec.describe EE::NotificationService, :mailer do
# subscribers
@subscriber = create :user
@unsubscriber = create :user
@unsubscribed_mentioned = create(:user, username: 'unsubscribed_mentioned')
@subscribed_participant = create_global_setting_for(create(:user, username: 'subscribed_participant'), :participating)
@watcher_and_subscriber = create_global_setting_for(create(:user), :watch)
......@@ -97,12 +98,12 @@ RSpec.describe EE::NotificationService, :mailer do
project.add_maintainer(@subscribed_participant)
project.add_maintainer(@watcher_and_subscriber)
merge_request.subscriptions.create(user: @unsubscribed_mentioned, subscribed: false)
merge_request.subscriptions.create(user: @subscriber, subscribed: true)
merge_request.subscriptions.create(user: @subscribed_participant, subscribed: true)
merge_request.subscriptions.create(user: @unsubscriber, subscribed: false)
merge_request.subscriptions.create!(user: @unsubscribed_mentioned, subscribed: false)
merge_request.subscriptions.create!(user: @subscriber, subscribed: true)
merge_request.subscriptions.create!(user: @subscribed_participant, subscribed: true)
merge_request.subscriptions.create!(user: @unsubscriber, subscribed: false)
# Make the watcher a subscriber to detect dupes
merge_request.subscriptions.create(user: @watcher_and_subscriber, subscribed: true)
merge_request.subscriptions.create!(user: @watcher_and_subscriber, subscribed: true)
end
end
......@@ -621,7 +622,7 @@ RSpec.describe EE::NotificationService, :mailer do
end
it 'sends notification to watcher and a participator' do
epic.subscriptions.create(user: participating, subscribed: true)
epic.subscriptions.create!(user: participating, subscribed: true)
execute
......@@ -729,12 +730,12 @@ RSpec.describe EE::NotificationService, :mailer do
create(:group_member, group: group, user: @watcher_and_subscriber)
create(:group_member, group: group, user: @unsubscribed_mentioned)
issuable.subscriptions.create(user: @unsubscribed_mentioned, subscribed: false)
issuable.subscriptions.create(user: @subscriber, subscribed: true)
issuable.subscriptions.create(user: @subscribed_participant, subscribed: true)
issuable.subscriptions.create(user: @unsubscriber, subscribed: false)
issuable.subscriptions.create!(user: @unsubscribed_mentioned, subscribed: false)
issuable.subscriptions.create!(user: @subscriber, subscribed: true)
issuable.subscriptions.create!(user: @subscribed_participant, subscribed: true)
issuable.subscriptions.create!(user: @unsubscriber, subscribed: false)
# Make the watcher a subscriber to detect dupes
issuable.subscriptions.create(user: @watcher_and_subscriber, subscribed: true)
issuable.subscriptions.create!(user: @watcher_and_subscriber, subscribed: true)
end
context 'Merge Requests' do
......@@ -863,12 +864,12 @@ RSpec.describe EE::NotificationService, :mailer do
project.add_maintainer(user)
end
issuable.subscriptions.create(user: @unsubscribed_mentioned, project: project, subscribed: false)
issuable.subscriptions.create(user: @subscriber, project: project, subscribed: true)
issuable.subscriptions.create(user: @subscribed_participant, project: project, subscribed: true)
issuable.subscriptions.create(user: @unsubscriber, project: project, subscribed: false)
issuable.subscriptions.create!(user: @unsubscribed_mentioned, project: project, subscribed: false)
issuable.subscriptions.create!(user: @subscriber, project: project, subscribed: true)
issuable.subscriptions.create!(user: @subscribed_participant, project: project, subscribed: true)
issuable.subscriptions.create!(user: @unsubscriber, project: project, subscribed: false)
# Make the watcher a subscriber to detect dupes
issuable.subscriptions.create(user: @watcher_and_subscriber, project: project, subscribed: true)
issuable.subscriptions.create!(user: @watcher_and_subscriber, project: project, subscribed: true)
end
end
......
......@@ -112,7 +112,7 @@ RSpec.describe EpicLinks::CreateService do
let(:expected_code) { 409 }
before do
epic.update(parent: epic_to_add)
epic.update!(parent: epic_to_add)
end
include_examples 'returns an error'
......@@ -126,7 +126,7 @@ RSpec.describe EpicLinks::CreateService do
# epic_to_add -> epic1 -> epic2 -> epic
epic1 = create(:epic, group: group, parent: epic_to_add)
epic2 = create(:epic, group: group, parent: epic1)
epic.update(parent: epic2)
epic.update!(parent: epic2)
end
include_examples 'returns an error'
......@@ -135,7 +135,7 @@ RSpec.describe EpicLinks::CreateService do
context 'when adding an epic that is already a child of the parent epic' do
before do
epic_to_add.update(parent: epic)
epic_to_add.update!(parent: epic)
end
let(:expected_error) { "This epic cannot be added. It is already assigned to the parent epic." }
......@@ -195,8 +195,8 @@ RSpec.describe EpicLinks::CreateService do
let(:expected_code) { 409 }
before do
epic_to_add.update(parent: epic)
another_epic.update(parent: epic)
epic_to_add.update!(parent: epic)
another_epic.update!(parent: epic)
end
include_examples 'returns an error'
......@@ -235,7 +235,7 @@ RSpec.describe EpicLinks::CreateService do
context 'when given child epic is parent of the given parent' do
before do
epic.update(parent: epic_to_add)
epic.update!(parent: epic_to_add)
end
include_examples 'returns an error'
......@@ -348,7 +348,7 @@ RSpec.describe EpicLinks::CreateService do
let(:another_epic) { create(:epic, group: group) }
before do
epic_to_add.update(parent: epic)
epic_to_add.update!(parent: epic)
end
subject do
......@@ -389,7 +389,7 @@ RSpec.describe EpicLinks::CreateService do
let(:another_epic) { create(:epic, group: group) }
before do
epic_to_add.update(parent: another_epic)
epic_to_add.update!(parent: another_epic)
end
subject { add_epic([valid_reference]) }
......
......@@ -73,7 +73,7 @@ RSpec.describe Epics::CloseService do
context 'when trying to close a closed epic' do
before do
epic.update(state: :closed)
epic.update!(state: :closed)
end
it 'does not change the epic state' do
......
......@@ -233,7 +233,7 @@ RSpec.describe Epics::IssuePromoteService, :aggregate_failures do
context 'when issue was already promoted' do
it 'raises error' do
epic = create(:epic, group: group)
issue.update(promoted_to_epic_id: epic.id)
issue.update!(promoted_to_epic_id: epic.id)
expect { subject.execute(issue) }
.to raise_error(Epics::IssuePromoteService::PromoteError, /already promoted/)
......@@ -270,7 +270,7 @@ RSpec.describe Epics::IssuePromoteService, :aggregate_failures do
context 'on other issue types' do
shared_examples_for 'raising error' do
before do
issue.update(issue_type: issue_type)
issue.update!(issue_type: issue_type)
end
it 'raises error' do
......
......@@ -73,7 +73,7 @@ RSpec.describe Epics::ReopenService do
context 'when trying to reopen an opened epic' do
before do
epic.update(state: :opened)
epic.update!(state: :opened)
end
it 'does not change the epic state' do
......
......@@ -96,7 +96,7 @@ RSpec.describe Epics::TreeReorderService do
let(:new_parent_id) { GitlabSchema.id_from_object(epic) }
before do
tree_object_2.update(epic: epic1)
tree_object_2.update!(epic: epic1)
end
it 'updates the parent' do
......@@ -111,7 +111,7 @@ RSpec.describe Epics::TreeReorderService do
context 'when object being moved is from another epic' do
before do
other_epic = create(:epic, group: group)
epic_issue2.update(epic: other_epic)
epic_issue2.update!(epic: other_epic)
end
context 'when the new_parent_id has not been provided' do
......@@ -154,7 +154,7 @@ RSpec.describe Epics::TreeReorderService do
let(:new_parent_id) { GitlabSchema.id_from_object(epic) }
before do
epic_issue2.update(parent: other_epic)
epic_issue2.update!(parent: other_epic)
end
it_behaves_like 'error for the tree update', 'You don\'t have permissions to move the objects.'
......@@ -173,8 +173,8 @@ RSpec.describe Epics::TreeReorderService do
let(:another_epic) { create(:epic, group: another_group) }
before do
epic_issue1.update(epic: another_epic)
epic_issue2.update(epic: another_epic)
epic_issue1.update!(epic: another_epic)
epic_issue2.update!(epic: another_epic)
end
context 'when new_parent_id is not provided' do
......@@ -199,7 +199,7 @@ RSpec.describe Epics::TreeReorderService do
let(:new_parent_id) { GitlabSchema.id_from_object(epic) }
before do
epic_issue2.update(epic: epic1)
epic_issue2.update!(epic: epic1)
end
it 'updates the parent' do
......@@ -244,7 +244,7 @@ RSpec.describe Epics::TreeReorderService do
let(:new_parent_id) { GitlabSchema.id_from_object(epic) }
before do
epic2.update(parent: other_epic)
epic2.update!(parent: other_epic)
end
it_behaves_like 'error for the tree update', 'You don\'t have permissions to move the objects.'
......@@ -266,8 +266,8 @@ RSpec.describe Epics::TreeReorderService do
before do
other_group.add_developer(user)
epic.update(group: other_group)
epic2.update(parent: epic1)
epic.update!(group: other_group)
epic2.update!(parent: epic1)
end
it_behaves_like 'error for the tree update', "This epic cannot be added. An epic must belong to the same group or subgroup as its parent epic."
......@@ -288,8 +288,8 @@ RSpec.describe Epics::TreeReorderService do
let(:another_epic) { create(:epic, group: another_group) }
before do
epic1.update(group: another_group, parent: another_epic)
epic2.update(group: another_group, parent: another_epic)
epic1.update!(group: another_group, parent: another_epic)
epic2.update!(group: another_group, parent: another_epic)
end
it_behaves_like 'error for the tree update', 'You don\'t have permissions to move the objects.'
......
......@@ -137,7 +137,7 @@ RSpec.describe Epics::UpdateDatesService do
context 'single milestone' do
before do
epic_issue1 = create(:epic_issue, epic: epic)
epic_issue1.issue.update(milestone: milestone1, project: project)
epic_issue1.issue.update!(milestone: milestone1, project: project)
end
context 'complete start and due dates' do
......@@ -329,7 +329,7 @@ RSpec.describe Epics::UpdateDatesService do
let(:parent_epic) { create(:epic, group: group, parent: top_level_parent_epic) }
before do
epic.update(parent: parent_epic)
epic.update!(parent: parent_epic)
end
it "propagates date changes to parent epics" do
......
......@@ -277,7 +277,7 @@ RSpec.describe Epics::UpdateService do
context 'todos' do
before do
group.update(visibility: Gitlab::VisibilityLevel::PUBLIC)
group.update!(visibility: Gitlab::VisibilityLevel::PUBLIC)
end
context 'creating todos' do
......@@ -285,7 +285,7 @@ RSpec.describe Epics::UpdateService do
let(:mentioned2) { create(:user) }
before do
epic.update(description: "FYI: #{mentioned1.to_reference}")
epic.update!(description: "FYI: #{mentioned1.to_reference}")
end
it 'creates todos for only newly mentioned users' do
......@@ -348,12 +348,12 @@ RSpec.describe Epics::UpdateService do
before do
group.add_developer(mentioned1)
epic.update(description: "FYI: #{group.to_reference}")
epic.update!(description: "FYI: #{group.to_reference}")
end
context 'when the group is public' do
before do
group.update(visibility: Gitlab::VisibilityLevel::PUBLIC)
group.update!(visibility: Gitlab::VisibilityLevel::PUBLIC)
end
it 'creates todos for only newly mentioned users' do
......@@ -365,7 +365,7 @@ RSpec.describe Epics::UpdateService do
context 'when the group is private' do
before do
group.update(visibility: Gitlab::VisibilityLevel::PRIVATE)
group.update!(visibility: Gitlab::VisibilityLevel::PRIVATE)
end
it 'creates todos for only newly mentioned users that are group members' 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