diff options
author | Hiroyuki Sato <sathiroyuki@gmail.com> | 2018-01-12 20:38:36 +0000 |
---|---|---|
committer | Clement Ho <clemmakesapps@gmail.com> | 2018-01-12 20:38:36 +0000 |
commit | a7d26f00c35b945199d40332349f463043ae6122 (patch) | |
tree | 7b76a2e305e4a4f81f61271337c9731d0d50e16f | |
parent | 1b5f179557d03683f92ed3af3d357df056cc8ece (diff) | |
download | gitlab-ce-a7d26f00c35b945199d40332349f463043ae6122.tar.gz |
Display related merge requests in commit detail page
-rw-r--r-- | app/assets/javascripts/commit_merge_requests.js | 73 | ||||
-rw-r--r-- | app/assets/javascripts/dispatcher.js | 2 | ||||
-rw-r--r-- | app/controllers/projects/commit_controller.rb | 14 | ||||
-rw-r--r-- | app/models/commit.rb | 4 | ||||
-rw-r--r-- | app/models/merge_request.rb | 4 | ||||
-rw-r--r-- | app/models/merge_request_diff.rb | 3 | ||||
-rw-r--r-- | app/views/projects/commit/_commit_box.html.haml | 6 | ||||
-rw-r--r-- | changelogs/unreleased/display-mr-in-commit-page.yml | 5 | ||||
-rw-r--r-- | config/routes/project.rb | 1 | ||||
-rw-r--r-- | db/migrate/20170827123848_add_index_on_merge_request_diff_commit_sha.rb | 17 | ||||
-rw-r--r-- | db/schema.rb | 1 | ||||
-rw-r--r-- | spec/javascripts/commit_merge_requests_spec.js | 60 | ||||
-rw-r--r-- | spec/models/commit_spec.rb | 13 | ||||
-rw-r--r-- | spec/models/merge_request_diff_spec.rb | 22 | ||||
-rw-r--r-- | spec/models/merge_request_spec.rb | 33 |
15 files changed, 256 insertions, 2 deletions
diff --git a/app/assets/javascripts/commit_merge_requests.js b/app/assets/javascripts/commit_merge_requests.js new file mode 100644 index 00000000000..f76c9b7e690 --- /dev/null +++ b/app/assets/javascripts/commit_merge_requests.js @@ -0,0 +1,73 @@ +/* global Flash */ + +import axios from './lib/utils/axios_utils'; +import { n__, s__ } from './locale'; + +export function getHeaderText(childElementCount, mergeRequestCount) { + if (childElementCount === 0) { + return `${mergeRequestCount} ${n__('merge request', 'merge requests', mergeRequestCount)}`; + } + return ','; +} + +export function createHeader(childElementCount, mergeRequestCount) { + const headerText = getHeaderText(childElementCount, mergeRequestCount); + + return $('<span />', { + class: 'append-right-5', + text: headerText, + }); +} + +export function createLink(mergeRequest) { + return $('<a />', { + class: 'append-right-5', + href: mergeRequest.path, + text: `!${mergeRequest.iid}`, + }); +} + +export function createTitle(mergeRequest) { + return $('<span />', { + text: mergeRequest.title, + }); +} + +export function createItem(mergeRequest) { + const $item = $('<span />'); + const $link = createLink(mergeRequest); + const $title = createTitle(mergeRequest); + $item.append($link); + $item.append($title); + + return $item; +} + +export function createContent(mergeRequests) { + const $content = $('<span />'); + + if (mergeRequests.length === 0) { + $content.text(s__('Commits|No related merge requests found')); + } else { + mergeRequests.forEach((mergeRequest) => { + const $header = createHeader($content.children().length, mergeRequests.length); + const $item = createItem(mergeRequest); + $content.append($header); + $content.append($item); + }); + } + + return $content; +} + +export function fetchCommitMergeRequests() { + const $container = $('.merge-requests'); + + axios.get($container.data('projectCommitPath')) + .then((response) => { + const $content = createContent(response.data); + + $container.html($content); + }) + .catch(() => Flash(s__('Commits|An error occurred while fetching merge requests data.'))); +} diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index 64fb8aaeb7a..04b90035bcb 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -68,6 +68,7 @@ import Diff from './diff'; import ProjectLabelSubscription from './project_label_subscription'; import SearchAutocomplete from './search_autocomplete'; import Activities from './activities'; +import { fetchCommitMergeRequests } from './commit_merge_requests'; (function() { var Dispatcher; @@ -311,6 +312,7 @@ import Activities from './activities'; const stickyBarPaddingTop = 16; initChangesDropdown(document.querySelector('.navbar-gitlab').offsetHeight - stickyBarPaddingTop); $('.commit-info.branches').load(document.querySelector('.js-commit-box').dataset.commitPath); + fetchCommitMergeRequests(); break; case 'projects:commit:pipelines': new MiniPipelineGraph({ diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index 2e7344b1cad..effb484ef0f 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -12,7 +12,7 @@ class Projects::CommitController < Projects::ApplicationController before_action :authorize_download_code! before_action :authorize_read_pipeline!, only: [:pipelines] before_action :commit - before_action :define_commit_vars, only: [:show, :diff_for_path, :pipelines] + before_action :define_commit_vars, only: [:show, :diff_for_path, :pipelines, :merge_requests] before_action :define_note_vars, only: [:show, :diff_for_path] before_action :authorize_edit_tree!, only: [:revert, :cherry_pick] @@ -52,6 +52,18 @@ class Projects::CommitController < Projects::ApplicationController end end + def merge_requests + @merge_requests = @commit.merge_requests.map do |mr| + { iid: mr.iid, path: merge_request_path(mr), title: mr.title } + end + + respond_to do |format| + format.json do + render json: @merge_requests.to_json + end + end + end + def branches # branch_names_contains/tag_names_contains can take a long time when there are thousands of # branches/tags - each `git branch --contains xxx` request can consume a cpu core. diff --git a/app/models/commit.rb b/app/models/commit.rb index ede8ad301e4..21904c87f01 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -238,6 +238,10 @@ class Commit notes.includes(:author) end + def merge_requests + @merge_requests ||= project.merge_requests.by_commit_sha(sha) + end + def method_missing(method, *args, &block) @raw.__send__(method, *args, &block) # rubocop:disable GitlabSecurity/PublicSend end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index b7762a5f281..8028ff3875b 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -140,7 +140,9 @@ class MergeRequest < ActiveRecord::Base scope :merged, -> { with_state(:merged) } scope :closed_and_merged, -> { with_states(:closed, :merged) } scope :from_source_branches, ->(branches) { where(source_branch: branches) } - + scope :by_commit_sha, ->(sha) do + where('EXISTS (?)', MergeRequestDiff.select(1).where('merge_requests.latest_merge_request_diff_id = merge_request_diffs.id').by_commit_sha(sha)).reorder(nil) + end scope :join_project, -> { joins(:target_project) } scope :references_project, -> { references(:target_project) } scope :assigned, -> { where("assignee_id IS NOT NULL") } diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index afab72930c1..69a846da9be 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -28,6 +28,9 @@ class MergeRequestDiff < ActiveRecord::Base end scope :viewable, -> { without_state(:empty) } + scope :by_commit_sha, ->(sha) do + joins(:merge_request_diff_commits).where(merge_request_diff_commits: { sha: sha }).reorder(nil) + end scope :recent, -> { order(id: :desc).limit(100) } diff --git a/app/views/projects/commit/_commit_box.html.haml b/app/views/projects/commit/_commit_box.html.haml index 09934c09865..461129a3e0e 100644 --- a/app/views/projects/commit/_commit_box.html.haml +++ b/app/views/projects/commit/_commit_box.html.haml @@ -64,6 +64,12 @@ .commit-info.branches %i.fa.fa-spinner.fa-spin + .well-segment.merge-request-info + .icon-container + = custom_icon('mr_bold') + %span.commit-info.merge-requests{ 'data-project-commit-path' => merge_requests_project_commit_path(@project, @commit.id, format: :json) } + = icon('spinner spin') + - if @commit.last_pipeline - last_pipeline = @commit.last_pipeline .well-segment.pipeline-info diff --git a/changelogs/unreleased/display-mr-in-commit-page.yml b/changelogs/unreleased/display-mr-in-commit-page.yml new file mode 100644 index 00000000000..a9224c00b66 --- /dev/null +++ b/changelogs/unreleased/display-mr-in-commit-page.yml @@ -0,0 +1,5 @@ +--- +title: Add link on commit page to merge request that introduced that commit +merge_request: 13713 +author: Hiroyuki Sato +type: added diff --git a/config/routes/project.rb b/config/routes/project.rb index bdf4b199c0a..43ada9ba145 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -50,6 +50,7 @@ constraints(ProjectUrlConstrainer.new) do post :revert post :cherry_pick get :diff_for_path + get :merge_requests end end diff --git a/db/migrate/20170827123848_add_index_on_merge_request_diff_commit_sha.rb b/db/migrate/20170827123848_add_index_on_merge_request_diff_commit_sha.rb new file mode 100644 index 00000000000..1b360b231a8 --- /dev/null +++ b/db/migrate/20170827123848_add_index_on_merge_request_diff_commit_sha.rb @@ -0,0 +1,17 @@ +# rubocop:disable RemoveIndex + +class AddIndexOnMergeRequestDiffCommitSha < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :merge_request_diff_commits, :sha, length: Gitlab::Database.mysql? ? 20 : nil + end + + def down + remove_index :merge_request_diff_commits, :sha if index_exists? :merge_request_diff_commits, :sha + end +end diff --git a/db/schema.rb b/db/schema.rb index 8a6db61250b..f47accca21a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1013,6 +1013,7 @@ ActiveRecord::Schema.define(version: 20180105212544) do end add_index "merge_request_diff_commits", ["merge_request_diff_id", "relative_order"], name: "index_merge_request_diff_commits_on_mr_diff_id_and_order", unique: true, using: :btree + add_index "merge_request_diff_commits", ["sha"], name: "index_merge_request_diff_commits_on_sha", using: :btree create_table "merge_request_diff_files", id: false, force: :cascade do |t| t.integer "merge_request_diff_id", null: false diff --git a/spec/javascripts/commit_merge_requests_spec.js b/spec/javascripts/commit_merge_requests_spec.js new file mode 100644 index 00000000000..3466ef51ea8 --- /dev/null +++ b/spec/javascripts/commit_merge_requests_spec.js @@ -0,0 +1,60 @@ +import * as CommitMergeRequests from '~/commit_merge_requests'; + +describe('CommitMergeRequests', () => { + describe('createContent', () => { + it('should return created content', () => { + const content1 = CommitMergeRequests.createContent([{ iid: 1, path: '/path1', title: 'foo' }, { iid: 2, path: '/path2', title: 'baz' }])[0]; + expect(content1.tagName).toEqual('SPAN'); + expect(content1.childElementCount).toEqual(4); + + const content2 = CommitMergeRequests.createContent([])[0]; + expect(content2.tagName).toEqual('SPAN'); + expect(content2.childElementCount).toEqual(0); + expect(content2.innerText).toEqual('No related merge requests found'); + }); + }); + + describe('getHeaderText', () => { + it('should return header text', () => { + expect(CommitMergeRequests.getHeaderText(0, 1)).toEqual('1 merge request'); + expect(CommitMergeRequests.getHeaderText(0, 2)).toEqual('2 merge requests'); + expect(CommitMergeRequests.getHeaderText(1, 1)).toEqual(','); + expect(CommitMergeRequests.getHeaderText(1, 2)).toEqual(','); + }); + }); + + describe('createHeader', () => { + it('should return created header', () => { + const header = CommitMergeRequests.createHeader(0, 1)[0]; + expect(header.tagName).toEqual('SPAN'); + expect(header.innerText).toEqual('1 merge request'); + }); + }); + + describe('createItem', () => { + it('should return created item', () => { + const item = CommitMergeRequests.createItem({ iid: 1, path: '/path', title: 'foo' })[0]; + expect(item.tagName).toEqual('SPAN'); + expect(item.childElementCount).toEqual(2); + expect(item.children[0].tagName).toEqual('A'); + expect(item.children[1].tagName).toEqual('SPAN'); + }); + }); + + describe('createLink', () => { + it('should return created link', () => { + const link = CommitMergeRequests.createLink({ iid: 1, path: '/path', title: 'foo' })[0]; + expect(link.tagName).toEqual('A'); + expect(link.href).toMatch(/\/path$/); + expect(link.innerText).toEqual('!1'); + }); + }); + + describe('createTitle', () => { + it('should return created title', () => { + const title = CommitMergeRequests.createTitle({ iid: 1, path: '/path', title: 'foo' })[0]; + expect(title.tagName).toEqual('SPAN'); + expect(title.innerText).toEqual('foo'); + }); + }); +}); diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 817254c7d1e..d3826417762 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -513,4 +513,17 @@ eos expect(described_class.valid_hash?('a' * 41)).to be false end end + + describe '#merge_requests' do + let!(:project) { create(:project, :repository) } + let!(:merge_request1) { create(:merge_request, source_project: project, source_branch: 'master', target_branch: 'feature') } + let!(:merge_request2) { create(:merge_request, source_project: project, source_branch: 'merged-target', target_branch: 'feature') } + let(:commit1) { merge_request1.merge_request_diff.commits.last } + let(:commit2) { merge_request1.merge_request_diff.commits.first } + + it 'returns merge_requests that introduced that commit' do + expect(commit1.merge_requests).to eq([merge_request1, merge_request2]) + expect(commit2.merge_requests).to eq([merge_request1]) + end + end end diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index d556004eccf..b4249d72fc8 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -15,6 +15,28 @@ describe MergeRequestDiff do it { expect(subject.start_commit_sha).to eq('0b4bc9a49b562e85de7cc9e834518ea6828729b9') } end + describe '.by_commit_sha' do + subject(:by_commit_sha) { described_class.by_commit_sha(sha) } + + let!(:merge_request) { create(:merge_request, :with_diffs) } + + context 'with sha contained in' do + let(:sha) { 'b83d6e391c22777fca1ed3012fce84f633d7fed0' } + + it 'returns merge request diffs' do + expect(by_commit_sha).to eq([merge_request.merge_request_diff]) + end + end + + context 'with sha not contained in' do + let(:sha) { 'b83d6e3' } + + it 'returns empty result' do + expect(by_commit_sha).to be_empty + end + end + end + describe '#latest' do let!(:mr) { create(:merge_request, :with_diffs) } let!(:first_diff) { mr.merge_request_diff } diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index ceb06b3be18..c76f32b3989 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -87,6 +87,39 @@ describe MergeRequest do it { is_expected.to respond_to(:merge_when_pipeline_succeeds) } end + describe '.by_commit_sha' do + subject(:by_commit_sha) { described_class.by_commit_sha(sha) } + + let!(:merge_request) { create(:merge_request, :with_diffs) } + + context 'with sha contained in latest merge request diff' do + let(:sha) { 'b83d6e391c22777fca1ed3012fce84f633d7fed0' } + + it 'returns merge requests' do + expect(by_commit_sha).to eq([merge_request]) + end + end + + context 'with sha contained not in latest merge request diff' do + let(:sha) { 'b83d6e391c22777fca1ed3012fce84f633d7fed0' } + + it 'returns empty requests' do + latest_merge_request_diff = merge_request.merge_request_diffs.create + latest_merge_request_diff.merge_request_diff_commits.where(sha: 'b83d6e391c22777fca1ed3012fce84f633d7fed0').delete_all + + expect(by_commit_sha).to be_empty + end + end + + context 'with sha not contained in' do + let(:sha) { 'b83d6e3' } + + it 'returns empty result' do + expect(by_commit_sha).to be_empty + end + end + end + describe '.in_projects' do it 'returns the merge requests for a set of projects' do expect(described_class.in_projects(Project.all)).to eq([subject]) |