From e8a9f878a240ef2a70e39b5e59df128aadb4b93e Mon Sep 17 00:00:00 2001 From: Vasilii Iakliushin Date: Mon, 14 Dec 2020 19:05:29 +0100 Subject: [PATCH] Show merge request for squashed and merge commits Contributes to https://gitlab.com/gitlab-org/gitlab/-/issues/21686 * Add database indices for `squash_commit_sha` field * Add new scope `by_related_commit_sha` to MergeRequest model * Update MergeRequestFinder logic to use `by_related_commit_sha` scope for filtering --- app/finders/merge_requests_finder.rb | 2 +- app/models/merge_request.rb | 13 ++++++ ...how_merge_request_for_squashed_commits.yml | 5 +++ ...01216151616_add_squash_commit_sha_index.rb | 22 ++++++++++ db/schema_migrations/20201216151616 | 1 + db/structure.sql | 2 + spec/finders/merge_requests_finder_spec.rb | 39 ++++++++++++++--- spec/models/merge_request_spec.rb | 42 +++++++++++++++++++ 8 files changed, 119 insertions(+), 7 deletions(-) create mode 100644 changelogs/unreleased/21686_show_merge_request_for_squashed_commits.yml create mode 100644 db/migrate/20201216151616_add_squash_commit_sha_index.rb create mode 100644 db/schema_migrations/20201216151616 diff --git a/app/finders/merge_requests_finder.rb b/app/finders/merge_requests_finder.rb index 978550aedaf4b9..87c190bdc457a7 100644 --- a/app/finders/merge_requests_finder.rb +++ b/app/finders/merge_requests_finder.rb @@ -84,7 +84,7 @@ def filter_negated_items(items) def by_commit(items) return items unless params[:commit_sha].presence - items.by_commit_sha(params[:commit_sha]) + items.by_related_commit_sha(params[:commit_sha]) end def source_branch diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index db066184e9115a..3918498f69420e 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -261,6 +261,19 @@ def public_merge_status scope :by_merge_commit_sha, -> (sha) do where(merge_commit_sha: sha) end + scope :by_squash_commit_sha, -> (sha) do + where(squash_commit_sha: sha) + end + scope :by_related_commit_sha, -> (sha) do + from_union( + [ + by_commit_sha(sha), + by_squash_commit_sha(sha), + by_merge_commit_sha(sha) + ], + remove_duplicates: false + ) + end scope :by_cherry_pick_sha, -> (sha) do joins(:notes).where(notes: { commit_id: sha }) end diff --git a/changelogs/unreleased/21686_show_merge_request_for_squashed_commits.yml b/changelogs/unreleased/21686_show_merge_request_for_squashed_commits.yml new file mode 100644 index 00000000000000..cc59ac92fd9c83 --- /dev/null +++ b/changelogs/unreleased/21686_show_merge_request_for_squashed_commits.yml @@ -0,0 +1,5 @@ +--- +title: Extend MergeRequestFinder to search by squash and merge commits +merge_request: 49968 +author: +type: added diff --git a/db/migrate/20201216151616_add_squash_commit_sha_index.rb b/db/migrate/20201216151616_add_squash_commit_sha_index.rb new file mode 100644 index 00000000000000..ac6d3fda2d2838 --- /dev/null +++ b/db/migrate/20201216151616_add_squash_commit_sha_index.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class AddSquashCommitShaIndex < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + INDEX_NAME = "index_merge_requests_on_target_project_id_and_squash_commit_sha" + + disable_ddl_transaction! + + def up + add_concurrent_index :merge_requests, + [:target_project_id, :squash_commit_sha], + name: INDEX_NAME + end + + def down + remove_concurrent_index :merge_requests, + [:target_project_id, :squash_commit_sha], + name: INDEX_NAME + end +end diff --git a/db/schema_migrations/20201216151616 b/db/schema_migrations/20201216151616 new file mode 100644 index 00000000000000..a8b27610716b29 --- /dev/null +++ b/db/schema_migrations/20201216151616 @@ -0,0 +1 @@ +f1c6927431895c6ce03fe7e0be30fcd0a1f4ccfeac8277ee0662d7434b97d257 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 58a1f4522096b5..a84ebe1a7b9ea9 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -21982,6 +21982,8 @@ CREATE INDEX index_merge_requests_on_target_project_id_and_iid_and_state_id ON m CREATE INDEX index_merge_requests_on_target_project_id_and_iid_jira_title ON merge_requests USING btree (target_project_id, iid) WHERE ((title)::text ~ '[A-Z][A-Z_0-9]+-\d+'::text); +CREATE INDEX index_merge_requests_on_target_project_id_and_squash_commit_sha ON merge_requests USING btree (target_project_id, squash_commit_sha); + CREATE INDEX index_merge_requests_on_target_project_id_and_target_branch ON merge_requests USING btree (target_project_id, target_branch) WHERE ((state_id = 1) AND (merge_when_pipeline_succeeds = true)); CREATE INDEX index_merge_requests_on_target_project_id_iid_jira_description ON merge_requests USING btree (target_project_id, iid) WHERE (description ~ '[A-Z][A-Z_0-9]+-\d+'::text); diff --git a/spec/finders/merge_requests_finder_spec.rb b/spec/finders/merge_requests_finder_spec.rb index 7b59b581b1c6c4..73621660e06de6 100644 --- a/spec/finders/merge_requests_finder_spec.rb +++ b/spec/finders/merge_requests_finder_spec.rb @@ -76,13 +76,40 @@ expect(merge_requests).to contain_exactly(merge_request3, merge_request4) end - it 'filters by commit sha' do - merge_requests = described_class.new( - user, - commit_sha: merge_request5.merge_request_diff.last_commit_sha - ).execute + context 'filters by commit sha' do + subject(:merge_requests) { described_class.new(user, commit_sha: commit_sha).execute } + + context 'when commit belongs to the merge request' do + let(:commit_sha) { merge_request5.merge_request_diff.last_commit_sha } + + it 'filters by commit sha' do + is_expected.to contain_exactly(merge_request5) + end + end + + context 'when commit is a squash commit' do + before do + merge_request4.update!(squash_commit_sha: commit_sha) + end + + let(:commit_sha) { '1234abcd' } - expect(merge_requests).to contain_exactly(merge_request5) + it 'filters by commit sha' do + is_expected.to contain_exactly(merge_request4) + end + end + + context 'when commit is a merge commit' do + before do + merge_request4.update!(merge_commit_sha: commit_sha) + end + + let(:commit_sha) { '1234dcba' } + + it 'filters by commit sha' do + is_expected.to contain_exactly(merge_request4) + end + end end context 'filters by merged_at date' do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 5c36622b1c6c86..d1f5a2c70771e6 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -411,6 +411,48 @@ end end + describe '.by_squash_commit_sha' do + subject { described_class.by_squash_commit_sha(sha) } + + let(:sha) { '123abc' } + let(:merge_request) { create(:merge_request, :merged, squash_commit_sha: sha) } + + it 'returns merge requests that match the given squash commit' do + is_expected.to eq([merge_request]) + end + end + + describe '.by_related_commit_sha' do + subject { described_class.by_related_commit_sha(sha) } + + context 'when commit is a squash commit' do + let!(:merge_request) { create(:merge_request, :merged, squash_commit_sha: sha) } + let(:sha) { '123abc' } + + it { is_expected.to eq([merge_request]) } + end + + context 'when commit is a part of the merge request' do + let!(:merge_request) { create(:merge_request, :with_diffs) } + let(:sha) { 'b83d6e391c22777fca1ed3012fce84f633d7fed0' } + + it { is_expected.to eq([merge_request]) } + end + + context 'when commit is a merge commit' do + let!(:merge_request) { create(:merge_request, :merged, merge_commit_sha: sha) } + let(:sha) { '123abc' } + + it { is_expected.to eq([merge_request]) } + end + + context 'when commit is not found' do + let(:sha) { '0000' } + + it { is_expected.to be_empty } + end + end + describe '.by_cherry_pick_sha' do it 'returns merge requests that match the given merge commit' do note = create(:track_mr_picking_note, commit_id: '456abc') -- GitLab