Commit c1b71e2f authored by Jan Provaznik's avatar Jan Provaznik Committed by Winnie Hellmann

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
parent 59a15895
...@@ -233,21 +233,21 @@ export default class SearchAutocomplete { ...@@ -233,21 +233,21 @@ export default class SearchAutocomplete {
const issueItems = [ const issueItems = [
{ {
text: 'Issues assigned to me', text: 'Issues assigned to me',
url: `${issuesPath}/?assignee_username=${userName}`, url: `${issuesPath}/?assignee_id=${userId}`,
}, },
{ {
text: "Issues I've created", text: "Issues I've created",
url: `${issuesPath}/?author_username=${userName}`, url: `${issuesPath}/?author_id=${userId}`,
}, },
]; ];
const mergeRequestItems = [ const mergeRequestItems = [
{ {
text: 'Merge requests assigned to me', text: 'Merge requests assigned to me',
url: `${mrPath}/?assignee_username=${userName}`, url: `${mrPath}/?assignee_id=${userId}`,
}, },
{ {
text: "Merge requests I've created", text: "Merge requests I've created",
url: `${mrPath}/?author_username=${userName}`, url: `${mrPath}/?author_id=${userId}`,
}, },
]; ];
......
...@@ -2,9 +2,17 @@ class DashboardController < Dashboard::ApplicationController ...@@ -2,9 +2,17 @@ class DashboardController < Dashboard::ApplicationController
include IssuesAction include IssuesAction
include MergeRequestsAction include MergeRequestsAction
FILTER_PARAMS = [
:author_id,
:assignee_id,
:milestone_title,
:label_name
].freeze
before_action :event_filter, only: :activity before_action :event_filter, only: :activity
before_action :projects, only: [:issues, :merge_requests] before_action :projects, only: [:issues, :merge_requests]
before_action :set_show_full_reference, 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 respond_to :html
...@@ -39,4 +47,10 @@ class DashboardController < Dashboard::ApplicationController ...@@ -39,4 +47,10 @@ class DashboardController < Dashboard::ApplicationController
def set_show_full_reference def set_show_full_reference
@show_full_reference = true @show_full_reference = true
end 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 end
...@@ -228,9 +228,7 @@ module ApplicationHelper ...@@ -228,9 +228,7 @@ module ApplicationHelper
scope: params[:scope], scope: params[:scope],
milestone_title: params[:milestone_title], milestone_title: params[:milestone_title],
assignee_id: params[:assignee_id], assignee_id: params[:assignee_id],
assignee_username: params[:assignee_username],
author_id: params[:author_id], author_id: params[:author_id],
author_username: params[:author_username],
search: params[:search], search: params[:search],
label_name: params[:label_name] label_name: params[:label_name]
} }
......
...@@ -159,16 +159,18 @@ module IssuablesHelper ...@@ -159,16 +159,18 @@ module IssuablesHelper
label_names.join(', ') label_names.join(', ')
end end
def issuables_state_counter_text(issuable_type, state) def issuables_state_counter_text(issuable_type, state, display_count)
titles = { titles = {
opened: "Open" opened: "Open"
} }
state_title = titles[state] || state.to_s.humanize 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, state_title)
if display_count
count = issuables_count_for_state(issuable_type, state)
html << " " << content_tag(:span, number_with_delimiter(count), class: 'badge') html << " " << content_tag(:span, number_with_delimiter(count), class: 'badge')
end
html.html_safe html.html_safe
end end
...@@ -191,24 +193,10 @@ module IssuablesHelper ...@@ -191,24 +193,10 @@ module IssuablesHelper
end end
end end
def issuable_filter_params
[
:search,
:author_id,
:assignee_id,
:milestone_title,
:label_name
]
end
def issuable_reference(issuable) def issuable_reference(issuable)
@show_full_reference ? issuable.to_reference(full: true) : issuable.to_reference(@group || @project) @show_full_reference ? issuable.to_reference(full: true) : issuable.to_reference(@group || @project)
end end
def issuable_filter_present?
issuable_filter_params.any? { |k| params.key?(k) }
end
def issuable_initial_data(issuable) def issuable_initial_data(issuable)
data = { data = {
endpoint: issuable_path(issuable), endpoint: issuable_path(issuable),
......
...@@ -5,7 +5,7 @@ ...@@ -5,7 +5,7 @@
= auto_discovery_link_tag(:atom, params.merge(rss_url_options), title: "#{current_user.name} issues") = auto_discovery_link_tag(:atom, params.merge(rss_url_options), title: "#{current_user.name} issues")
.top-area .top-area
= render 'shared/issuable/nav', type: :issues = render 'shared/issuable/nav', type: :issues, display_count: !@no_filters_set
.nav-controls .nav-controls
= link_to params.merge(rss_url_options), class: 'btn has-tooltip', data: { container: 'body' }, title: 'Subscribe' do = link_to params.merge(rss_url_options), class: 'btn has-tooltip', data: { container: 'body' }, title: 'Subscribe' do
= icon('rss') = icon('rss')
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
- header_title "Merge Requests", merge_requests_dashboard_path(assignee_id: current_user.id) - header_title "Merge Requests", merge_requests_dashboard_path(assignee_id: current_user.id)
.top-area .top-area
= render 'shared/issuable/nav', type: :merge_requests = render 'shared/issuable/nav', type: :merge_requests, display_count: !@no_filters_set
.nav-controls .nav-controls
= render 'shared/new_project_item_select', path: 'merge_requests/new', label: "New merge request", with_feature_enabled: 'merge_requests', type: :merge_requests = render 'shared/new_project_item_select', path: 'merge_requests/new', label: "New merge request", with_feature_enabled: 'merge_requests', type: :merge_requests
......
...@@ -24,10 +24,6 @@ ...@@ -24,10 +24,6 @@
.filter-item.inline.labels-filter .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[]" } = 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 .pull-right
= render 'shared/sort_dropdown' = render 'shared/sort_dropdown'
......
- type = local_assigns.fetch(:type, :issues) - type = local_assigns.fetch(:type, :issues)
- page_context_word = type.to_s.humanize(capitalize: false) - 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 %ul.nav-links.issues-state-filters.mobile-separator
%li{ class: active_when(params[:state] == 'opened') }> %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 = 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 - if type == :merge_requests
%li{ class: active_when(params[:state] == 'merged') }> %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 = 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') }> %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 = 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 - else
%li{ class: active_when(params[:state] == 'closed') }> %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 = 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)
...@@ -11,9 +11,11 @@ describe DashboardController do ...@@ -11,9 +11,11 @@ describe DashboardController do
describe 'GET issues' do describe 'GET issues' do
it_behaves_like 'issuables list meta-data', :issue, :issues it_behaves_like 'issuables list meta-data', :issue, :issues
it_behaves_like 'issuables requiring filter', :issues
end end
describe 'GET merge requests' do describe 'GET merge requests' do
it_behaves_like 'issuables list meta-data', :merge_request, :merge_requests it_behaves_like 'issuables list meta-data', :merge_request, :merge_requests
it_behaves_like 'issuables requiring filter', :merge_requests
end end
end end
...@@ -51,15 +51,6 @@ RSpec.describe 'Dashboard Issues' do ...@@ -51,15 +51,6 @@ RSpec.describe 'Dashboard Issues' do
expect(page).not_to have_content(other_issue.title) expect(page).not_to have_content(other_issue.title)
end 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 it 'state filter tabs work' do
find('#state-closed').click find('#state-closed').click
expect(page).to have_current_path(issues_dashboard_url(assignee_id: current_user.id, state: 'closed'), url: true) expect(page).to have_current_path(issues_dashboard_url(assignee_id: current_user.id, state: 'closed'), url: true)
......
...@@ -5,9 +5,9 @@ shared_examples 'issuables list meta-data' do |issuable_type, action = nil| ...@@ -5,9 +5,9 @@ shared_examples 'issuables list meta-data' do |issuable_type, action = nil|
%w[fix improve/awesome].each do |source_branch| %w[fix improve/awesome].each do |source_branch|
issuable = issuable =
if issuable_type == :issue if issuable_type == :issue
create(issuable_type, project: project) create(issuable_type, project: project, author: project.creator)
else 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 end
@issuable_ids << issuable.id @issuable_ids << issuable.id
...@@ -16,7 +16,7 @@ shared_examples 'issuables list meta-data' do |issuable_type, action = nil| ...@@ -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 it "creates indexed meta-data object for issuable notes and votes count" do
if action if action
get action get action, author_id: project.creator.id
else else
get :index, namespace_id: project.namespace, project_id: project get :index, namespace_id: project.namespace, project_id: project
end end
...@@ -35,7 +35,7 @@ shared_examples 'issuables list meta-data' do |issuable_type, action = nil| ...@@ -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 it "doesn't execute any queries with false conditions" do
get_action = get_action =
if action if action
proc { get action } proc { get action, author_id: project.creator.id }
else else
proc { get :index, namespace_id: project2.namespace, project_id: project2 } proc { get :index, namespace_id: project2.namespace, project_id: project2 }
end end
......
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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment