Merge branch '26155-merge-request-tabs-don-t-render-when-no-commits-available' into 'master'

Resolve "Merge request tabs don't render when no commits available"

Closes #26155 and #24556

See merge request !8347

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

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)

app/views/projects/merge_requests/show/_commits.html.haml

-%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

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}")

changelogs/unreleased/26155-merge-request-tabs-don-t-render-when-no-commits-available.yml

+---
+title: display merge request discussion tab for empty branches
+merge_request: 8347
+author:

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

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