From 7c5198219aacaf18bfc7e8c523dcfa15d013139a Mon Sep 17 00:00:00 2001 From: "Luke \"Jared\" Bennett" Date: Wed, 5 Apr 2017 12:43:03 +0000 Subject: [PATCH] MR empty state --- app/assets/stylesheets/framework/blocks.scss | 4 ++ .../groups/application_controller.rb | 1 + app/views/groups/merge_requests.html.haml | 30 ++++---- .../merge_requests/_merge_requests.html.haml | 8 +-- .../projects/merge_requests/index.html.haml | 25 ++++--- app/views/shared/_merge_requests.html.haml | 2 +- .../empty_states/_merge_requests.html.haml | 22 ++++++ .../empty_states/icons/_merge_requests.svg | 1 + ...er-empty-state-for-merge-requests-view.yml | 4 ++ features/dashboard/dashboard.feature | 3 +- features/project/issues/issues.feature | 6 +- features/steps/project/fork.rb | 3 +- .../steps/project/forked_merge_requests.rb | 2 +- features/steps/project/merge_requests.rb | 2 +- features/steps/shared/project.rb | 4 ++ .../features/dashboard/merge_requests_spec.rb | 32 +++++++++ spec/features/groups/empty_states_spec.rb | 70 +++++++++++++++++++ .../merge_requests/create_new_mr_spec.rb | 8 +-- .../projects/merge_requests/list_spec.rb | 24 +++++++ spec/features/search_spec.rb | 2 + 20 files changed, 213 insertions(+), 40 deletions(-) create mode 100644 app/views/shared/empty_states/_merge_requests.html.haml create mode 100644 app/views/shared/empty_states/icons/_merge_requests.svg create mode 100644 changelogs/unreleased/20841-getting-started-better-empty-state-for-merge-requests-view.yml create mode 100644 spec/features/dashboard/merge_requests_spec.rb create mode 100644 spec/features/groups/empty_states_spec.rb diff --git a/app/assets/stylesheets/framework/blocks.scss b/app/assets/stylesheets/framework/blocks.scss index 9a4129cdc8d..52425262925 100644 --- a/app/assets/stylesheets/framework/blocks.scss +++ b/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; } diff --git a/app/controllers/groups/application_controller.rb b/app/controllers/groups/application_controller.rb index c411c21bb80..8b69c18d689 100644 --- a/app/controllers/groups/application_controller.rb +++ b/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 diff --git a/app/views/groups/merge_requests.html.haml b/app/views/groups/merge_requests.html.haml index 6ad76d23df5..8fe0bd149f3 100644 --- a/app/views/groups/merge_requests.html.haml +++ b/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' diff --git a/app/views/projects/merge_requests/_merge_requests.html.haml b/app/views/projects/merge_requests/_merge_requests.html.haml index fe82f751f53..4e97f74dd6a 100644 --- a/app/views/projects/merge_requests/_merge_requests.html.haml +++ b/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" diff --git a/app/views/projects/merge_requests/index.html.haml b/app/views/projects/merge_requests/index.html.haml index 8a96c8dacf6..64f17ab34b1 100644 --- a/app/views/projects/merge_requests/index.html.haml +++ b/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) diff --git a/app/views/shared/_merge_requests.html.haml b/app/views/shared/_merge_requests.html.haml index b7982b7fe9b..eecbb32e90e 100644 --- a/app/views/shared/_merge_requests.html.haml +++ b/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' diff --git a/app/views/shared/empty_states/_merge_requests.html.haml b/app/views/shared/empty_states/_merge_requests.html.haml new file mode 100644 index 00000000000..7f2f99f3406 --- /dev/null +++ b/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. diff --git a/app/views/shared/empty_states/icons/_merge_requests.svg b/app/views/shared/empty_states/icons/_merge_requests.svg new file mode 100644 index 00000000000..e77f6319a95 --- /dev/null +++ b/app/views/shared/empty_states/icons/_merge_requests.svg @@ -0,0 +1 @@ + diff --git a/changelogs/unreleased/20841-getting-started-better-empty-state-for-merge-requests-view.yml b/changelogs/unreleased/20841-getting-started-better-empty-state-for-merge-requests-view.yml new file mode 100644 index 00000000000..34909c06df3 --- /dev/null +++ b/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: diff --git a/features/dashboard/dashboard.feature b/features/dashboard/dashboard.feature index b1d5e4a7acb..1af4d46dec9 100644 --- a/features/dashboard/dashboard.feature +++ b/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" diff --git a/features/project/issues/issues.feature b/features/project/issues/issues.feature index b2b4fe72220..27fa67c1843 100644 --- a/features/project/issues/issues.feature +++ b/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" diff --git a/features/steps/project/fork.rb b/features/steps/project/fork.rb index 79db9728227..7591e7d5612 100644 --- a/features/steps/project/fork.rb +++ b/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 diff --git a/features/steps/project/forked_merge_requests.rb b/features/steps/project/forked_merge_requests.rb index c0827ff8fc7..ef1bb453615 100644 --- a/features/steps/project/forked_merge_requests.rb +++ b/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 diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb index c9c4f537fad..5510c65265a 100644 --- a/features/steps/project/merge_requests.rb +++ b/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 diff --git a/features/steps/shared/project.rb b/features/steps/shared/project.rb index 345a28f27dc..782009a32a7 100644 --- a/features/steps/shared/project.rb +++ b/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) diff --git a/spec/features/dashboard/merge_requests_spec.rb b/spec/features/dashboard/merge_requests_spec.rb new file mode 100644 index 00000000000..508ca38d7e5 --- /dev/null +++ b/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 diff --git a/spec/features/groups/empty_states_spec.rb b/spec/features/groups/empty_states_spec.rb new file mode 100644 index 00000000000..fef8e41bffe --- /dev/null +++ b/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 diff --git a/spec/features/merge_requests/create_new_mr_spec.rb b/spec/features/merge_requests/create_new_mr_spec.rb index f1ad4a55246..f36781167fb 100644 --- a/spec/features/merge_requests/create_new_mr_spec.rb +++ b/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') diff --git a/spec/features/projects/merge_requests/list_spec.rb b/spec/features/projects/merge_requests/list_spec.rb index 5dd58ad66a7..7e8a796c55d 100644 --- a/spec/features/projects/merge_requests/list_spec.rb +++ b/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 diff --git a/spec/features/search_spec.rb b/spec/features/search_spec.rb index 40ef4c098b9..e8ad28a00f0 100644 --- a/spec/features/search_spec.rb +++ b/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