summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJan Provaznik <jprovaznik@gitlab.com>2018-03-27 17:29:13 +0200
committerJan Provaznik <jprovaznik@gitlab.com>2018-03-27 17:29:13 +0200
commitcddad851bebcc03f31282ebeb4cbf3929de0a5bc (patch)
tree16ea26e98a360305c681f3439d02ef4311b078af
parentb81c0c5989d887d745a5f28b83a8da6f83638465 (diff)
downloadgitlab-ce-43246-checkfilter.tar.gz
-rw-r--r--app/controllers/dashboard_controller.rb11
-rw-r--r--changelogs/unreleased/43246-checkfilter.yml6
-rw-r--r--spec/features/atom/dashboard_issues_spec.rb17
-rw-r--r--spec/features/dashboard/issues_filter_spec.rb24
-rw-r--r--spec/features/dashboard/merge_requests_spec.rb8
-rw-r--r--spec/features/search/user_uses_header_search_field_spec.rb127
-rw-r--r--spec/helpers/issuables_helper_spec.rb29
7 files changed, 124 insertions, 98 deletions
diff --git a/app/controllers/dashboard_controller.rb b/app/controllers/dashboard_controller.rb
index d228956d8e3..68d328fa797 100644
--- a/app/controllers/dashboard_controller.rb
+++ b/app/controllers/dashboard_controller.rb
@@ -12,7 +12,7 @@ class DashboardController < Dashboard::ApplicationController
before_action :event_filter, only: :activity
before_action :projects, only: [:issues, :merge_requests]
before_action :set_show_full_reference, only: [:issues, :merge_requests]
- before_action :check_filters_presence, only: [:issues, :merge_requests]
+ before_action :check_filters_presence!, only: [:issues, :merge_requests]
respond_to :html
@@ -48,9 +48,14 @@ class DashboardController < Dashboard::ApplicationController
@show_full_reference = true
end
- def check_filters_presence
+ def check_filters_presence!
@no_filters_set = FILTER_PARAMS.none? { |k| params.key?(k) }
- render action: action_name if @no_filters_set
+ return unless @no_filters_set
+
+ respond_to do |format|
+ format.html
+ format.atom { head :bad_request }
+ end
end
end
diff --git a/changelogs/unreleased/43246-checkfilter.yml b/changelogs/unreleased/43246-checkfilter.yml
new file mode 100644
index 00000000000..e6c0e716213
--- /dev/null
+++ b/changelogs/unreleased/43246-checkfilter.yml
@@ -0,0 +1,6 @@
+---
+title: Require at least one filter when listing issues or merge requests on dashboard
+ page
+merge_request:
+author:
+type: performance
diff --git a/spec/features/atom/dashboard_issues_spec.rb b/spec/features/atom/dashboard_issues_spec.rb
index d673bac4995..fb6c71ce997 100644
--- a/spec/features/atom/dashboard_issues_spec.rb
+++ b/spec/features/atom/dashboard_issues_spec.rb
@@ -13,17 +13,26 @@ describe "Dashboard Issues Feed" do
end
describe "atom feed" do
- it "renders atom feed via personal access token" do
+ it "returns 400 if no filter is used" do
personal_access_token = create(:personal_access_token, user: user)
visit issues_dashboard_path(:atom, private_token: personal_access_token.token)
expect(response_headers['Content-Type']).to have_content('application/atom+xml')
+ expect(page.status_code).to eq(400)
+ end
+
+ it "renders atom feed via personal access token" do
+ personal_access_token = create(:personal_access_token, user: user)
+
+ visit issues_dashboard_path(:atom, private_token: personal_access_token.token, assignee_id: user.id)
+
+ expect(response_headers['Content-Type']).to have_content('application/atom+xml')
expect(body).to have_selector('title', text: "#{user.name} issues")
end
it "renders atom feed via RSS token" do
- visit issues_dashboard_path(:atom, rss_token: user.rss_token)
+ visit issues_dashboard_path(:atom, rss_token: user.rss_token, assignee_id: user.id)
expect(response_headers['Content-Type']).to have_content('application/atom+xml')
expect(body).to have_selector('title', text: "#{user.name} issues")
@@ -44,7 +53,7 @@ describe "Dashboard Issues Feed" do
let!(:issue2) { create(:issue, author: user, assignees: [assignee], project: project2, description: 'test desc') }
it "renders issue fields" do
- visit issues_dashboard_path(:atom, rss_token: user.rss_token)
+ visit issues_dashboard_path(:atom, rss_token: user.rss_token, assignee_id: assignee.id)
entry = find(:xpath, "//feed/entry[contains(summary/text(),'#{issue2.title}')]")
@@ -67,7 +76,7 @@ describe "Dashboard Issues Feed" do
end
it "renders issue label and milestone info" do
- visit issues_dashboard_path(:atom, rss_token: user.rss_token)
+ visit issues_dashboard_path(:atom, rss_token: user.rss_token, assignee_id: assignee.id)
entry = find(:xpath, "//feed/entry[contains(summary/text(),'#{issue1.title}')]")
diff --git a/spec/features/dashboard/issues_filter_spec.rb b/spec/features/dashboard/issues_filter_spec.rb
index 8759950e013..b277ac6a8ad 100644
--- a/spec/features/dashboard/issues_filter_spec.rb
+++ b/spec/features/dashboard/issues_filter_spec.rb
@@ -17,6 +17,12 @@ feature 'Dashboard Issues filtering', :js do
visit_issues
end
+ context 'without any filter' do
+ it 'shows error message' do
+ expect(page).to have_content 'The Issue Tracker is the place to add things'
+ end
+ end
+
context 'filtering by milestone' do
it 'shows all issues with no milestone' do
show_milestone_dropdown
@@ -27,15 +33,6 @@ feature 'Dashboard Issues filtering', :js do
expect(page).to have_selector('.issue', count: 1)
end
- it 'shows all issues with any milestone' do
- show_milestone_dropdown
-
- click_link 'Any Milestone'
-
- expect(page).to have_issuable_counts(open: 2, closed: 0, all: 2)
- expect(page).to have_selector('.issue', count: 2)
- end
-
it 'shows all issues with the selected milestone' do
show_milestone_dropdown
@@ -68,13 +65,6 @@ feature 'Dashboard Issues filtering', :js do
let(:label) { create(:label, project: project) }
let!(:label_link) { create(:label_link, label: label, target: issue) }
- it 'shows all issues without filter' do
- page.within 'ul.content-list' do
- expect(page).to have_content issue.title
- expect(page).to have_content issue2.title
- end
- end
-
it 'shows all issues with the selected label' do
page.within '.labels-filter' do
find('.dropdown').click
@@ -91,7 +81,7 @@ feature 'Dashboard Issues filtering', :js do
context 'sorting' do
it 'shows sorted issues' do
sorting_by('Created date')
- visit_issues
+ visit_issues(assignee_id: user.id)
expect(find('.issues-filters')).to have_content('Created date')
end
diff --git a/spec/features/dashboard/merge_requests_spec.rb b/spec/features/dashboard/merge_requests_spec.rb
index c8f3a8449f5..7f20fbe8aaf 100644
--- a/spec/features/dashboard/merge_requests_spec.rb
+++ b/spec/features/dashboard/merge_requests_spec.rb
@@ -103,15 +103,11 @@ feature 'Dashboard Merge Requests' do
expect(page).not_to have_content(other_merge_request.title)
end
- it 'shows all merge requests', :js do
+ it 'shows error message without filter', :js do
filter_item_select('Any Assignee', '.js-assignee-search')
filter_item_select('Any Author', '.js-author-search')
- expect(page).to have_content(authored_merge_request.title)
- expect(page).to have_content(authored_merge_request_from_fork.title)
- expect(page).to have_content(assigned_merge_request.title)
- expect(page).to have_content(assigned_merge_request_from_fork.title)
- expect(page).to have_content(other_merge_request.title)
+ expect(page).to have_content('There are no merge requests to show')
end
it 'shows sorted merge requests' do
diff --git a/spec/features/search/user_uses_header_search_field_spec.rb b/spec/features/search/user_uses_header_search_field_spec.rb
index 5ddea36add5..a9128104b87 100644
--- a/spec/features/search/user_uses_header_search_field_spec.rb
+++ b/spec/features/search/user_uses_header_search_field_spec.rb
@@ -9,49 +9,25 @@ describe 'User uses header search field' do
before do
project.add_reporter(user)
sign_in(user)
-
- visit(project_path(project))
- end
-
- it 'starts searching by pressing the enter key', :js do
- fill_in('search', with: 'gitlab')
- find('#search').native.send_keys(:enter)
-
- page.within('.breadcrumbs-sub-title') do
- expect(page).to have_content('Search')
- end
end
- it 'contains location badge' do
- expect(page).to have_selector('.has-location-badge')
- end
-
- context 'when clicking the search field', :js do
+ context 'when user is in a global scope', :js do
before do
+ visit(root_path)
page.find('#search').click
end
- it 'shows category search dropdown' do
- expect(page).to have_selector('.dropdown-header', text: /#{project.name}/i)
- end
-
context 'when clicking issues' do
- let!(:issue) { create(:issue, project: project, author: user, assignees: [user]) }
-
it 'shows assigned issues' do
- find('.dropdown-menu').click_link('Issues assigned to me')
+ find('.search-input-container .dropdown-menu').click_link('Issues assigned to me')
- expect(page).to have_selector('.filtered-search')
- expect_tokens([assignee_token(user.name)])
- expect_filtered_search_input_empty
+ expect(find('.js-assignee-search')).to have_content(user.name)
end
it 'shows created issues' do
- find('.dropdown-menu').click_link("Issues I've created")
+ find('.search-input-container .dropdown-menu').click_link("Issues I've created")
- expect(page).to have_selector('.filtered-search')
- expect_tokens([author_token(user.name)])
- expect_filtered_search_input_empty
+ expect(find('.js-author-search')).to have_content(user.name)
end
end
@@ -59,32 +35,97 @@ describe 'User uses header search field' do
let!(:merge_request) { create(:merge_request, source_project: project, author: user, assignee: user) }
it 'shows assigned merge requests' do
- find('.dropdown-menu').click_link('Merge requests assigned to me')
+ find('.search-input-container .dropdown-menu').click_link('Merge requests assigned to me')
- expect(page).to have_selector('.merge-requests-holder')
- expect_tokens([assignee_token(user.name)])
- expect_filtered_search_input_empty
+ expect(find('.js-assignee-search')).to have_content(user.name)
end
it 'shows created merge requests' do
- find('.dropdown-menu').click_link("Merge requests I've created")
+ find('.search-input-container .dropdown-menu').click_link("Merge requests I've created")
- expect(page).to have_selector('.merge-requests-holder')
- expect_tokens([author_token(user.name)])
- expect_filtered_search_input_empty
+ expect(find('.js-author-search')).to have_content(user.name)
end
end
end
- context 'when entering text into the search field', :js do
+ context 'when user is in a project scope' do
before do
- page.within('.search-input-wrap') do
- fill_in('search', with: project.name[0..3])
+ visit(project_path(project))
+ end
+
+ it 'starts searching by pressing the enter key', :js do
+ fill_in('search', with: 'gitlab')
+ find('#search').native.send_keys(:enter)
+
+ page.within('.breadcrumbs-sub-title') do
+ expect(page).to have_content('Search')
end
end
- it 'does not display the category search dropdown' do
- expect(page).not_to have_selector('.dropdown-header', text: /#{project.name}/i)
+ it 'contains location badge' do
+ expect(page).to have_selector('.has-location-badge')
+ end
+
+ context 'when clicking the search field', :js do
+ before do
+ page.find('#search').click
+ end
+
+ it 'shows category search dropdown' do
+ expect(page).to have_selector('.dropdown-header', text: /#{project.name}/i)
+ end
+
+ context 'when clicking issues' do
+ let!(:issue) { create(:issue, project: project, author: user, assignees: [user]) }
+
+ it 'shows assigned issues' do
+ find('.dropdown-menu').click_link('Issues assigned to me')
+
+ expect(page).to have_selector('.filtered-search')
+ expect_tokens([assignee_token(user.name)])
+ expect_filtered_search_input_empty
+ end
+
+ it 'shows created issues' do
+ find('.dropdown-menu').click_link("Issues I've created")
+
+ expect(page).to have_selector('.filtered-search')
+ expect_tokens([author_token(user.name)])
+ expect_filtered_search_input_empty
+ end
+ end
+
+ context 'when clicking merge requests' do
+ let!(:merge_request) { create(:merge_request, source_project: project, author: user, assignee: user) }
+
+ it 'shows assigned merge requests' do
+ find('.dropdown-menu').click_link('Merge requests assigned to me')
+
+ expect(page).to have_selector('.merge-requests-holder')
+ expect_tokens([assignee_token(user.name)])
+ expect_filtered_search_input_empty
+ end
+
+ it 'shows created merge requests' do
+ find('.dropdown-menu').click_link("Merge requests I've created")
+
+ expect(page).to have_selector('.merge-requests-holder')
+ expect_tokens([author_token(user.name)])
+ expect_filtered_search_input_empty
+ end
+ end
+ end
+
+ context 'when entering text into the search field', :js do
+ before do
+ page.within('.search-input-wrap') do
+ fill_in('search', with: project.name[0..3])
+ end
+ end
+
+ it 'does not display the category search dropdown' do
+ expect(page).not_to have_selector('.dropdown-header', text: /#{project.name}/i)
+ end
end
end
end
diff --git a/spec/helpers/issuables_helper_spec.rb b/spec/helpers/issuables_helper_spec.rb
index 2fecd1a3d27..4224cea4652 100644
--- a/spec/helpers/issuables_helper_spec.rb
+++ b/spec/helpers/issuables_helper_spec.rb
@@ -40,22 +40,22 @@ describe IssuablesHelper do
end
it 'returns "Open" when state is :opened' do
- expect(helper.issuables_state_counter_text(:issues, :opened))
+ expect(helper.issuables_state_counter_text(:issues, :opened, true))
.to eq('<span>Open</span> <span class="badge">42</span>')
end
it 'returns "Closed" when state is :closed' do
- expect(helper.issuables_state_counter_text(:issues, :closed))
+ expect(helper.issuables_state_counter_text(:issues, :closed, true))
.to eq('<span>Closed</span> <span class="badge">42</span>')
end
it 'returns "Merged" when state is :merged' do
- expect(helper.issuables_state_counter_text(:merge_requests, :merged))
+ expect(helper.issuables_state_counter_text(:merge_requests, :merged, true))
.to eq('<span>Merged</span> <span class="badge">42</span>')
end
it 'returns "All" when state is :all' do
- expect(helper.issuables_state_counter_text(:merge_requests, :all))
+ expect(helper.issuables_state_counter_text(:merge_requests, :all, true))
.to eq('<span>All</span> <span class="badge">42</span>')
end
end
@@ -101,27 +101,6 @@ describe IssuablesHelper do
end
end
- describe '#issuable_filter_present?' do
- it 'returns true when any key is present' do
- allow(helper).to receive(:params).and_return(
- ActionController::Parameters.new(milestone_title: 'Velit consectetur asperiores natus delectus.',
- project_id: 'gitlabhq',
- scope: 'all')
- )
-
- expect(helper.issuable_filter_present?).to be_truthy
- end
-
- it 'returns false when no key is present' do
- allow(helper).to receive(:params).and_return(
- ActionController::Parameters.new(project_id: 'gitlabhq',
- scope: 'all')
- )
-
- expect(helper.issuable_filter_present?).to be_falsey
- end
- end
-
describe '#updated_at_by' do
let(:user) { create(:user) }
let(:unedited_issuable) { create(:issue) }