From bf8c20729baffbf55605484230ff33680d72139b Mon Sep 17 00:00:00 2001 From: Oswaldo Ferreira Date: Thu, 7 Dec 2017 15:41:30 -0200 Subject: [PATCH] Cache merged and closed events data in merge_request_metrics table --- .../components/states/mr_widget_closed.js | 6 +- .../components/states/mr_widget_merged.js | 6 +- .../stores/mr_widget_store.js | 42 +++--- app/models/concerns/issuable.rb | 11 +- app/models/issue.rb | 5 + app/models/merge_request/metrics.rb | 10 +- app/serializers/event_entity.rb | 4 - .../merge_request_metrics_entity.rb | 6 + .../merge_request_widget_entity.rb | 32 ++++- app/services/event_create_service.rb | 2 +- app/services/merge_request_metrics_service.rb | 19 +++ app/services/merge_requests/base_service.rb | 4 + app/services/merge_requests/close_service.rb | 13 +- .../merge_requests/post_merge_service.rb | 11 +- app/services/merge_requests/reopen_service.rb | 13 +- ...osw-introduce-merge-request-statistics.yml | 5 + ...elated_columns_to_merge_request_metrics.rb | 37 +++++ ..._merge_request_metrics_with_events_data.rb | 68 +++++++++ db/schema.rb | 5 + ..._merge_request_metrics_with_events_data.rb | 135 ++++++++++++++++++ .../entities/merge_request_metrics.json | 21 +++ .../entities/merge_request_widget.json | 6 +- spec/fixtures/api/schemas/entities/user.json | 17 +++ .../states/mr_widget_closed_spec.js | 13 +- .../states/mr_widget_merged_spec.js | 11 +- spec/javascripts/vue_mr_widget/mock_data.js | 9 +- ...e_request_metrics_with_events_data_spec.rb | 124 ++++++++++++++++ ...e_request_metrics_with_events_data_spec.rb | 24 ++++ spec/models/issue_spec.rb | 26 ++++ spec/models/merge_request/metrics_spec.rb | 15 +- spec/models/merge_request_spec.rb | 19 +++ spec/requests/api/notes_spec.rb | 2 +- spec/requests/api/project_milestones_spec.rb | 2 +- spec/requests/api/v3/milestones_spec.rb | 2 +- spec/requests/api/v3/notes_spec.rb | 2 +- spec/serializers/event_entity_spec.rb | 13 -- .../merge_request_widget_entity_spec.rb | 74 ++++++++++ .../create_deployment_service_spec.rb | 2 +- .../merge_requests/close_service_spec.rb | 13 ++ .../merge_requests/post_merge_service_spec.rb | 13 ++ .../merge_requests/reopen_service_spec.rb | 13 ++ ...date_merge_request_metrics_service_spec.rb | 42 ++++++ 42 files changed, 799 insertions(+), 98 deletions(-) delete mode 100644 app/serializers/event_entity.rb create mode 100644 app/serializers/merge_request_metrics_entity.rb create mode 100644 app/services/merge_request_metrics_service.rb create mode 100644 changelogs/unreleased/osw-introduce-merge-request-statistics.yml create mode 100644 db/migrate/20171127151038_add_events_related_columns_to_merge_request_metrics.rb create mode 100644 db/post_migrate/20171128214150_schedule_populate_merge_request_metrics_with_events_data.rb create mode 100644 lib/gitlab/background_migration/populate_merge_request_metrics_with_events_data.rb create mode 100644 spec/fixtures/api/schemas/entities/merge_request_metrics.json create mode 100644 spec/fixtures/api/schemas/entities/user.json create mode 100644 spec/lib/gitlab/background_migration/populate_merge_request_metrics_with_events_data_spec.rb create mode 100644 spec/migrations/schedule_populate_merge_request_metrics_with_events_data_spec.rb delete mode 100644 spec/serializers/event_entity_spec.rb create mode 100644 spec/services/update_merge_request_metrics_service_spec.rb diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_closed.js b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_closed.js index b25cc3443ef..dd8b2665b1d 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_closed.js +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_closed.js @@ -16,9 +16,9 @@ export default {

diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.js b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.js index 2dfd87ed904..2a421192568 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.js +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.js @@ -68,9 +68,9 @@ export default {

+ :author="mr.metrics.mergedBy" + :date-title="mr.metrics.mergedAt" + :date-readable="mr.metrics.readableMergedAt" /> { it('should have correct elements', () => { expect(el.querySelector('h4').textContent).toContain('Closed by'); - expect(el.querySelector('h4').textContent).toContain(mr.closedEvent.author.name); + expect(el.querySelector('h4').textContent).toContain(mr.metrics.closedBy.name); expect(el.textContent).toContain('The changes were not merged into'); expect(el.querySelector('.label-branch').getAttribute('href')).toEqual(mr.targetBranchPath); expect(el.querySelector('.label-branch').textContent).toContain(mr.targetBranch); diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_merged_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_merged_spec.js index 2714e8294fa..90972469630 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_merged_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_merged_spec.js @@ -14,10 +14,13 @@ const createComponent = () => { canRevertInCurrentMR: true, canRemoveSourceBranch: true, sourceBranchRemoved: true, - mergedEvent: { - author: {}, - updatedAt: 'mergedUpdatedAt', - formattedUpdatedAt: '', + metrics: { + mergedBy: {}, + mergedAt: 'mergedUpdatedAt', + readableMergedAt: '', + closedBy: {}, + closedAt: 'mergedUpdatedAt', + readableClosedAt: '', }, updatedAt: 'mrUpdatedAt', targetBranch, diff --git a/spec/javascripts/vue_mr_widget/mock_data.js b/spec/javascripts/vue_mr_widget/mock_data.js index 1ad7c2d8efa..ca29c9fee32 100644 --- a/spec/javascripts/vue_mr_widget/mock_data.js +++ b/spec/javascripts/vue_mr_widget/mock_data.js @@ -33,8 +33,8 @@ export default { "source_project_id": 19, "target_branch": "master", "target_project_id": 19, - "merge_event": { - "author": { + "metrics": { + "merged_by": { "name": "Administrator", "username": "root", "id": 1, @@ -42,9 +42,10 @@ export default { "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", "web_url": "http://localhost:3000/root" }, - "updated_at": "2017-04-07T15:39:25.696Z" + "merged_at": "2017-04-07T15:39:25.696Z", + "closed_by": null, + "closed_at": null }, - "closed_event": null, "author": { "name": "Administrator", "username": "root", diff --git a/spec/lib/gitlab/background_migration/populate_merge_request_metrics_with_events_data_spec.rb b/spec/lib/gitlab/background_migration/populate_merge_request_metrics_with_events_data_spec.rb new file mode 100644 index 00000000000..dfe3b31f1c0 --- /dev/null +++ b/spec/lib/gitlab/background_migration/populate_merge_request_metrics_with_events_data_spec.rb @@ -0,0 +1,124 @@ +require 'rails_helper' + +describe Gitlab::BackgroundMigration::PopulateMergeRequestMetricsWithEventsData, :migration, schema: 20171128214150 do + describe '#perform' do + let(:mr_with_event) { create(:merge_request) } + let!(:merged_event) { create(:event, :merged, target: mr_with_event) } + let!(:closed_event) { create(:event, :closed, target: mr_with_event) } + + before do + # Make sure no metrics are created and kept through after_* callbacks. + mr_with_event.metrics.destroy! + end + + it 'inserts metrics and updates closed and merged events' do + subject.perform(mr_with_event.id, mr_with_event.id) + + mr_with_event.reload + + expect(mr_with_event.metrics).to have_attributes(latest_closed_by_id: closed_event.author_id, + merged_by_id: merged_event.author_id) + expect(mr_with_event.metrics.latest_closed_at.to_s).to eq(closed_event.updated_at.to_s) + end + end + + describe '#insert_metrics_for_range' do + let!(:mrs_without_metrics) { create_list(:merge_request, 3) } + let!(:mrs_with_metrics) { create_list(:merge_request, 2) } + + before do + # Make sure no metrics are created and kept through after_* callbacks. + mrs_without_metrics.each { |m| m.metrics.destroy! } + end + + it 'inserts merge_request_metrics for merge_requests without one' do + expect { subject.insert_metrics_for_range(MergeRequest.first.id, MergeRequest.last.id) } + .to change(MergeRequest::Metrics, :count).from(2).to(5) + + mrs_without_metrics.each do |mr_without_metrics| + expect(mr_without_metrics.reload.metrics).to be_present + end + end + + it 'does not inserts merge_request_metrics for MRs out of given range' do + expect { subject.insert_metrics_for_range(mrs_with_metrics.first.id, mrs_with_metrics.last.id) } + .not_to change(MergeRequest::Metrics, :count).from(2) + end + end + + describe '#update_metrics_with_events_data' do + context 'closed events data update' do + let(:users) { create_list(:user, 3) } + let(:mrs_with_event) { create_list(:merge_request, 3) } + + before do + create_list(:event, 2, :closed, author: users.first, target: mrs_with_event.first) + create_list(:event, 3, :closed, author: users.second, target: mrs_with_event.second) + create(:event, :closed, author: users.third, target: mrs_with_event.third) + end + + it 'migrates multiple MR metrics with closed event data' do + mr_without_event = create(:merge_request) + create(:event, :merged) + + subject.update_metrics_with_events_data(mrs_with_event.first.id, mrs_with_event.last.id) + + mrs_with_event.each do |mr_with_event| + latest_event = Event.where(action: 3, target: mr_with_event).last + + mr_with_event.metrics.reload + + expect(mr_with_event.metrics.latest_closed_by).to eq(latest_event.author) + expect(mr_with_event.metrics.latest_closed_at.to_s).to eq(latest_event.updated_at.to_s) + end + + expect(mr_without_event.metrics.reload).to have_attributes(latest_closed_by_id: nil, + latest_closed_at: nil) + end + + it 'does not updates metrics out of given range' do + out_of_range_mr = create(:merge_request) + create(:event, :closed, author: users.last, target: out_of_range_mr) + + expect { subject.perform(mrs_with_event.first.id, mrs_with_event.second.id) } + .not_to change { out_of_range_mr.metrics.reload.merged_by } + .from(nil) + end + end + + context 'merged events data update' do + let(:users) { create_list(:user, 3) } + let(:mrs_with_event) { create_list(:merge_request, 3) } + + before do + create_list(:event, 2, :merged, author: users.first, target: mrs_with_event.first) + create_list(:event, 3, :merged, author: users.second, target: mrs_with_event.second) + create(:event, :merged, author: users.third, target: mrs_with_event.third) + end + + it 'migrates multiple MR metrics with merged event data' do + mr_without_event = create(:merge_request) + create(:event, :merged) + + subject.update_metrics_with_events_data(mrs_with_event.first.id, mrs_with_event.last.id) + + mrs_with_event.each do |mr_with_event| + latest_event = Event.where(action: Event::MERGED, target: mr_with_event).last + + expect(mr_with_event.metrics.reload.merged_by).to eq(latest_event.author) + end + + expect(mr_without_event.metrics.reload).to have_attributes(merged_by_id: nil) + end + + it 'does not updates metrics out of given range' do + out_of_range_mr = create(:merge_request) + create(:event, :merged, author: users.last, target: out_of_range_mr) + + expect { subject.perform(mrs_with_event.first.id, mrs_with_event.second.id) } + .not_to change { out_of_range_mr.metrics.reload.merged_by } + .from(nil) + end + end + end +end diff --git a/spec/migrations/schedule_populate_merge_request_metrics_with_events_data_spec.rb b/spec/migrations/schedule_populate_merge_request_metrics_with_events_data_spec.rb new file mode 100644 index 00000000000..97e089c5cb8 --- /dev/null +++ b/spec/migrations/schedule_populate_merge_request_metrics_with_events_data_spec.rb @@ -0,0 +1,24 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20171128214150_schedule_populate_merge_request_metrics_with_events_data.rb') + +describe SchedulePopulateMergeRequestMetricsWithEventsData, :migration, :sidekiq do + let!(:mrs) { create_list(:merge_request, 3) } + + it 'correctly schedules background migrations' do + stub_const("#{described_class.name}::BATCH_SIZE", 2) + + Sidekiq::Testing.fake! do + Timecop.freeze do + migrate! + + expect(described_class::MIGRATION) + .to be_scheduled_migration(10.minutes, mrs.first.id, mrs.second.id) + + expect(described_class::MIGRATION) + .to be_scheduled_migration(20.minutes, mrs.third.id, mrs.third.id) + + expect(BackgroundMigrationWorker.jobs.size).to eq(2) + end + end + end +end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index c9deeb45c1a..5ced000cdb6 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -23,6 +23,32 @@ describe Issue do it { is_expected.to have_db_index(:deleted_at) } end + describe 'callbacks' do + describe '#ensure_metrics' do + it 'creates metrics after saving' do + issue = create(:issue) + + expect(issue.metrics).to be_persisted + expect(Issue::Metrics.count).to eq(1) + end + + it 'does not create duplicate metrics for an issue' do + issue = create(:issue) + + issue.close! + + expect(issue.metrics).to be_persisted + expect(Issue::Metrics.count).to eq(1) + end + + it 'records current metrics' do + expect_any_instance_of(Issue::Metrics).to receive(:record!) + + create(:issue) + end + end + end + describe '#order_by_position_and_priority' do let(:project) { create :project } let(:p1) { create(:label, title: 'P1', project: project, priority: 1) } diff --git a/spec/models/merge_request/metrics_spec.rb b/spec/models/merge_request/metrics_spec.rb index 9353d5c3c8a..02ff7839739 100644 --- a/spec/models/merge_request/metrics_spec.rb +++ b/spec/models/merge_request/metrics_spec.rb @@ -1,16 +1,11 @@ require 'spec_helper' describe MergeRequest::Metrics do - subject { create(:merge_request) } + subject { described_class.new } - describe "when recording the default set of metrics on merge request save" do - it "records the merge time" do - time = Time.now - Timecop.freeze(time) { subject.mark_as_merged } - metrics = subject.metrics - - expect(metrics).to be_present - expect(metrics.merged_at).to be_like_time(time) - end + describe 'associations' do + it { is_expected.to belong_to(:merge_request) } + it { is_expected.to belong_to(:latest_closed_by).class_name('User') } + it { is_expected.to belong_to(:merged_by).class_name('User') } end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index df94617a19e..d8ebd46faab 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -65,6 +65,25 @@ describe MergeRequest do end end + describe 'callbacks' do + describe '#ensure_merge_request_metrics' do + it 'creates metrics after saving' do + merge_request = create(:merge_request) + + expect(merge_request.metrics).to be_persisted + expect(MergeRequest::Metrics.count).to eq(1) + end + + it 'does not duplicate metrics for a merge request' do + merge_request = create(:merge_request) + + merge_request.mark_as_merged! + + expect(MergeRequest::Metrics.count).to eq(1) + end + end + end + describe 'respond to' do it { is_expected.to respond_to(:unchecked?) } it { is_expected.to respond_to(:can_be_merged?) } diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index be8d9c19125..981c9c27325 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -464,7 +464,7 @@ describe API::Notes do describe "POST /projects/:id/noteable/:noteable_id/notes to test observer on create" do it "creates an activity event when an issue note is created" do - expect(Event).to receive(:create) + expect(Event).to receive(:create!) post api("/projects/#{project.id}/issues/#{issue.iid}/notes", user), body: 'hi!' end diff --git a/spec/requests/api/project_milestones_spec.rb b/spec/requests/api/project_milestones_spec.rb index 6fe8ab5a3f6..08ea7314bb3 100644 --- a/spec/requests/api/project_milestones_spec.rb +++ b/spec/requests/api/project_milestones_spec.rb @@ -16,7 +16,7 @@ describe API::ProjectMilestones do describe 'PUT /projects/:id/milestones/:milestone_id to test observer on close' do it 'creates an activity event when an milestone is closed' do - expect(Event).to receive(:create) + expect(Event).to receive(:create!) put api("/projects/#{project.id}/milestones/#{milestone.id}", user), state_event: 'close' diff --git a/spec/requests/api/v3/milestones_spec.rb b/spec/requests/api/v3/milestones_spec.rb index 9ee71ea9c5d..6021600e09c 100644 --- a/spec/requests/api/v3/milestones_spec.rb +++ b/spec/requests/api/v3/milestones_spec.rb @@ -161,7 +161,7 @@ describe API::V3::Milestones do describe 'PUT /projects/:id/milestones/:milestone_id to test observer on close' do it 'creates an activity event when an milestone is closed' do - expect(Event).to receive(:create) + expect(Event).to receive(:create!) put v3_api("/projects/#{project.id}/milestones/#{milestone.id}", user), state_event: 'close' diff --git a/spec/requests/api/v3/notes_spec.rb b/spec/requests/api/v3/notes_spec.rb index 6428d9daaba..5532795ab02 100644 --- a/spec/requests/api/v3/notes_spec.rb +++ b/spec/requests/api/v3/notes_spec.rb @@ -302,7 +302,7 @@ describe API::V3::Notes do describe "POST /projects/:id/noteable/:noteable_id/notes to test observer on create" do it "creates an activity event when an issue note is created" do - expect(Event).to receive(:create) + expect(Event).to receive(:create!) post v3_api("/projects/#{project.id}/issues/#{issue.id}/notes", user), body: 'hi!' end diff --git a/spec/serializers/event_entity_spec.rb b/spec/serializers/event_entity_spec.rb deleted file mode 100644 index bb54597c967..00000000000 --- a/spec/serializers/event_entity_spec.rb +++ /dev/null @@ -1,13 +0,0 @@ -require 'spec_helper' - -describe EventEntity do - subject { described_class.represent(create(:event)).as_json } - - it 'exposes author' do - expect(subject).to include(:author) - end - - it 'exposes core elements of event' do - expect(subject).to include(:updated_at) - end -end diff --git a/spec/serializers/merge_request_widget_entity_spec.rb b/spec/serializers/merge_request_widget_entity_spec.rb index a5924a8589c..9bbe815c115 100644 --- a/spec/serializers/merge_request_widget_entity_spec.rb +++ b/spec/serializers/merge_request_widget_entity_spec.rb @@ -35,6 +35,80 @@ describe MergeRequestWidgetEntity do end end + describe 'metrics' do + context 'when metrics record exists with merged data' do + before do + resource.mark_as_merged! + resource.metrics.update!(merged_by: user) + end + + it 'matches merge request metrics schema' do + expect(subject[:metrics].with_indifferent_access) + .to match_schema('entities/merge_request_metrics') + end + + it 'returns values from metrics record' do + expect(subject.dig(:metrics, :merged_by, :id)) + .to eq(resource.metrics.merged_by_id) + end + end + + context 'when metrics record exists with closed data' do + before do + resource.close! + resource.metrics.update!(latest_closed_by: user) + end + + it 'matches merge request metrics schema' do + expect(subject[:metrics].with_indifferent_access) + .to match_schema('entities/merge_request_metrics') + end + + it 'returns values from metrics record' do + expect(subject.dig(:metrics, :closed_by, :id)) + .to eq(resource.metrics.latest_closed_by_id) + end + end + + context 'when metrics does not exists' do + before do + resource.metrics.destroy! + resource.reload + end + + context 'when events exists' do + let!(:closed_event) { create(:event, :closed, project: project, target: resource) } + let!(:merge_event) { create(:event, :merged, project: project, target: resource) } + + it 'matches merge request metrics schema' do + expect(subject[:metrics].with_indifferent_access) + .to match_schema('entities/merge_request_metrics') + end + + it 'returns values from events record' do + expect(subject.dig(:metrics, :merged_by, :id)) + .to eq(merge_event.author_id) + + expect(subject.dig(:metrics, :closed_by, :id)) + .to eq(closed_event.author_id) + + expect(subject.dig(:metrics, :merged_at).to_s) + .to eq(merge_event.updated_at.to_s) + + expect(subject.dig(:metrics, :closed_at).to_s) + .to eq(closed_event.updated_at.to_s) + end + end + + context 'when events does not exists' do + it 'matches merge request metrics schema' do + expect(subject[:metrics].with_indifferent_access) + .to match_schema('entities/merge_request_metrics') + end + end + end + end + it 'has email_patches_path' do expect(subject[:email_patches_path]) .to eq("/#{resource.project.full_path}/merge_requests/#{resource.iid}.patch") diff --git a/spec/services/create_deployment_service_spec.rb b/spec/services/create_deployment_service_spec.rb index 08267d6e6a0..b9bfbb11511 100644 --- a/spec/services/create_deployment_service_spec.rb +++ b/spec/services/create_deployment_service_spec.rb @@ -266,7 +266,7 @@ describe CreateDeploymentService do context "while updating the 'first_deployed_to_production_at' time" do before do - merge_request.mark_as_merged + merge_request.metrics.update!(merged_at: Time.now) end context "for merge requests merged before the current deploy" do diff --git a/spec/services/merge_requests/close_service_spec.rb b/spec/services/merge_requests/close_service_spec.rb index 2a59bc4594a..4d12de3ecce 100644 --- a/spec/services/merge_requests/close_service_spec.rb +++ b/spec/services/merge_requests/close_service_spec.rb @@ -52,6 +52,19 @@ describe MergeRequests::CloseService do end end + it 'updates metrics' do + metrics = merge_request.metrics + metrics_service = double(MergeRequestMetricsService) + allow(MergeRequestMetricsService) + .to receive(:new) + .with(metrics) + .and_return(metrics_service) + + expect(metrics_service).to receive(:close) + + described_class.new(project, user, {}).execute(merge_request) + end + it 'refreshes the number of open merge requests for a valid MR', :use_clean_rails_memory_store_caching do service = described_class.new(project, user, {}) diff --git a/spec/services/merge_requests/post_merge_service_spec.rb b/spec/services/merge_requests/post_merge_service_spec.rb index 8f2c5df5907..70957431942 100644 --- a/spec/services/merge_requests/post_merge_service_spec.rb +++ b/spec/services/merge_requests/post_merge_service_spec.rb @@ -22,5 +22,18 @@ describe MergeRequests::PostMergeService do expect { service.execute(merge_request) } .to change { project.open_merge_requests_count }.from(1).to(0) end + + it 'updates metrics' do + metrics = merge_request.metrics + metrics_service = double(MergeRequestMetricsService) + allow(MergeRequestMetricsService) + .to receive(:new) + .with(metrics) + .and_return(metrics_service) + + expect(metrics_service).to receive(:merge) + + described_class.new(project, user, {}).execute(merge_request) + end end end diff --git a/spec/services/merge_requests/reopen_service_spec.rb b/spec/services/merge_requests/reopen_service_spec.rb index 94f31ff139c..a44d63e5f9f 100644 --- a/spec/services/merge_requests/reopen_service_spec.rb +++ b/spec/services/merge_requests/reopen_service_spec.rb @@ -47,6 +47,19 @@ describe MergeRequests::ReopenService do end end + it 'updates metrics' do + metrics = merge_request.metrics + service = double(MergeRequestMetricsService) + allow(MergeRequestMetricsService) + .to receive(:new) + .with(metrics) + .and_return(service) + + expect(service).to receive(:reopen) + + described_class.new(project, user, {}).execute(merge_request) + end + it 'refreshes the number of open merge requests for a valid MR' do service = described_class.new(project, user, {}) diff --git a/spec/services/update_merge_request_metrics_service_spec.rb b/spec/services/update_merge_request_metrics_service_spec.rb new file mode 100644 index 00000000000..b5fb999381d --- /dev/null +++ b/spec/services/update_merge_request_metrics_service_spec.rb @@ -0,0 +1,42 @@ +require 'rails_helper' + +describe MergeRequestMetricsService do + let(:metrics) { create(:merge_request).metrics } + + describe '#merge' do + it 'updates metrics' do + user = create(:user) + service = described_class.new(metrics) + event = double(Event, author_id: user.id, created_at: Time.now) + + service.merge(event) + + expect(metrics.merged_by).to eq(user) + expect(metrics.merged_at).to eq(event.created_at) + end + end + + describe '#close' do + it 'updates metrics' do + user = create(:user) + service = described_class.new(metrics) + event = double(Event, author_id: user.id, created_at: Time.now) + + service.close(event) + + expect(metrics.latest_closed_by).to eq(user) + expect(metrics.latest_closed_at).to eq(event.created_at) + end + end + + describe '#reopen' do + it 'updates metrics' do + service = described_class.new(metrics) + + service.reopen + + expect(metrics.latest_closed_by).to be_nil + expect(metrics.latest_closed_at).to be_nil + end + end +end -- GitLab