From bc493bd88e35deb9e228aadb029320506814f863 Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Date: Tue, 30 Aug 2016 15:25:41 -0500 Subject: Add pipelines tab to new MR --- app/views/projects/merge_requests/_new_submit.html.haml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/app/views/projects/merge_requests/_new_submit.html.haml b/app/views/projects/merge_requests/_new_submit.html.haml index da6927879a4..3a670cb70dd 100644 --- a/app/views/projects/merge_requests/_new_submit.html.haml +++ b/app/views/projects/merge_requests/_new_submit.html.haml @@ -18,6 +18,7 @@ = f.hidden_field :target_branch .mr-compare.merge-request +<<<<<<< 4a8cb230c6ba28ccdcd6da91855005e4fa0c46bf - if @commits.empty? .commits-empty %h4 @@ -30,6 +31,10 @@ Commits %span.badge= @commits.size - if @pipeline + %li.builds-tab + = link_to url_for(params), data: {target: 'div#pipelines', action: 'pipelines', toggle: 'tab'} do + Pipelines + %span.badge= @statuses.size %li.builds-tab = link_to url_for(params), data: {target: 'div#builds', action: 'builds', toggle: 'tab'} do Builds @@ -47,6 +52,8 @@ - if @pipeline #builds.builds.tab-pane = render "projects/merge_requests/show/builds" + #pipelines.pipelines.tab-pane + = render "projects/merge_requests/show/pipelines" .mr-loading-status = spinner -- cgit v1.2.3 From 04c03d9d5ce1b6240ad0a7f500b2faeba453401e Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Gray Date: Mon, 10 Oct 2016 10:22:04 -0500 Subject: Fix pipeline tab content display and badge count --- app/views/projects/merge_requests/show/_pipelines.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/projects/merge_requests/show/_pipelines.html.haml b/app/views/projects/merge_requests/show/_pipelines.html.haml index afe3f3430c6..db0ba88417b 100644 --- a/app/views/projects/merge_requests/show/_pipelines.html.haml +++ b/app/views/projects/merge_requests/show/_pipelines.html.haml @@ -1 +1 @@ -= render "projects/commit/pipelines_list", pipelines: @pipelines, link_to_commit: true += render "projects/commit/pipelines_list", pipelines: @pipeline, link_to_commit: true -- cgit v1.2.3 From 4945959b7c3c930778d1b88367d5e60fd06b52a0 Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Gray Date: Tue, 11 Oct 2016 08:46:53 -0500 Subject: Fix variable to show multiple pipelines for MR --- app/views/projects/merge_requests/_new_submit.html.haml | 1 - app/views/projects/merge_requests/show/_pipelines.html.haml | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/app/views/projects/merge_requests/_new_submit.html.haml b/app/views/projects/merge_requests/_new_submit.html.haml index 3a670cb70dd..bfe461e6943 100644 --- a/app/views/projects/merge_requests/_new_submit.html.haml +++ b/app/views/projects/merge_requests/_new_submit.html.haml @@ -18,7 +18,6 @@ = f.hidden_field :target_branch .mr-compare.merge-request -<<<<<<< 4a8cb230c6ba28ccdcd6da91855005e4fa0c46bf - if @commits.empty? .commits-empty %h4 diff --git a/app/views/projects/merge_requests/show/_pipelines.html.haml b/app/views/projects/merge_requests/show/_pipelines.html.haml index db0ba88417b..afe3f3430c6 100644 --- a/app/views/projects/merge_requests/show/_pipelines.html.haml +++ b/app/views/projects/merge_requests/show/_pipelines.html.haml @@ -1 +1 @@ -= render "projects/commit/pipelines_list", pipelines: @pipeline, link_to_commit: true += render "projects/commit/pipelines_list", pipelines: @pipelines, link_to_commit: true -- cgit v1.2.3 From 27c762b8db3e2b6896ef88ea1a078745ae33909d Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 17 Oct 2016 11:48:36 +0200 Subject: Remove obsolete variable assigned to MR widget view --- app/controllers/projects/merge_requests_controller.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index a39b47b6d95..cfa463b6452 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -517,7 +517,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController def define_widget_vars @pipeline = @merge_request.pipeline - @pipelines = [@pipeline].compact end def define_commit_vars -- cgit v1.2.3 From db0e700d2ff9a372a8bd903faf305c0debd9b372 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 17 Oct 2016 12:55:18 +0200 Subject: Use all pipelines variable when creating merge request --- app/controllers/projects/merge_requests_controller.rb | 4 ++-- app/views/projects/merge_requests/_new_submit.html.haml | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index cfa463b6452..bce89d0a9ad 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -558,8 +558,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController @commit = @merge_request.diff_head_commit @base_commit = @merge_request.diff_base_commit - @pipeline = @merge_request.pipeline - @statuses = @pipeline.statuses.relevant if @pipeline + @pipelines = @merge_request.all_pipelines + @statuses = @pipelines.first.statuses.relevant if @pipelines.any? @note_counts = Note.where(commit_id: @commits.map(&:id)). group(:commit_id).count end diff --git a/app/views/projects/merge_requests/_new_submit.html.haml b/app/views/projects/merge_requests/_new_submit.html.haml index bfe461e6943..9c6f562f7db 100644 --- a/app/views/projects/merge_requests/_new_submit.html.haml +++ b/app/views/projects/merge_requests/_new_submit.html.haml @@ -29,11 +29,11 @@ = link_to url_for(params), data: {target: 'div#commits', action: 'new', toggle: 'tab'} do Commits %span.badge= @commits.size - - if @pipeline + - if @pipelines.any? %li.builds-tab = link_to url_for(params), data: {target: 'div#pipelines', action: 'pipelines', toggle: 'tab'} do Pipelines - %span.badge= @statuses.size + %span.badge= @pipelines.size %li.builds-tab = link_to url_for(params), data: {target: 'div#builds', action: 'builds', toggle: 'tab'} do Builds @@ -48,7 +48,7 @@ = render "projects/merge_requests/show/commits" #diffs.diffs.tab-pane - # This tab is always loaded via AJAX - - if @pipeline + - if @pipelines.any? #builds.builds.tab-pane = render "projects/merge_requests/show/builds" #pipelines.pipelines.tab-pane @@ -65,5 +65,5 @@ :javascript var merge_request = new MergeRequest({ action: "#{(@show_changes_tab ? 'new/diffs' : 'new')}", - buildsLoaded: "#{@pipeline ? 'true' : 'false'}" + buildsLoaded: "#{@pipelines.any? ? 'true' : 'false'}" }); -- cgit v1.2.3 From f7da506529ab0989069fe1c9d9bf9d819c13211d Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 17 Oct 2016 13:55:30 +0200 Subject: Extract pipeline vars in merge requests controller --- app/controllers/projects/merge_requests_controller.rb | 18 +++++++++++++----- app/views/projects/merge_requests/_show.html.haml | 2 +- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index bce89d0a9ad..4df0648a502 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -483,13 +483,12 @@ class Projects::MergeRequestsController < Projects::ApplicationController @noteable = @merge_request @commits_count = @merge_request.commits.count - @pipeline = @merge_request.pipeline - @statuses = @pipeline.statuses.relevant if @pipeline - if @merge_request.locked_long_ago? @merge_request.unlock_mr @merge_request.close end + + define_pipelines_vars end # Discussion tab data is rendered on html responses of actions @@ -543,6 +542,15 @@ class Projects::MergeRequestsController < Projects::ApplicationController ) end + def define_pipelines_vars + @pipelines = @merge_request.all_pipelines + + if @pipelines.any? + @pipeline = @pipelines.first + @statuses = @pipeline.statuses.relevant + end + end + def define_new_vars @noteable = @merge_request @@ -558,10 +566,10 @@ class Projects::MergeRequestsController < Projects::ApplicationController @commit = @merge_request.diff_head_commit @base_commit = @merge_request.diff_base_commit - @pipelines = @merge_request.all_pipelines - @statuses = @pipelines.first.statuses.relevant if @pipelines.any? @note_counts = Note.where(commit_id: @commits.map(&:id)). group(:commit_id).count + + define_pipelines_vars end def invalid_mr diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index 662463bc72b..fb4afe3bff2 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -61,7 +61,7 @@ %li.pipelines-tab = link_to pipelines_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: '#pipelines', action: 'pipelines', toggle: 'tab' } do Pipelines - %span.badge= @merge_request.all_pipelines.size + %span.badge= @pipelines.size %li.builds-tab = link_to builds_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: '#builds', action: 'builds', toggle: 'tab' } do Builds -- cgit v1.2.3 From d5b1d0ea501bb6eb0e18f63f4ddc0ba3f32d372a Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 17 Oct 2016 14:15:06 +0200 Subject: Use temporary compare commits when MR not persisted --- app/models/merge_request.rb | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 8c6905a442d..88e46ecdc8b 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -787,21 +787,19 @@ class MergeRequest < ActiveRecord::Base def all_pipelines return unless source_project - @all_pipelines ||= begin - sha = if persisted? - all_commits_sha - else - diff_head_sha - end - - source_project.pipelines.order(id: :desc). - where(sha: sha, ref: source_branch) - end + @all_pipelines ||= source_project.pipelines + .where(sha: all_commits_sha, ref: source_branch) + .order(id: :desc) end # Note that this could also return SHA from now dangling commits + # def all_commits_sha - merge_request_diffs.flat_map(&:commits_sha).uniq + if persisted? + merge_request_diffs.flat_map(&:commits_sha).uniq + else + compare_commits.reverse.map(&:id) + end end def merge_commit -- cgit v1.2.3 From ab8ef17fb2a2aa85a76d9106894280be9b910fda Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 17 Oct 2016 15:31:21 +0200 Subject: Extend merge request tests for all commits method --- app/models/merge_request.rb | 7 +++-- spec/models/merge_request_spec.rb | 58 +++++++++++++++++++++++++++------------ 2 files changed, 45 insertions(+), 20 deletions(-) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 88e46ecdc8b..ced0c13b837 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -788,8 +788,8 @@ class MergeRequest < ActiveRecord::Base return unless source_project @all_pipelines ||= source_project.pipelines - .where(sha: all_commits_sha, ref: source_branch) - .order(id: :desc) + .where(sha: all_commits_sha, ref: source_branch) + .order(id: :desc) end # Note that this could also return SHA from now dangling commits @@ -798,7 +798,8 @@ class MergeRequest < ActiveRecord::Base if persisted? merge_request_diffs.flat_map(&:commits_sha).uniq else - compare_commits.reverse.map(&:id) + cached_commits = compare_commits.to_a.reverse.map(&:id) + cached_commits.any? ? cached_commits : [diff_head_sha] end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 91a423b670c..344f69a703e 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -640,32 +640,56 @@ describe MergeRequest, models: true do end describe '#all_commits_sha' do - let(:all_commits_sha) do - subject.merge_request_diffs.flat_map(&:commits).map(&:sha).uniq - end + context 'when merge request is persisted' do + let(:all_commits_sha) do + subject.merge_request_diffs.flat_map(&:commits).map(&:sha).uniq + end - shared_examples 'returning all SHA' do - it 'returns all SHA from all merge_request_diffs' do - expect(subject.merge_request_diffs.size).to eq(2) - expect(subject.all_commits_sha).to eq(all_commits_sha) + shared_examples 'returning all SHA' do + it 'returns all SHA from all merge_request_diffs' do + expect(subject.merge_request_diffs.size).to eq(2) + expect(subject.all_commits_sha).to eq(all_commits_sha) + end end - end - context 'with a completely different branch' do - before do - subject.update(target_branch: 'v1.0.0') + context 'with a completely different branch' do + before do + subject.update(target_branch: 'v1.0.0') + end + + it_behaves_like 'returning all SHA' end - it_behaves_like 'returning all SHA' + context 'with a branch having no difference' do + before do + subject.update(target_branch: 'v1.1.0') + subject.reload # make sure commits were not cached + end + + it_behaves_like 'returning all SHA' + end end - context 'with a branch having no difference' do - before do - subject.update(target_branch: 'v1.1.0') - subject.reload # make sure commits were not cached + context 'when merge request is not persisted' do + context 'when compare commits are set in the service' do + let(:commit) { spy('commit') } + + subject do + build(:merge_request, compare_commits: [commit, commit]) + end + + it 'returns commits from compare commits temporary data' do + expect(subject.all_commits_sha).to eq [commit, commit] + end end - it_behaves_like 'returning all SHA' + context 'when compare commits are not set in the service' do + subject { build(:merge_request) } + + it 'returns array with diff head sha element only' do + expect(subject.all_commits_sha).to eq [subject.diff_head_sha] + end + end end end -- cgit v1.2.3 From 72d84e48511fcf88b1d9efb622eb37cdff95aa1c Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 18 Oct 2016 12:05:47 +0200 Subject: Improve code that creates a list of commits for MR --- app/models/merge_request.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index ced0c13b837..fedc35102ef 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -797,9 +797,10 @@ class MergeRequest < ActiveRecord::Base def all_commits_sha if persisted? merge_request_diffs.flat_map(&:commits_sha).uniq + elsif compare_commits + compare_commits.to_a.reverse.map(&:id) else - cached_commits = compare_commits.to_a.reverse.map(&:id) - cached_commits.any? ? cached_commits : [diff_head_sha] + [diff_head_sha] end end -- cgit v1.2.3