Commit 3ff8dd5d authored by Vitali Tatarintev's avatar Vitali Tatarintev

Merge branch 'issue_220040_8' into 'master'

Fix Rails/SaveBang offenses for ee/spec/services - I I

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