diff options
7 files changed, 80 insertions, 57 deletions
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 926944bc3b3..70005a87f4b 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -221,7 +221,7 @@ class MergeRequest < ActiveRecord::Base # true base commit, so we can't simply have `#diff_base_commit` fall back on # this method. def likely_diff_base_commit - first_commit.parent || first_commit + first_commit.try(:parent) || first_commit end def diff_start_commit diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index d9a3220b002..1f63803c24e 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -50,56 +50,55 @@ .content-block.content-block-small = render 'award_emoji/awards_block', awardable: @merge_request, inline: true - - if @commits_count.nonzero? - .merge-request-tabs-holder{ class: ("js-tabs-affix" unless ENV['RAILS_ENV'] == 'test') } - %div{ class: container_class } - %ul.merge-request-tabs.nav-links.no-top.no-bottom - %li.notes-tab - = link_to namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#notes', action: 'notes', toggle: 'tab' } do - Discussion - %span.badge= @merge_request.related_notes.user.count - - if @merge_request.source_project - %li.commits-tab - = 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 @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 - %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 - %span.badge= @merge_request.diff_size - %li#resolve-count-app.line-resolve-all-container.pull-right.prepend-top-10.hidden-xs{ "v-cloak" => true } - %resolve-count{ "inline-template" => true, ":logged-out" => "#{current_user.nil?}" } - %div - .line-resolve-all{ "v-show" => "discussionCount > 0", - ":class" => "{ 'has-next-btn': !loggedOut && resolvedDiscussionCount !== discussionCount }" } - %span.line-resolve-btn.is-disabled{ type: "button", - ":class" => "{ 'is-active': resolvedDiscussionCount === discussionCount }" } - = render "shared/icons/icon_status_success.svg" - %span.line-resolve-text - {{ resolvedDiscussionCount }}/{{ discussionCount }} {{ resolvedCountText }} resolved - = render "discussions/jump_to_next" + .merge-request-tabs-holder{ class: ("js-tabs-affix" unless ENV['RAILS_ENV'] == 'test') } + %div{ class: container_class } + %ul.merge-request-tabs.nav-links.no-top.no-bottom + %li.notes-tab + = link_to namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#notes', action: 'notes', toggle: 'tab' } do + Discussion + %span.badge= @merge_request.related_notes.user.count + - if @merge_request.source_project + %li.commits-tab + = 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 @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 + %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 + %span.badge= @merge_request.diff_size + %li#resolve-count-app.line-resolve-all-container.pull-right.prepend-top-10.hidden-xs{ "v-cloak" => true } + %resolve-count{ "inline-template" => true, ":logged-out" => "#{current_user.nil?}" } + %div + .line-resolve-all{ "v-show" => "discussionCount > 0", + ":class" => "{ 'has-next-btn': !loggedOut && resolvedDiscussionCount !== discussionCount }" } + %span.line-resolve-btn.is-disabled{ type: "button", + ":class" => "{ 'is-active': resolvedDiscussionCount === discussionCount }" } + = render "shared/icons/icon_status_success.svg" + %span.line-resolve-text + {{ resolvedDiscussionCount }}/{{ discussionCount }} {{ resolvedCountText }} resolved + = render "discussions/jump_to_next" - .tab-content#diff-notes-app - #notes.notes.tab-pane.voting_notes - .row - %section.col-md-12 - .issuable-discussion - = render "projects/merge_requests/discussion" + .tab-content#diff-notes-app + #notes.notes.tab-pane.voting_notes + .row + %section.col-md-12 + .issuable-discussion + = render "projects/merge_requests/discussion" - #commits.commits.tab-pane - - # This tab is always loaded via AJAX - #pipelines.pipelines.tab-pane - - # This tab is always loaded via AJAX - #diffs.diffs.tab-pane - - # This tab is always loaded via AJAX + #commits.commits.tab-pane + - # This tab is always loaded via AJAX + #pipelines.pipelines.tab-pane + - # This tab is always loaded via AJAX + #diffs.diffs.tab-pane + - # This tab is always loaded via AJAX - .mr-loading-status - = spinner + .mr-loading-status + = spinner = render 'shared/issuable/sidebar', issuable: @merge_request - if @merge_request.can_be_reverted?(current_user) diff --git a/app/views/projects/merge_requests/show/_commits.html.haml b/app/views/projects/merge_requests/show/_commits.html.haml index baa1ade5eee..11793919ff7 100644 --- a/app/views/projects/merge_requests/show/_commits.html.haml +++ b/app/views/projects/merge_requests/show/_commits.html.haml @@ -1,2 +1,8 @@ -%ol#commits-list.list-unstyled - = render "projects/commits/commits", project: @merge_request.source_project, ref: @merge_request.source_branch +- if @commits.empty? + .commits-empty + %h4 + There are no commits yet. + = custom_icon ('illustration_no_commits') +- else + %ol#commits-list.list-unstyled + = render "projects/commits/commits", project: @merge_request.source_project, ref: @merge_request.source_branch diff --git a/app/views/projects/notes/_notes_with_form.html.haml b/app/views/projects/notes/_notes_with_form.html.haml index f01bcfac898..fbd2bff5bbb 100644 --- a/app/views/projects/notes/_notes_with_form.html.haml +++ b/app/views/projects/notes/_notes_with_form.html.haml @@ -23,4 +23,4 @@ to post a comment :javascript - var notes = new Notes("#{namespace_project_notes_path(namespace_id: @project.namespace, target_id: @noteable.id, target_type: @noteable.class.name.underscore)}", #{@notes.map(&:id).to_json}, #{Time.now.to_i}, "#{diff_view}") + var notes = new Notes("#{namespace_project_notes_path(namespace_id: @project.namespace, project_id: @project, target_id: @noteable.id, target_type: @noteable.class.name.underscore)}", #{@notes.map(&:id).to_json}, #{Time.now.to_i}, "#{diff_view}") diff --git a/changelogs/unreleased/26155-merge-request-tabs-don-t-render-when-no-commits-available.yml b/changelogs/unreleased/26155-merge-request-tabs-don-t-render-when-no-commits-available.yml new file mode 100644 index 00000000000..242a77b5d48 --- /dev/null +++ b/changelogs/unreleased/26155-merge-request-tabs-don-t-render-when-no-commits-available.yml @@ -0,0 +1,4 @@ +--- +title: display merge request discussion tab for empty branches +merge_request: 8347 +author: diff --git a/spec/features/merge_requests/deleted_source_branch_spec.rb b/spec/features/merge_requests/deleted_source_branch_spec.rb index d5c9ed8a3b7..0952b17b63e 100644 --- a/spec/features/merge_requests/deleted_source_branch_spec.rb +++ b/spec/features/merge_requests/deleted_source_branch_spec.rb @@ -4,6 +4,8 @@ require 'spec_helper' # message to be shown by JavaScript when the source branch was deleted. # Please do not remove "js: true". describe 'Deleted source branch', feature: true, js: true do + include WaitForAjax + let(:user) { create(:user) } let(:merge_request) { create(:merge_request) } @@ -13,7 +15,8 @@ describe 'Deleted source branch', feature: true, js: true do merge_request.update!(source_branch: 'this-branch-does-not-exist') visit namespace_project_merge_request_path( merge_request.project.namespace, - merge_request.project, merge_request + merge_request.project, + merge_request ) end @@ -23,11 +26,17 @@ describe 'Deleted source branch', feature: true, js: true do ) end - it 'hides Discussion, Commits and Changes tabs' do + it 'still contains Discussion, Commits and Changes tabs' do within '.merge-request-details' do - expect(page).to have_no_content('Discussion') - expect(page).to have_no_content('Commits') - expect(page).to have_no_content('Changes') + expect(page).to have_content('Discussion') + expect(page).to have_content('Commits') + expect(page).to have_content('Changes') end + + click_on 'Changes' + wait_for_ajax + + expect(page).to have_selector('.diffs.tab-pane .nothing-here-block') + expect(page).to have_content('Nothing to merge from this-branch-does-not-exist into feature') end end diff --git a/spec/views/projects/merge_requests/show.html.haml_spec.rb b/spec/views/projects/merge_requests/show.html.haml_spec.rb index 33cabd14913..7f123b15194 100644 --- a/spec/views/projects/merge_requests/show.html.haml_spec.rb +++ b/spec/views/projects/merge_requests/show.html.haml_spec.rb @@ -7,6 +7,7 @@ describe 'projects/merge_requests/show.html.haml' do let(:project) { create(:project) } let(:fork_project) { create(:project, forked_from_project: project) } let(:unlink_project) { Projects::UnlinkForkService.new(fork_project, user) } + let(:note) { create(:note_on_merge_request, project: project, noteable: closed_merge_request) } let(:closed_merge_request) do create(:closed_merge_request, @@ -19,8 +20,12 @@ describe 'projects/merge_requests/show.html.haml' do assign(:project, project) assign(:merge_request, closed_merge_request) assign(:commits_count, 0) + assign(:note, note) + assign(:noteable, closed_merge_request) + assign(:notes, []) + assign(:pipelines, Ci::Pipeline.none) - allow(view).to receive(:can?).and_return(true) + allow(view).to receive_messages(current_user: user, can?: true) end context 'when the merge request is closed' do |