Commit 8b3e261b authored by Igor Drozdov's avatar Igor Drozdov

Merge branch 'cleanup-track-iteration-change-events-ff' into 'master'

Cleanup track_iteration_change_events feature flag

See merge request gitlab-org/gitlab!43397
parents 5e373d14 59f49287
...@@ -6,14 +6,8 @@ info: To determine the technical writer assigned to the Stage/Group associated w ...@@ -6,14 +6,8 @@ info: To determine the technical writer assigned to the Stage/Group associated w
# Resource iteration events API **(STARTER)** # Resource iteration events API **(STARTER)**
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40850) in [GitLab Starter](https://about.gitlab.com/pricing/) 13.4 > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/229463) in [GitLab Starter](https://about.gitlab.com/pricing/) 13.4.
> - It's [deployed behind a feature flag](../user/feature_flags.md), enabled by default. > - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/229463) in [GitLab Starter](https://about.gitlab.com/pricing/) 13.5.
> - It's enabled on GitLab.com.
> - It's recommended for production use.
> - For GitLab self-managed instances, GitLab administrators can opt to [disable it](#enable-or-disable-iterations-events-tracking).
NOTE: **Note:**
This feature might not be available to you. Check the **version history** note above for details.
Resource iteration events keep track of what happens to GitLab [issues](../user/project/issues/). Resource iteration events keep track of what happens to GitLab [issues](../user/project/issues/).
...@@ -154,22 +148,3 @@ Example response: ...@@ -154,22 +148,3 @@ Example response:
"action": "remove" "action": "remove"
} }
``` ```
### Enable or disable iterations events tracking **(STARTER)**
Iterations events tracking is under development but ready for production use.
It is deployed behind a feature flag that is **enabled by default**.
[GitLab administrators with access to the GitLab Rails console](../administration/feature_flags.md)
can opt to disable it.
To enable it:
```ruby
Feature.enable(:track_iteration_change_events)
```
To disable it:
```ruby
Feature.disable(:track_iteration_change_events)
```
...@@ -36,11 +36,7 @@ module EE ...@@ -36,11 +36,7 @@ module EE
def handle_iteration_change def handle_iteration_change
return unless issuable.previous_changes.include?('sprint_id') return unless issuable.previous_changes.include?('sprint_id')
if iteration_changes_tracking_enabled? ::ResourceEvents::ChangeIterationService.new(issuable, current_user, old_iteration_id: issuable.sprint_id_before_last_save).execute
::EE::ResourceEvents::ChangeIterationService.new(issuable, current_user, old_iteration_id: issuable.sprint_id_before_last_save).execute
else
::SystemNoteService.change_iteration(issuable, current_user, issuable.iteration)
end
end end
def handle_weight_change def handle_weight_change
...@@ -54,10 +50,6 @@ module EE ...@@ -54,10 +50,6 @@ module EE
::SystemNoteService.change_health_status_note(issuable, issuable.project, current_user) ::SystemNoteService.change_health_status_note(issuable, issuable.project, current_user)
end end
def iteration_changes_tracking_enabled?
::Feature.enabled?(:track_iteration_change_events, issuable.project, default_enabled: true)
end
end end
end end
end end
# frozen_string_literal: true
module EE
module ResourceEvents
class ChangeIterationService < ::ResourceEvents::BaseChangeTimeboxService
attr_reader :iteration, :old_iteration_id
def initialize(resource, user, created_at: Time.current, old_iteration_id:)
super(resource, user, created_at: created_at)
@iteration = resource&.iteration
@old_iteration_id = old_iteration_id
end
private
def create_event
ResourceIterationEvent.create(build_resource_args)
end
def build_resource_args
action = iteration.blank? ? :remove : :add
super.merge({
action: ResourceTimeboxEvent.actions[action],
iteration_id: iteration.blank? ? old_iteration_id : iteration&.id
})
end
end
end
end
...@@ -36,10 +36,6 @@ module EE ...@@ -36,10 +36,6 @@ module EE
epics_service(epic, user).issue_epic_change(issue) epics_service(epic, user).issue_epic_change(issue)
end end
def change_iteration(noteable, author, iteration)
issuables_service(noteable, noteable.project, author).change_iteration(iteration)
end
# Called when the health_stauts of an Issue is changed # Called when the health_stauts of an Issue is changed
# #
# noteable - Noteable object # noteable - Noteable object
......
...@@ -31,23 +31,6 @@ module EE ...@@ -31,23 +31,6 @@ module EE
create_note(NoteSummary.new(noteable, project, author, body, action: 'published')) create_note(NoteSummary.new(noteable, project, author, body, action: 'published'))
end end
# Called when the iteration of a Noteable is changed
#
# iteration - Iteration being assigned, or nil
#
# Example Note text:
#
# "removed iteration"
#
# "changed iteration to 7.11"
#
# Returns the created Note object
def change_iteration(iteration)
body = iteration.nil? ? 'removed iteration' : "changed iteration to #{iteration.to_reference(project, format: :id)}"
create_note(NoteSummary.new(noteable, project, author, body, action: 'iteration'))
end
end end
end end
end end
# frozen_string_literal: true
module ResourceEvents
class ChangeIterationService < ::ResourceEvents::BaseChangeTimeboxService
attr_reader :iteration, :old_iteration_id
def initialize(resource, user, created_at: Time.current, old_iteration_id:)
super(resource, user, created_at: created_at)
@iteration = resource&.iteration
@old_iteration_id = old_iteration_id
end
private
def create_event
ResourceIterationEvent.create(build_resource_args)
end
def build_resource_args
action = iteration.blank? ? :remove : :add
super.merge({
action: ResourceTimeboxEvent.actions[action],
iteration_id: iteration.blank? ? old_iteration_id : iteration&.id
})
end
end
end
---
name: track_iteration_change_events
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/37620
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/229463
group: group::project management
type: development
default_enabled: true
...@@ -23,22 +23,6 @@ RSpec.describe Issuable::CommonSystemNotesService do ...@@ -23,22 +23,6 @@ RSpec.describe Issuable::CommonSystemNotesService do
expect(event.iteration.id).to eq iteration.id expect(event.iteration.id).to eq iteration.id
expect(event.user_id).to eq user.id expect(event.user_id).to eq user.id
end end
context 'when resource iteration event tracking is disabled' do
before do
stub_feature_flags(track_iteration_change_events: false)
end
it 'does not created a resource weight event' do
expect { subject }.not_to change { ResourceIterationEvent.count }
end
it 'does create a system note' do
expect { subject }.to change { Note.count }.from(0).to(1)
expect(Note.first.note).to eq("changed iteration to #{iteration.to_reference(issuable.resource_parent, format: :id)}")
end
end
end end
end end
......
...@@ -119,68 +119,30 @@ RSpec.describe Issues::UpdateService do ...@@ -119,68 +119,30 @@ RSpec.describe Issues::UpdateService do
group.add_maintainer(user) group.add_maintainer(user)
end end
context 'when track_iteration_change_events is disabled' do RSpec.shared_examples 'creates iteration resource event' do
before do it 'creates a system note' do
stub_feature_flags(track_iteration_change_events: false) expect do
end update_issue(iteration: iteration)
end.not_to change { Note.system.count }
RSpec.shared_examples 'creates iteration system note' do
it 'creates a system note' do
expect do
update_issue(iteration: iteration)
end.to change { Note.system.count }.by(1)
end
it 'does not create a iteration change event' do
expect do
update_issue(iteration: iteration)
end.not_to change { ResourceIterationEvent.count }
end
end
context 'group iterations' do
let(:iteration) { create(:iteration, group: group) }
it_behaves_like 'creates iteration system note'
end end
context 'project iterations' do it 'does not create a iteration change event' do
let(:iteration) { create(:iteration, :skip_project_validation, project: project) } expect do
update_issue(iteration: iteration)
it_behaves_like 'creates iteration system note' end.to change { ResourceIterationEvent.count }.by(1)
end end
end end
context 'when track_iteration_change_events is enabled' do context 'group iterations' do
before do let(:iteration) { create(:iteration, group: group) }
stub_feature_flags(track_iteration_change_events: true)
end
RSpec.shared_examples 'creates iteration resource event' do
it 'creates a system note' do
expect do
update_issue(iteration: iteration)
end.not_to change { Note.system.count }
end
it 'does not create a iteration change event' do
expect do
update_issue(iteration: iteration)
end.to change { ResourceIterationEvent.count }.by(1)
end
end
context 'group iterations' do
let(:iteration) { create(:iteration, group: group) }
it_behaves_like 'creates iteration resource event' it_behaves_like 'creates iteration resource event'
end end
context 'project iterations' do context 'project iterations' do
let(:iteration) { create(:iteration, :skip_project_validation, project: project) } let(:iteration) { create(:iteration, :skip_project_validation, project: project) }
it_behaves_like 'creates iteration resource event' it_behaves_like 'creates iteration resource event'
end
end end
end end
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe EE::ResourceEvents::ChangeIterationService do RSpec.describe ResourceEvents::ChangeIterationService do
let_it_be(:timebox) { create(:iteration) } let_it_be(:timebox) { create(:iteration) }
let(:created_at_time) { Time.utc(2019, 12, 30) } let(:created_at_time) { Time.utc(2019, 12, 30) }
......
...@@ -56,58 +56,4 @@ RSpec.describe ::SystemNotes::IssuablesService do ...@@ -56,58 +56,4 @@ RSpec.describe ::SystemNotes::IssuablesService do
expect(subject.note).to eq 'published this issue to the status page' expect(subject.note).to eq 'published this issue to the status page'
end end
end end
describe '#change_iteration' do
subject { service.change_iteration(iteration) }
context 'for a project iteration' do
let(:iteration) { create(:iteration, :skip_project_validation, project: project) }
it_behaves_like 'a system note' do
let(:action) { 'iteration' }
end
it_behaves_like 'a note with overridable created_at'
context 'when iteration added' do
it 'sets the note text' do
reference = iteration.to_reference(format: :id)
expect(subject.note).to eq "changed iteration to #{reference}"
end
end
context 'when iteration removed' do
let(:iteration) { nil }
it 'sets the note text' do
expect(subject.note).to eq 'removed iteration'
end
end
end
context 'for a group iteration' do
let(:iteration) { create(:iteration, group: group) }
it_behaves_like 'a system note' do
let(:action) { 'iteration' }
end
it_behaves_like 'a note with overridable created_at'
context 'when iteration added' do
it 'sets the note text to use the iteration id' do
expect(subject.note).to eq "changed iteration to #{iteration.to_reference(format: :id)}"
end
end
context 'when iteration removed' do
let(:iteration) { nil }
it 'sets the note text' do
expect(subject.note).to eq 'removed iteration'
end
end
end
end
end end
...@@ -161,14 +161,4 @@ RSpec.describe SystemNoteService do ...@@ -161,14 +161,4 @@ RSpec.describe SystemNoteService do
described_class.publish_issue_to_status_page(noteable, project, author) described_class.publish_issue_to_status_page(noteable, project, author)
end end
end end
describe '.change_iteration' do
it 'calls IssuablesService' do
expect_next_instance_of(::SystemNotes::IssuablesService) do |service|
expect(service).to receive(:change_iteration)
end
described_class.change_iteration(noteable, author, nil)
end
end
end end
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