Commit cfe7ead9 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Track state change events for Epics

Instead of creating system notes, we should use resource state events
just like we do with issues and merge requests
parent 2ff0ba5f
...@@ -19,3 +19,5 @@ class ResourceStateEvent < ResourceEvent ...@@ -19,3 +19,5 @@ class ResourceStateEvent < ResourceEvent
issue || merge_request issue || merge_request
end end
end end
ResourceStateEvent.prepend_if_ee('EE::ResourceStateEvent')
...@@ -13,14 +13,14 @@ module ResourceEvents ...@@ -13,14 +13,14 @@ module ResourceEvents
ResourceStateEvent.create( ResourceStateEvent.create(
user: user, user: user,
issue: issue, resource.class.underscore => resource,
merge_request: merge_request,
source_commit: commit_id_of(mentionable_source), source_commit: commit_id_of(mentionable_source),
source_merge_request_id: merge_request_id_of(mentionable_source), source_merge_request_id: merge_request_id_of(mentionable_source),
state: ResourceStateEvent.states[state], state: ResourceStateEvent.states[state],
close_after_error_tracking_resolve: close_after_error_tracking_resolve, close_after_error_tracking_resolve: close_after_error_tracking_resolve,
close_auto_resolve_prometheus_alert: close_auto_resolve_prometheus_alert, close_auto_resolve_prometheus_alert: close_auto_resolve_prometheus_alert,
created_at: Time.zone.now) created_at: Time.zone.now
)
resource.expire_note_etag_cache resource.expire_note_etag_cache
end end
...@@ -56,17 +56,5 @@ module ResourceEvents ...@@ -56,17 +56,5 @@ module ResourceEvents
mentionable_source.id mentionable_source.id
end end
def issue
return unless resource.is_a?(Issue)
resource
end
def merge_request
return unless resource.is_a?(MergeRequest)
resource
end
end end
end end
...@@ -12,6 +12,7 @@ module EE ...@@ -12,6 +12,7 @@ module EE
include Referable include Referable
include Awardable include Awardable
include LabelEventable include LabelEventable
include StateEventable
include UsageStatistics include UsageStatistics
include FromUnion include FromUnion
include EpicTreeSorting include EpicTreeSorting
......
# frozen_string_literal: true
module EE
module ResourceStateEvent
extend ActiveSupport::Concern
extend ::Gitlab::Utils::Override
prepended do
belongs_to :epic
end
class_methods do
def issuable_attrs
%i(epic).freeze + super
end
end
override :issuable
def issuable
epic || super
end
end
end
---
title: Track state changes using resource state events for Epics
merge_request: 42088
author:
type: fixed
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe ResourceStateEvent do
subject { build(:resource_state_event) }
it { is_expected.to belong_to(:epic) }
describe 'validations' do
describe 'Issuable validation' do
it 'is valid if only epic is set' do
subject.attributes = { epic: build_stubbed(:epic), issue: nil, merge_request: nil }
expect(subject).to be_valid
end
it 'is invalid if an epic and an issue is set' do
subject.attributes = { epic: build_stubbed(:epic), issue: build_stubbed(:issue), merge_request: nil }
expect(subject).not_to be_valid
end
end
end
end
...@@ -2,9 +2,9 @@ ...@@ -2,9 +2,9 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Epics::CloseService do RSpec.describe Epics::CloseService do
let(:group) { create(:group, :internal) } let_it_be(:group) { create(:group, :internal) }
let(:user) { create(:user) } let_it_be(:user) { create(:user) }
let(:epic) { create(:epic, group: group) } let_it_be(:epic, reload: true) { create(:epic, group: group) }
describe '#execute' do describe '#execute' do
subject { described_class.new(group, user) } subject { described_class.new(group, user) }
...@@ -25,7 +25,7 @@ RSpec.describe Epics::CloseService do ...@@ -25,7 +25,7 @@ RSpec.describe Epics::CloseService do
end end
context 'when a user has permissions to update the epic' do context 'when a user has permissions to update the epic' do
before do before_all do
group.add_maintainer(user) group.add_maintainer(user)
end end
...@@ -42,6 +42,25 @@ RSpec.describe Epics::CloseService do ...@@ -42,6 +42,25 @@ RSpec.describe Epics::CloseService do
expect { subject.execute(epic) }.to change { epic.closed_at } expect { subject.execute(epic) }.to change { epic.closed_at }
end end
context 'when state event tracking is enabled' do
before do
stub_feature_flags(track_resource_state_change_events: true)
end
it 'creates a resource state event' do
expect { subject.execute(epic) }.to change { epic.resource_state_events.count }.by(1)
event = epic.resource_state_events.last
expect(event.state).to eq('closed')
end
end
context 'when state event tracking is disabled' do
before do
stub_feature_flags(track_resource_state_change_events: false)
end
it 'creates a system note about epic close' do it 'creates a system note about epic close' do
expect { subject.execute(epic) }.to change { epic.notes.count }.by(1) expect { subject.execute(epic) }.to change { epic.notes.count }.by(1)
...@@ -50,6 +69,7 @@ RSpec.describe Epics::CloseService do ...@@ -50,6 +69,7 @@ RSpec.describe Epics::CloseService do
expect(note.note).to eq('closed') expect(note.note).to eq('closed')
expect(note.system_note_metadata.action).to eq('closed') expect(note.system_note_metadata.action).to eq('closed')
end end
end
it 'notifies the subscribers' do it 'notifies the subscribers' do
notification_service = double notification_service = double
...@@ -82,8 +102,8 @@ RSpec.describe Epics::CloseService do ...@@ -82,8 +102,8 @@ RSpec.describe Epics::CloseService do
expect { subject.execute(epic) }.not_to change { epic.closed_by } expect { subject.execute(epic) }.not_to change { epic.closed_by }
end end
it 'does not create a system note' do it 'does not create a resource state event' do
expect { subject.execute(epic) }.not_to change { epic.notes.count } expect { subject.execute(epic) }.not_to change { epic.resource_state_events.count }
end end
it 'does not send any emails' do it 'does not send any emails' do
......
...@@ -2,9 +2,9 @@ ...@@ -2,9 +2,9 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Epics::ReopenService do RSpec.describe Epics::ReopenService do
let(:group) { create(:group, :internal) } let_it_be(:group) { create(:group, :internal) }
let(:user) { create(:user) } let_it_be(:user) { create(:user) }
let(:epic) { create(:epic, group: group, state: :closed, closed_at: Date.today, closed_by: user) } let_it_be(:epic, reload: true) { create(:epic, group: group, state: :closed, closed_at: Date.today, closed_by: user) }
describe '#execute' do describe '#execute' do
subject { described_class.new(group, user) } subject { described_class.new(group, user) }
...@@ -25,7 +25,7 @@ RSpec.describe Epics::ReopenService do ...@@ -25,7 +25,7 @@ RSpec.describe Epics::ReopenService do
end end
context 'when a user has permissions to update the epic' do context 'when a user has permissions to update the epic' do
before do before_all do
group.add_maintainer(user) group.add_maintainer(user)
end end
...@@ -42,6 +42,25 @@ RSpec.describe Epics::ReopenService do ...@@ -42,6 +42,25 @@ RSpec.describe Epics::ReopenService do
expect { subject.execute(epic) }.to change { epic.closed_at }.to(nil) expect { subject.execute(epic) }.to change { epic.closed_at }.to(nil)
end end
context 'when state event tracking is enabled' do
before do
stub_feature_flags(track_resource_state_change_events: true)
end
it 'creates a resource state event' do
expect { subject.execute(epic) }.to change { epic.resource_state_events.count }.by(1)
event = epic.resource_state_events.last
expect(event.state).to eq('opened')
end
end
context 'when state event tracking is disabled' do
before do
stub_feature_flags(track_resource_state_change_events: false)
end
it 'creates a system note about epic reopen' do it 'creates a system note about epic reopen' do
expect { subject.execute(epic) }.to change { epic.notes.count }.by(1) expect { subject.execute(epic) }.to change { epic.notes.count }.by(1)
...@@ -50,6 +69,7 @@ RSpec.describe Epics::ReopenService do ...@@ -50,6 +69,7 @@ RSpec.describe Epics::ReopenService do
expect(note.note).to eq('opened') expect(note.note).to eq('opened')
expect(note.system_note_metadata.action).to eq('opened') expect(note.system_note_metadata.action).to eq('opened')
end end
end
it 'notifies the subscribers' do it 'notifies the subscribers' do
notification_service = double notification_service = double
...@@ -82,8 +102,8 @@ RSpec.describe Epics::ReopenService do ...@@ -82,8 +102,8 @@ RSpec.describe Epics::ReopenService do
expect { subject.execute(epic) }.not_to change { epic.closed_by } expect { subject.execute(epic) }.not_to change { epic.closed_by }
end end
it 'does not create a system note' do it 'does not create a resource state event' do
expect { subject.execute(epic) }.not_to change { epic.notes.count } expect { subject.execute(epic) }.not_to change { epic.resource_state_events.count }
end end
it 'does not send any emails' do it 'does not send any emails' do
......
...@@ -688,6 +688,7 @@ epic: ...@@ -688,6 +688,7 @@ epic:
- due_date_sourcing_epic - due_date_sourcing_epic
- events - events
- resource_label_events - resource_label_events
- resource_state_events
- user_mentions - user_mentions
- note_authors - note_authors
- boards_epic_user_preferences - boards_epic_user_preferences
......
...@@ -11,4 +11,32 @@ RSpec.describe ResourceStateEvent, type: :model do ...@@ -11,4 +11,32 @@ RSpec.describe ResourceStateEvent, type: :model do
it_behaves_like 'a resource event' it_behaves_like 'a resource event'
it_behaves_like 'a resource event for issues' it_behaves_like 'a resource event for issues'
it_behaves_like 'a resource event for merge requests' it_behaves_like 'a resource event for merge requests'
describe 'validations' do
describe 'Issuable validation' do
it 'is valid if an issue is set' do
subject.attributes = { issue: build_stubbed(:issue), merge_request: nil }
expect(subject).to be_valid
end
it 'is valid if a merge request is set' do
subject.attributes = { issue: nil, merge_request: build_stubbed(:merge_request) }
expect(subject).to be_valid
end
it 'is invalid if both issue and merge request are set' do
subject.attributes = { issue: build_stubbed(:issue), merge_request: build_stubbed(:merge_request) }
expect(subject).not_to be_valid
end
it 'is invalid if there is no issuable set' do
subject.attributes = { issue: nil, merge_request: nil }
expect(subject).not_to be_valid
end
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