Merge branch '20841-getting-started-better-empty-state-for-merge-requests-view' into 'master'

MR empty state

Closes #20854 and #20841

See merge request !7342

app/assets/stylesheets/framework/blocks.scss

@@ -292,6 +292,10 @@
   }
 
   @media(min-width: $screen-xs-max) {
+    &.merge-requests .text-content {
+      margin-top: 40px;
+    }
+
     &.labels .text-content {
       margin-top: 70px;
     }

app/controllers/groups/application_controller.rb

@@ -10,6 +10,7 @@ class Groups::ApplicationController < ApplicationController
     unless @group
       id = params[:group_id] || params[:id]
       @group = Group.find_by_full_path(id)
+      @group_merge_requests = MergeRequestsFinder.new(current_user, group_id: @group.id).execute
 
       unless @group && can?(current_user, :read_group, @group)
         @group = nil

app/views/groups/merge_requests.html.haml

@@ -1,18 +1,22 @@
 - page_title "Merge Requests"
 
-.top-area
-  = render 'shared/issuable/nav', type: :merge_requests
-  - if current_user
-    .nav-controls
-      = render 'shared/new_project_item_select', path: 'merge_requests/new', label: "New Merge Request"
+- if @group_merge_requests.empty?
+  = render 'shared/empty_states/merge_requests', project_select_button: true
+- else
+  .top-area
+    = render 'shared/issuable/nav', type: :merge_requests
+    - if current_user
+      .nav-controls
+        = render 'shared/new_project_item_select', path: 'merge_requests/new', label: "New merge request"
 
-= render 'shared/issuable/filter', type: :merge_requests
+  = render 'shared/issuable/filter', type: :merge_requests
 
-.row-content-block.second-block
-  Only merge requests from
-  %strong= @group.name
-  group are listed here.
-  - if current_user
-    To see all merge requests you should visit #{link_to 'dashboard', merge_requests_dashboard_path} page.
+  .row-content-block.second-block
+    Only merge requests from
+    %strong= @group.name
+    group are listed here.
+    - if current_user
+      To see all merge requests you should visit #{link_to 'dashboard', merge_requests_dashboard_path} page.
 
-= render 'shared/merge_requests'
+  .prepend-top-default
+    = render 'shared/merge_requests'

app/views/projects/merge_requests/_merge_requests.html.haml

@@ -1,8 +1,8 @@
 %ul.content-list.mr-list.issuable-list
-  = render @merge_requests
-  - if @merge_requests.blank?
-    %li
-      .nothing-here-block No merge requests to show
+  - if @merge_requests.exists?
+    = render @merge_requests
+  - else
+    = render 'shared/empty_states/merge_requests'
 
 - if @merge_requests.present?
   = paginate @merge_requests, theme: "gitlab"

app/views/projects/merge_requests/index.html.haml

@@ -7,16 +7,19 @@
 - content_for :page_specific_javascripts do
   = page_specific_javascript_bundle_tag('filtered_search')
 
-%div{ class: container_class }
-  .top-area
-    = render 'shared/issuable/nav', type: :merge_requests
-    .nav-controls
-      - merge_project = can?(current_user, :create_merge_request, @project) ? @project : (current_user && current_user.fork_of(@project))
-      - if merge_project
-        = link_to new_namespace_project_merge_request_path(merge_project.namespace, merge_project), class: "btn btn-new", title: "New Merge Request" do
-          New Merge Request
+- if @project.merge_requests.exists?
+  %div{ class: container_class }
+    .top-area
+      = render 'shared/issuable/nav', type: :merge_requests
+      .nav-controls
+        - merge_project = can?(current_user, :create_merge_request, @project) ? @project : (current_user && current_user.fork_of(@project))
+        - if merge_project
+          = link_to new_namespace_project_merge_request_path(merge_project.namespace, merge_project), class: "btn btn-new", title: "New merge request" do
+            New merge request
 
-  = render 'shared/issuable/search_bar', type: :merge_requests
+    = render 'shared/issuable/search_bar', type: :merge_requests
 
-  .merge-requests-holder
-    = render 'merge_requests'
+    .merge-requests-holder
+      = render 'merge_requests'
+- else
+  = render 'shared/empty_states/merge_requests', button_path: new_namespace_project_merge_request_path(@project.namespace, @project)

app/views/shared/_merge_requests.html.haml

@@ -6,4 +6,4 @@
   = paginate @merge_requests, theme: "gitlab"
 
 - else
-  .nothing-here-block No merge requests to show
+  = render 'shared/empty_states/merge_requests'

app/views/shared/empty_states/_merge_requests.html.haml

@@ -0,0 +1,22 @@
+- button_path = local_assigns.fetch(:button_path, false)
+- project_select_button = local_assigns.fetch(:project_select_button, false)
+- has_button = button_path || project_select_button
+
+.row.empty-state.merge-requests
+  .col-xs-12{ class: "#{'col-sm-6 pull-right' if has_button}" }
+    .svg-content
+      = render 'shared/empty_states/icons/merge_requests.svg'
+  .col-xs-12{ class: "#{'col-sm-6' if has_button}" }
+    .text-content
+      - if has_button
+        %h4
+          Merge requests are a place to propose changes you've made to a project and discuss those changes with others.
+        %p
+          Interested parties can even contribute by pushing commits if they want to.
+        - if project_select_button
+          = render 'shared/new_project_item_select', path: 'merge_requests/new', label: 'New merge request'
+        - else
+          = link_to 'New merge request', button_path, class: 'btn btn-new', title: 'New merge request', id: 'new_merge_request_link'
+      - else
+        %h4.text-center
+          There are no merge requests to show.

changelogs/unreleased/20841-getting-started-better-empty-state-for-merge-requests-view.yml

@@ -0,0 +1,4 @@
+---
+title: Added merge requests empty state
+merge_request: 7342
+author: 

features/dashboard/dashboard.feature

@@ -63,7 +63,8 @@ Feature: Dashboard
 
   @javascript
   Scenario: Visiting Project's merge requests after sorting
-    Given I visit dashboard merge requests page
+    Given project "Shop" has a "Bugfix MR" merge request open
+    And I visit dashboard merge requests page
     And I sort the list by "Oldest updated"
     And I visit project "Shop" merge requests page
     Then The list should be sorted by "Oldest updated"

features/project/issues/issues.feature

@@ -56,14 +56,16 @@ Feature: Project Issues
 
   @javascript
   Scenario: Visiting Merge Requests after being sorted the list
-    Given I visit project "Shop" issues page
+    Given project "Shop" has a "Bugfix MR" merge request open
+    And I visit project "Shop" issues page
     And I sort the list by "Oldest updated"
     And I visit project "Shop" merge requests page
     Then The list should be sorted by "Oldest updated"
 
   @javascript
   Scenario: Visiting Merge Requests from a differente Project after sorting
-    Given I visit project "Shop" merge requests page
+    Given project "Shop" has a "Bugfix MR" merge request open
+    And I visit project "Shop" merge requests page
     And I sort the list by "Oldest updated"
     And I visit dashboard merge requests page
     Then The list should be sorted by "Oldest updated"

features/steps/project/fork.rb

@@ -42,8 +42,7 @@ class Spinach::Features::ProjectFork < Spinach::FeatureSteps
   end
 
   step 'I click link "New merge request"' do
-    expect(page).to have_content(/new merge request/i)
-    click_link "New Merge Request"
+    page.has_link?('New Merge Request') ? click_link("New Merge Request") : click_link('New merge request')
   end
 
   step 'I should see the new merge request page for my namespace' do

features/steps/project/forked_merge_requests.rb

@@ -16,7 +16,7 @@ class Spinach::Features::ProjectForkedMergeRequests < Spinach::FeatureSteps
   end
 
   step 'I click link "New Merge Request"' do
-    click_link "New Merge Request"
+    page.has_link?('New Merge Request') ? click_link("New Merge Request") : click_link('New merge request')
   end
 
   step 'I should see merge request "Merge Request On Forked Project"' do

features/steps/project/merge_requests.rb

@@ -14,7 +14,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
   end
 
   step 'I click link "New Merge Request"' do
-    click_link "New Merge Request"
+    page.has_link?('New Merge Request') ? click_link("New Merge Request") : click_link('New merge request')
   end
 
   step 'I click link "Bug NS-04"' do

features/steps/shared/project.rb

@@ -273,6 +273,10 @@ module SharedProject
     @project.update(public_builds: false)
   end
 
+  step 'project "Shop" has a "Bugfix MR" merge request open' do
+    create(:merge_request, title: "Bugfix MR", target_project: project, source_project: project, author: project.users.first)
+  end
+
   def user_owns_project(user_name:, project_name:, visibility: :private)
     user = user_exists(user_name, username: user_name.gsub(/\s/, '').underscore)
     project = Project.find_by(name: project_name)

spec/features/dashboard/merge_requests_spec.rb

@@ -0,0 +1,32 @@
+require 'spec_helper'
+
+describe 'Dashboard Merge Requests' do
+  let(:current_user) { create :user }
+  let(:project) do
+    create(:empty_project) do |project|
+      project.add_master(current_user)
+    end
+  end
+
+  before do
+    login_as(current_user)
+  end
+
+  it 'should show an empty state' do
+    visit merge_requests_dashboard_path(assignee_id: current_user.id)
+
+    expect(page).to have_selector('.empty-state')
+  end
+
+  context 'if there are merge requests' do
+    before do
+      create(:merge_request, assignee: current_user, source_project: project)
+
+      visit merge_requests_dashboard_path(assignee_id: current_user.id)
+    end
+
+    it 'should not show an empty state' do
+      expect(page).not_to have_selector('.empty-state')
+    end
+  end
+end

spec/features/groups/empty_states_spec.rb

@@ -0,0 +1,70 @@
+require 'spec_helper'
+
+feature 'Groups Merge Requests Empty States' do
+  let(:group) { create(:group) }
+  let(:user) { create(:group_member, :developer, user: create(:user), group: group ).user }
+
+  before do
+    login_as(user)
+  end
+
+  context 'group has a project' do
+    let(:project) { create(:empty_project, namespace: group) }
+
+    before do
+      project.add_master(user)
+    end
+
+    context 'the project has a merge request' do
+      before do
+        create(:merge_request, source_project: project)
+
+        visit merge_requests_group_path(group)
+      end
+
+      it 'should not display an empty state' do
+        expect(page).not_to have_selector('.empty-state')
+      end
+    end
+
+    context 'the project has no merge requests', :js do
+      before do
+        visit merge_requests_group_path(group)
+      end
+
+      it 'should display an empty state' do
+        expect(page).to have_selector('.empty-state')
+      end
+
+      it 'should show a new merge request button' do
+        within '.empty-state' do
+          expect(page).to have_content('New merge request')
+        end
+      end
+
+      it 'the new merge request button opens a project dropdown' do
+        within '.empty-state' do
+          find('.new-project-item-select-button').click
+        end
+
+        expect(page).to have_selector('.ajax-project-dropdown')
+      end
+    end
+  end
+
+  context 'group without a project' do
+    before do
+      visit merge_requests_group_path(group)
+    end
+
+    it 'should display an empty state' do
+      expect(page).to have_selector('.empty-state')
+    end
+
+    it 'should not show a new merge request button' do
+      within '.empty-state' do
+        expect(page).not_to have_link('New merge request')
+      end
+    end
+  end
+end

spec/features/merge_requests/create_new_mr_spec.rb

@@ -15,7 +15,7 @@ feature 'Create New Merge Request', feature: true, js: true do
   it 'selects the source branch sha when a tag with the same name exists' do
     visit namespace_project_merge_requests_path(project.namespace, project)
 
-    click_link 'New Merge Request'
+    click_link 'New merge request'
     expect(page).to have_content('Source branch')
     expect(page).to have_content('Target branch')
 
@@ -27,8 +27,8 @@ feature 'Create New Merge Request', feature: true, js: true do
 
   it 'selects the target branch sha when a tag with the same name exists' do
     visit namespace_project_merge_requests_path(project.namespace, project)
-    
-    click_link 'New Merge Request'
+
+    click_link 'New merge request'
 
     expect(page).to have_content('Source branch')
     expect(page).to have_content('Target branch')
@@ -42,7 +42,7 @@ feature 'Create New Merge Request', feature: true, js: true do
   it 'generates a diff for an orphaned branch' do
     visit namespace_project_merge_requests_path(project.namespace, project)
 
-    click_link 'New Merge Request'
+    page.has_link?('New Merge Request') ? click_link("New Merge Request") : click_link('New merge request')
     expect(page).to have_content('Source branch')
     expect(page).to have_content('Target branch')
 

spec/features/projects/merge_requests/list_spec.rb

@@ -17,4 +17,28 @@ feature 'Merge Requests List' do
 
     expect(page).not_to have_selector('.js-new-board-list')
   end
+
+  it 'should show an empty state' do
+    visit namespace_project_merge_requests_path(project.namespace, project)
+
+    expect(page).to have_selector('.empty-state')
+  end
+
+  it 'empty state should have a create merge request button' do
+    visit namespace_project_merge_requests_path(project.namespace, project)
+
+    expect(page).to have_link 'New merge request', href: new_namespace_project_merge_request_path(project.namespace, project)
+  end
+
+  context 'if there are merge requests' do
+    before do
+      create(:merge_request, assignee: user, source_project: project)
+
+      visit namespace_project_merge_requests_path(project.namespace, project)
+    end
+
+    it 'should not show an empty state' do
+      expect(page).not_to have_selector('.empty-state')
+    end
+  end
 end

spec/features/search_spec.rb

@@ -164,6 +164,8 @@ describe "Search", feature: true  do
       end
 
       context 'click the links in the category search dropdown', js: true do
+        let!(:merge_request) { create(:merge_request, source_project: project, author: user, assignee: user) }
+
         before do
           page.find('#search').click
         end