From d9a2093e7e6b0d532131b18dc57c240f5b4a7c55 Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Mon, 28 Nov 2016 12:03:53 +0100 Subject: [PATCH] Prevent error when submitting a merge request and pipeline is not defined --- .../projects/merge_requests_controller.rb | 2 +- app/models/merge_request.rb | 2 +- .../merge_requests/_new_submit.html.haml | 8 +++-- .../projects/merge_requests/_show.html.haml | 5 +-- ...undefined-method-size-for-nil-nilclass.yml | 4 +++ .../_new_submit.html.haml_spec.rb | 31 +++++++++++++++++++ 6 files changed, 45 insertions(+), 7 deletions(-) create mode 100644 changelogs/unreleased/24860-actionview-template-error-undefined-method-size-for-nil-nilclass.yml create mode 100644 spec/views/projects/merge_requests/_new_submit.html.haml_spec.rb diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index e24a670631f..a2225cc8343 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -564,7 +564,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController def define_pipelines_vars @pipelines = @merge_request.all_pipelines @pipeline = @merge_request.pipeline - @statuses = @pipeline.statuses.relevant if @pipeline.present? + @statuses_count = @pipeline.present? ? @pipeline.statuses.relevant.count : 0 end def define_new_vars diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 69c6aa700d6..38d8c15e6b0 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -781,7 +781,7 @@ class MergeRequest < ActiveRecord::Base end def all_pipelines - return unless source_project + return Ci::Pipeline.none unless source_project @all_pipelines ||= source_project.pipelines .where(sha: all_commits_sha, ref: source_branch) diff --git a/app/views/projects/merge_requests/_new_submit.html.haml b/app/views/projects/merge_requests/_new_submit.html.haml index 9c6f562f7db..4a08ed045f4 100644 --- a/app/views/projects/merge_requests/_new_submit.html.haml +++ b/app/views/projects/merge_requests/_new_submit.html.haml @@ -34,10 +34,11 @@ = link_to url_for(params), data: {target: 'div#pipelines', action: 'pipelines', toggle: 'tab'} do Pipelines %span.badge= @pipelines.size + - if @pipeline.present? %li.builds-tab = link_to url_for(params), data: {target: 'div#builds', action: 'builds', toggle: 'tab'} do Builds - %span.badge= @statuses.size + %span.badge= @statuses_count %li.diffs-tab = link_to url_for(params.merge(action: 'new_diffs')), data: {target: 'div#diffs', action: 'new/diffs', toggle: 'tab'} do Changes @@ -48,9 +49,10 @@ = render "projects/merge_requests/show/commits" #diffs.diffs.tab-pane - # This tab is always loaded via AJAX - - if @pipelines.any? + - if @pipeline.present? #builds.builds.tab-pane = render "projects/merge_requests/show/builds" + - if @pipelines.any? #pipelines.pipelines.tab-pane = render "projects/merge_requests/show/pipelines" @@ -65,5 +67,5 @@ :javascript var merge_request = new MergeRequest({ action: "#{(@show_changes_tab ? 'new/diffs' : 'new')}", - buildsLoaded: "#{@pipelines.any? ? 'true' : 'false'}" + buildsLoaded: "#{@pipeline.present? ? 'true' : 'false'}" }); diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index a497f418c7c..0e2975bd551 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -59,15 +59,16 @@ = link_to commits_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#commits', action: 'commits', toggle: 'tab' } do Commits %span.badge= @commits_count - - if @pipeline + - if @pipelines.any? %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= @pipelines.size + - if @pipeline.present? %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 - %span.badge= @statuses.size + %span.badge= @statuses_count %li.diffs-tab = link_to diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#diffs', action: 'diffs', toggle: 'tab' } do Changes diff --git a/changelogs/unreleased/24860-actionview-template-error-undefined-method-size-for-nil-nilclass.yml b/changelogs/unreleased/24860-actionview-template-error-undefined-method-size-for-nil-nilclass.yml new file mode 100644 index 00000000000..4b4aea79380 --- /dev/null +++ b/changelogs/unreleased/24860-actionview-template-error-undefined-method-size-for-nil-nilclass.yml @@ -0,0 +1,4 @@ +--- +title: Prevent error when submitting a merge request and pipeline is not defined +merge_request: 7707 +author: diff --git a/spec/views/projects/merge_requests/_new_submit.html.haml_spec.rb b/spec/views/projects/merge_requests/_new_submit.html.haml_spec.rb new file mode 100644 index 00000000000..4f698a34ab5 --- /dev/null +++ b/spec/views/projects/merge_requests/_new_submit.html.haml_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe 'projects/merge_requests/_new_submit.html.haml', :view do + let(:merge_request) { create(:merge_request) } + let!(:pipeline) { create(:ci_empty_pipeline) } + + before do + controller.prepend_view_path('app/views/projects') + + assign(:merge_request, merge_request) + assign(:commits, merge_request.commits) + assign(:project, merge_request.target_project) + + allow(view).to receive(:can?).and_return(true) + allow(view).to receive(:url_for).and_return('#') + allow(view).to receive(:current_user).and_return(merge_request.author) + end + + context 'when there are pipelines for merge request but no pipeline for last commit' do + before do + assign(:pipelines, Ci::Pipeline.all) + assign(:pipeline, nil) + end + + it 'shows <> tab and hides <> tab' do + render + expect(rendered).to have_text('Pipelines 1') + expect(rendered).not_to have_text('Builds') + end + end +end