Commit 4b17356d authored by Adam Hegyi's avatar Adam Hegyi

Apply reviewer feedback

- Setup associations
- Minor stylistics changes
parent 51097771
...@@ -28,14 +28,15 @@ module Resolvers ...@@ -28,14 +28,15 @@ module Resolvers
argument_names.delete(:merged_before) argument_names.delete(:merged_before)
argument_names.delete(:merged_after) argument_names.delete(:merged_after)
# no extra filtering arguments are provided
return unless argument_names.empty?
return unless args[:merged_after] || args[:merged_before]
# Detecting a specific query pattern: # Detecting a specific query pattern:
# mergeRequests(mergedAfter: "X", mergedBefore: "Y") { # mergeRequests(mergedAfter: "X", mergedBefore: "Y") {
# count # count
# } # }
lookahead.selects?(:count) && lookahead.selects?(:count) && lookahead.selections.size == 1 # no other nodes are selected
lookahead.selections.size == 1 && # no other nodes are selected
(args[:merged_after] || args[:merged_before]) &&
argument_names.empty? # no extra filtering arguments are provided
end end
end end
end end
...@@ -5,7 +5,7 @@ class MergeRequest::Metrics < ApplicationRecord ...@@ -5,7 +5,7 @@ class MergeRequest::Metrics < ApplicationRecord
belongs_to :pipeline, class_name: 'Ci::Pipeline', foreign_key: :pipeline_id belongs_to :pipeline, class_name: 'Ci::Pipeline', foreign_key: :pipeline_id
belongs_to :latest_closed_by, class_name: 'User' belongs_to :latest_closed_by, class_name: 'User'
belongs_to :merged_by, class_name: 'User' belongs_to :merged_by, class_name: 'User'
belongs_to :target_project, class_name: 'Project' belongs_to :target_project, class_name: 'Project', inverse_of: :merge_requests
before_save :ensure_target_project_id before_save :ensure_target_project_id
......
...@@ -218,6 +218,7 @@ class Project < ApplicationRecord ...@@ -218,6 +218,7 @@ class Project < ApplicationRecord
# Merge Requests for target project should be removed with it # Merge Requests for target project should be removed with it
has_many :merge_requests, foreign_key: 'target_project_id', inverse_of: :target_project has_many :merge_requests, foreign_key: 'target_project_id', inverse_of: :target_project
has_many :merge_request_metrics, foreign_key: 'target_project', class_name: 'MergeRequest::Metrics', inverse_of: :target_project
has_many :source_of_merge_requests, foreign_key: 'source_project_id', class_name: 'MergeRequest' has_many :source_of_merge_requests, foreign_key: 'source_project_id', class_name: 'MergeRequest'
has_many :issues has_many :issues
has_many :labels, class_name: 'ProjectLabel' has_many :labels, class_name: 'ProjectLabel'
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
name: optimized_merge_request_count_with_merged_at_filter name: optimized_merge_request_count_with_merged_at_filter
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/52113 introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/52113
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/299347 rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/299347
milestone: '13.8' milestone: '13.9'
type: development type: development
group: group::optimize group: group::optimize
default_enabled: false default_enabled: false
...@@ -6,13 +6,13 @@ RSpec.describe MergeRequest::MetricsFinder do ...@@ -6,13 +6,13 @@ RSpec.describe MergeRequest::MetricsFinder do
let_it_be(:current_user) { create(:user) } let_it_be(:current_user) { create(:user) }
let_it_be(:project) { create(:project, :repository) } let_it_be(:project) { create(:project, :repository) }
let_it_be(:merge_request_not_merged) { create(:merge_request, :unique_branches, source_project: project) } let_it_be(:merge_request_not_merged) { create(:merge_request, :unique_branches, source_project: project) }
let_it_be(:merged_at) { Time.new(2020, 5, 1) }
let_it_be(:merge_request_merged) do let_it_be(:merge_request_merged) do
create(:merge_request, :unique_branches, :merged, source_project: project).tap do |mr| create(:merge_request, :unique_branches, :merged, source_project: project).tap do |mr|
mr.metrics.update!(merged_at: Time.new(2020, 5, 1)) mr.metrics.update!(merged_at: merged_at)
end end
end end
let(:merged_at) { merge_request_merged.metrics.merged_at }
let(:params) do let(:params) do
{ {
target_project: project, target_project: project,
......
...@@ -561,6 +561,7 @@ project: ...@@ -561,6 +561,7 @@ project:
- exported_protected_branches - exported_protected_branches
- incident_management_oncall_schedules - incident_management_oncall_schedules
- debian_distributions - debian_distributions
- merge_request_metrics
award_emoji: award_emoji:
- awardable - awardable
- user - user
...@@ -589,6 +590,7 @@ lfs_file_locks: ...@@ -589,6 +590,7 @@ lfs_file_locks:
project_badges: project_badges:
- project - project
metrics: metrics:
- target_project
- merge_request - merge_request
- latest_closed_by - latest_closed_by
- merged_by - merged_by
......
...@@ -5,6 +5,7 @@ require 'spec_helper' ...@@ -5,6 +5,7 @@ require 'spec_helper'
RSpec.describe MergeRequest::Metrics do RSpec.describe MergeRequest::Metrics do
describe 'associations' do describe 'associations' do
it { is_expected.to belong_to(:merge_request) } it { is_expected.to belong_to(:merge_request) }
it { is_expected.to belong_to(:target_project) }
it { is_expected.to belong_to(:latest_closed_by).class_name('User') } it { is_expected.to belong_to(:latest_closed_by).class_name('User') }
it { is_expected.to belong_to(:merged_by).class_name('User') } it { is_expected.to belong_to(:merged_by).class_name('User') }
end end
...@@ -36,5 +37,15 @@ RSpec.describe MergeRequest::Metrics do ...@@ -36,5 +37,15 @@ RSpec.describe MergeRequest::Metrics do
is_expected.not_to include([metrics_2]) is_expected.not_to include([metrics_2])
end end
end end
describe '.by_target_project' do
let(:target_project) { metrics_1.target_project }
subject { described_class.by_target_project(target_project) }
it 'finds metrics record with the associated target project' do
is_expected.to eq([metrics_1])
end
end
end end
end end
...@@ -21,6 +21,7 @@ RSpec.describe Project, factory_default: :keep do ...@@ -21,6 +21,7 @@ RSpec.describe Project, factory_default: :keep do
it { is_expected.to have_many(:services) } it { is_expected.to have_many(:services) }
it { is_expected.to have_many(:events) } it { is_expected.to have_many(:events) }
it { is_expected.to have_many(:merge_requests) } it { is_expected.to have_many(:merge_requests) }
it { is_expected.to have_many(:merge_request_metrics) }
it { is_expected.to have_many(:issues) } it { is_expected.to have_many(:issues) }
it { is_expected.to have_many(:milestones) } it { is_expected.to have_many(:milestones) }
it { is_expected.to have_many(:iterations) } it { is_expected.to have_many(:iterations) }
......
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