summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJan Provaznik <jprovaznik@gitlab.com>2018-03-23 15:27:15 +0100
committerWinnie Hellmann <winnie@gitlab.com>2018-04-03 20:19:09 +0200
commitc1b71e2fa1e49da82b15ee7f12148a372face09c (patch)
treec88dbf140e95c5d8f3cd6ea201e058a4cd1a1c0b
parent59a158955e1ae09420ad05e53782e0dbc512e9c8 (diff)
downloadgitlab-ce-c1b71e2fa1e49da82b15ee7f12148a372face09c.tar.gz
Check if at least one filter is set on dashboard
When listing issues and merge requests on dasboard page, make sure that at least one filter is enabled. User's id is used in search autocomplete widget instead of username, which allows presetting user in filter dropdowns. Related to #43246
-rw-r--r--app/assets/javascripts/search_autocomplete.js8
-rw-r--r--app/controllers/dashboard_controller.rb14
-rw-r--r--app/helpers/application_helper.rb2
-rw-r--r--app/helpers/issuables_helper.rb24
-rw-r--r--app/views/dashboard/issues.html.haml2
-rw-r--r--app/views/dashboard/merge_requests.html.haml2
-rw-r--r--app/views/shared/issuable/_filter.html.haml4
-rw-r--r--app/views/shared/issuable/_nav.html.haml11
-rw-r--r--spec/controllers/dashboard_controller_spec.rb2
-rw-r--r--spec/features/dashboard/issues_spec.rb9
-rw-r--r--spec/support/issuables_list_metadata_shared_examples.rb8
-rw-r--r--spec/support/issuables_requiring_filter_shared_examples.rb15
12 files changed, 53 insertions, 48 deletions
diff --git a/app/assets/javascripts/search_autocomplete.js b/app/assets/javascripts/search_autocomplete.js
index 7dd3e9858c6..2da022fde63 100644
--- a/app/assets/javascripts/search_autocomplete.js
+++ b/app/assets/javascripts/search_autocomplete.js
@@ -233,21 +233,21 @@ export default class SearchAutocomplete {
const issueItems = [
{
text: 'Issues assigned to me',
- url: `${issuesPath}/?assignee_username=${userName}`,
+ url: `${issuesPath}/?assignee_id=${userId}`,
},
{
text: "Issues I've created",
- url: `${issuesPath}/?author_username=${userName}`,
+ url: `${issuesPath}/?author_id=${userId}`,
},
];
const mergeRequestItems = [
{
text: 'Merge requests assigned to me',
- url: `${mrPath}/?assignee_username=${userName}`,
+ url: `${mrPath}/?assignee_id=${userId}`,
},
{
text: "Merge requests I've created",
- url: `${mrPath}/?author_username=${userName}`,
+ url: `${mrPath}/?author_id=${userId}`,
},
];
diff --git a/app/controllers/dashboard_controller.rb b/app/controllers/dashboard_controller.rb
index 280ed93faf8..d228956d8e3 100644
--- a/app/controllers/dashboard_controller.rb
+++ b/app/controllers/dashboard_controller.rb
@@ -2,9 +2,17 @@ class DashboardController < Dashboard::ApplicationController
include IssuesAction
include MergeRequestsAction
+ FILTER_PARAMS = [
+ :author_id,
+ :assignee_id,
+ :milestone_title,
+ :label_name
+ ].freeze
+
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]
respond_to :html
@@ -39,4 +47,10 @@ class DashboardController < Dashboard::ApplicationController
def set_show_full_reference
@show_full_reference = true
end
+
+ def check_filters_presence
+ @no_filters_set = FILTER_PARAMS.none? { |k| params.key?(k) }
+
+ render action: action_name if @no_filters_set
+ end
end
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 86ec500ceb3..228c8d2e8f9 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -228,9 +228,7 @@ module ApplicationHelper
scope: params[:scope],
milestone_title: params[:milestone_title],
assignee_id: params[:assignee_id],
- assignee_username: params[:assignee_username],
author_id: params[:author_id],
- author_username: params[:author_username],
search: params[:search],
label_name: params[:label_name]
}
diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb
index 6d6b840f485..06c3e569c84 100644
--- a/app/helpers/issuables_helper.rb
+++ b/app/helpers/issuables_helper.rb
@@ -159,16 +159,18 @@ module IssuablesHelper
label_names.join(', ')
end
- def issuables_state_counter_text(issuable_type, state)
+ def issuables_state_counter_text(issuable_type, state, display_count)
titles = {
opened: "Open"
}
state_title = titles[state] || state.to_s.humanize
- count = issuables_count_for_state(issuable_type, state)
-
html = content_tag(:span, state_title)
- html << " " << content_tag(:span, number_with_delimiter(count), class: 'badge')
+
+ if display_count
+ count = issuables_count_for_state(issuable_type, state)
+ html << " " << content_tag(:span, number_with_delimiter(count), class: 'badge')
+ end
html.html_safe
end
@@ -191,24 +193,10 @@ module IssuablesHelper
end
end
- def issuable_filter_params
- [
- :search,
- :author_id,
- :assignee_id,
- :milestone_title,
- :label_name
- ]
- end
-
def issuable_reference(issuable)
@show_full_reference ? issuable.to_reference(full: true) : issuable.to_reference(@group || @project)
end
- def issuable_filter_present?
- issuable_filter_params.any? { |k| params.key?(k) }
- end
-
def issuable_initial_data(issuable)
data = {
endpoint: issuable_path(issuable),
diff --git a/app/views/dashboard/issues.html.haml b/app/views/dashboard/issues.html.haml
index 3e85535dae0..24d69b56c20 100644
--- a/app/views/dashboard/issues.html.haml
+++ b/app/views/dashboard/issues.html.haml
@@ -5,7 +5,7 @@
= auto_discovery_link_tag(:atom, params.merge(rss_url_options), title: "#{current_user.name} issues")
.top-area
- = render 'shared/issuable/nav', type: :issues
+ = render 'shared/issuable/nav', type: :issues, display_count: !@no_filters_set
.nav-controls
= link_to params.merge(rss_url_options), class: 'btn has-tooltip', data: { container: 'body' }, title: 'Subscribe' do
= icon('rss')
diff --git a/app/views/dashboard/merge_requests.html.haml b/app/views/dashboard/merge_requests.html.haml
index 53cd1130299..fb57ec7835c 100644
--- a/app/views/dashboard/merge_requests.html.haml
+++ b/app/views/dashboard/merge_requests.html.haml
@@ -3,7 +3,7 @@
- header_title "Merge Requests", merge_requests_dashboard_path(assignee_id: current_user.id)
.top-area
- = render 'shared/issuable/nav', type: :merge_requests
+ = render 'shared/issuable/nav', type: :merge_requests, display_count: !@no_filters_set
.nav-controls
= render 'shared/new_project_item_select', path: 'merge_requests/new', label: "New merge request", with_feature_enabled: 'merge_requests', type: :merge_requests
diff --git a/app/views/shared/issuable/_filter.html.haml b/app/views/shared/issuable/_filter.html.haml
index 7704c88905b..58b37290cd5 100644
--- a/app/views/shared/issuable/_filter.html.haml
+++ b/app/views/shared/issuable/_filter.html.haml
@@ -24,10 +24,6 @@
.filter-item.inline.labels-filter
= render "shared/issuable/label_dropdown", selected: selected_labels, use_id: false, selected_toggle: params[:label_name], data_options: { field_name: "label_name[]" }
- - if issuable_filter_present?
- .filter-item.inline.reset-filters
- %a{ href: page_filter_path(without: issuable_filter_params) } Reset filters
-
.pull-right
= render 'shared/sort_dropdown'
diff --git a/app/views/shared/issuable/_nav.html.haml b/app/views/shared/issuable/_nav.html.haml
index 4d8109eb90c..a5f40ea934b 100644
--- a/app/views/shared/issuable/_nav.html.haml
+++ b/app/views/shared/issuable/_nav.html.haml
@@ -1,22 +1,23 @@
- type = local_assigns.fetch(:type, :issues)
- page_context_word = type.to_s.humanize(capitalize: false)
+- display_count = local_assigns.fetch(:display_count, :true)
%ul.nav-links.issues-state-filters.mobile-separator
%li{ class: active_when(params[:state] == 'opened') }>
= link_to page_filter_path(state: 'opened', label: true), id: 'state-opened', title: "Filter by #{page_context_word} that are currently opened.", data: { state: 'opened' } do
- #{issuables_state_counter_text(type, :opened)}
+ #{issuables_state_counter_text(type, :opened, display_count)}
- if type == :merge_requests
%li{ class: active_when(params[:state] == 'merged') }>
= link_to page_filter_path(state: 'merged', label: true), id: 'state-merged', title: 'Filter by merge requests that are currently merged.', data: { state: 'merged' } do
- #{issuables_state_counter_text(type, :merged)}
+ #{issuables_state_counter_text(type, :merged, display_count)}
%li{ class: active_when(params[:state] == 'closed') }>
= link_to page_filter_path(state: 'closed', label: true), id: 'state-closed', title: 'Filter by merge requests that are currently closed and unmerged.', data: { state: 'closed' } do
- #{issuables_state_counter_text(type, :closed)}
+ #{issuables_state_counter_text(type, :closed, display_count)}
- else
%li{ class: active_when(params[:state] == 'closed') }>
= link_to page_filter_path(state: 'closed', label: true), id: 'state-closed', title: 'Filter by issues that are currently closed.', data: { state: 'closed' } do
- #{issuables_state_counter_text(type, :closed)}
+ #{issuables_state_counter_text(type, :closed, display_count)}
- = render 'shared/issuable/nav_links/all', page_context_word: page_context_word, counter: issuables_state_counter_text(type, :all)
+ = render 'shared/issuable/nav_links/all', page_context_word: page_context_word, counter: issuables_state_counter_text(type, :all, display_count)
diff --git a/spec/controllers/dashboard_controller_spec.rb b/spec/controllers/dashboard_controller_spec.rb
index 97c2c3fb940..3458d679107 100644
--- a/spec/controllers/dashboard_controller_spec.rb
+++ b/spec/controllers/dashboard_controller_spec.rb
@@ -11,9 +11,11 @@ describe DashboardController do
describe 'GET issues' do
it_behaves_like 'issuables list meta-data', :issue, :issues
+ it_behaves_like 'issuables requiring filter', :issues
end
describe 'GET merge requests' do
it_behaves_like 'issuables list meta-data', :merge_request, :merge_requests
+ it_behaves_like 'issuables requiring filter', :merge_requests
end
end
diff --git a/spec/features/dashboard/issues_spec.rb b/spec/features/dashboard/issues_spec.rb
index 8d1d5a51750..e41a2e4ce09 100644
--- a/spec/features/dashboard/issues_spec.rb
+++ b/spec/features/dashboard/issues_spec.rb
@@ -51,15 +51,6 @@ RSpec.describe 'Dashboard Issues' do
expect(page).not_to have_content(other_issue.title)
end
- it 'shows all issues' do
- click_link('Reset filters')
-
- expect(page).to have_content(authored_issue.title)
- expect(page).to have_content(authored_issue_on_public_project.title)
- expect(page).to have_content(assigned_issue.title)
- expect(page).to have_content(other_issue.title)
- end
-
it 'state filter tabs work' do
find('#state-closed').click
expect(page).to have_current_path(issues_dashboard_url(assignee_id: current_user.id, state: 'closed'), url: true)
diff --git a/spec/support/issuables_list_metadata_shared_examples.rb b/spec/support/issuables_list_metadata_shared_examples.rb
index 75982432ab4..e61983c60b4 100644
--- a/spec/support/issuables_list_metadata_shared_examples.rb
+++ b/spec/support/issuables_list_metadata_shared_examples.rb
@@ -5,9 +5,9 @@ shared_examples 'issuables list meta-data' do |issuable_type, action = nil|
%w[fix improve/awesome].each do |source_branch|
issuable =
if issuable_type == :issue
- create(issuable_type, project: project)
+ create(issuable_type, project: project, author: project.creator)
else
- create(issuable_type, source_project: project, source_branch: source_branch)
+ create(issuable_type, source_project: project, source_branch: source_branch, author: project.creator)
end
@issuable_ids << issuable.id
@@ -16,7 +16,7 @@ shared_examples 'issuables list meta-data' do |issuable_type, action = nil|
it "creates indexed meta-data object for issuable notes and votes count" do
if action
- get action
+ get action, author_id: project.creator.id
else
get :index, namespace_id: project.namespace, project_id: project
end
@@ -35,7 +35,7 @@ shared_examples 'issuables list meta-data' do |issuable_type, action = nil|
it "doesn't execute any queries with false conditions" do
get_action =
if action
- proc { get action }
+ proc { get action, author_id: project.creator.id }
else
proc { get :index, namespace_id: project2.namespace, project_id: project2 }
end
diff --git a/spec/support/issuables_requiring_filter_shared_examples.rb b/spec/support/issuables_requiring_filter_shared_examples.rb
new file mode 100644
index 00000000000..439ef5ed92e
--- /dev/null
+++ b/spec/support/issuables_requiring_filter_shared_examples.rb
@@ -0,0 +1,15 @@
+shared_examples 'issuables requiring filter' do |action|
+ it "doesn't load any issuables if no filter is set" do
+ expect_any_instance_of(described_class).not_to receive(:issuables_collection)
+
+ get action
+
+ expect(response).to render_template(action)
+ end
+
+ it "loads issuables if at least one filter is set" do
+ expect_any_instance_of(described_class).to receive(:issuables_collection).and_call_original
+
+ get action, author_id: user.id
+ end
+end