summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorHiroyuki Sato <sathiroyuki@gmail.com>2018-01-12 20:38:36 +0000
committerClement Ho <clemmakesapps@gmail.com>2018-01-12 20:38:36 +0000
commita7d26f00c35b945199d40332349f463043ae6122 (patch)
tree7b76a2e305e4a4f81f61271337c9731d0d50e16f
parent1b5f179557d03683f92ed3af3d357df056cc8ece (diff)
downloadgitlab-ce-a7d26f00c35b945199d40332349f463043ae6122.tar.gz
Display related merge requests in commit detail page
-rw-r--r--app/assets/javascripts/commit_merge_requests.js73
-rw-r--r--app/assets/javascripts/dispatcher.js2
-rw-r--r--app/controllers/projects/commit_controller.rb14
-rw-r--r--app/models/commit.rb4
-rw-r--r--app/models/merge_request.rb4
-rw-r--r--app/models/merge_request_diff.rb3
-rw-r--r--app/views/projects/commit/_commit_box.html.haml6
-rw-r--r--changelogs/unreleased/display-mr-in-commit-page.yml5
-rw-r--r--config/routes/project.rb1
-rw-r--r--db/migrate/20170827123848_add_index_on_merge_request_diff_commit_sha.rb17
-rw-r--r--db/schema.rb1
-rw-r--r--spec/javascripts/commit_merge_requests_spec.js60
-rw-r--r--spec/models/commit_spec.rb13
-rw-r--r--spec/models/merge_request_diff_spec.rb22
-rw-r--r--spec/models/merge_request_spec.rb33
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])