Commit 972734a9 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'improve_sorting_list' into 'master'

Improve list of sorting options

Closes #36929

See merge request gitlab-org/gitlab-ce!14320
parents 0948d031 0d236638
......@@ -117,19 +117,32 @@ module IssuableCollections
key = 'issuable_sort'
cookies[key] = params[:sort] if params[:sort].present?
# id_desc and id_asc are old values for these two.
cookies[key] = sort_value_recently_created if cookies[key] == 'id_desc'
cookies[key] = sort_value_oldest_created if cookies[key] == 'id_asc'
cookies[key] = update_cookie_value(cookies[key])
params[:sort] = cookies[key]
end
def default_sort_order
case params[:state]
when 'opened', 'all' then sort_value_recently_created
when 'opened', 'all' then sort_value_created_date
when 'merged', 'closed' then sort_value_recently_updated
else sort_value_recently_created
else sort_value_created_date
end
end
# Update old values to the actual ones.
def update_cookie_value(value)
case value
when 'id_asc' then sort_value_oldest_created
when 'id_desc' then sort_value_recently_created
when 'created_asc' then sort_value_created_date
when 'created_desc' then sort_value_created_date
when 'due_date_asc' then sort_value_due_date
when 'due_date_desc' then sort_value_due_date
when 'milestone_due_asc' then sort_value_milestone
when 'milestone_due_desc' then sort_value_milestone
when 'downvotes_asc' then sort_value_popularity
when 'downvotes_desc' then sort_value_popularity
else value
end
end
end
This diff is collapsed.
......@@ -143,16 +143,18 @@ module Issuable
end
def sort(method, excluded_labels: [])
sorted = case method.to_s
when 'milestone_due_asc' then order_milestone_due_asc
when 'milestone_due_desc' then order_milestone_due_desc
when 'downvotes_desc' then order_downvotes_desc
when 'upvotes_desc' then order_upvotes_desc
when 'label_priority' then order_labels_priority(excluded_labels: excluded_labels)
when 'priority' then order_due_date_and_labels_priority(excluded_labels: excluded_labels)
else
order_by(method)
end
sorted =
case method.to_s
when 'downvotes_desc' then order_downvotes_desc
when 'label_priority' then order_labels_priority(excluded_labels: excluded_labels)
when 'milestone' then order_milestone_due_asc
when 'milestone_due_asc' then order_milestone_due_asc
when 'milestone_due_desc' then order_milestone_due_desc
when 'popularity' then order_upvotes_desc
when 'priority' then order_due_date_and_labels_priority(excluded_labels: excluded_labels)
when 'upvotes_desc' then order_upvotes_desc
else order_by(method)
end
# Break ties with the ID column for pagination
sorted.order(id: :desc)
......@@ -214,7 +216,7 @@ module Issuable
def grouping_columns(sort)
grouping_columns = [arel_table[:id]]
if %w(milestone_due_desc milestone_due_asc).include?(sort)
if %w(milestone_due_desc milestone_due_asc milestone).include?(sort)
milestone_table = Milestone.arel_table
grouping_columns << milestone_table[:id]
grouping_columns << milestone_table[:due_date]
......
......@@ -19,14 +19,15 @@ module Sortable
module ClassMethods
def order_by(method)
case method.to_s
when 'name_asc' then order_name_asc
when 'name_desc' then order_name_desc
when 'updated_asc' then order_updated_asc
when 'updated_desc' then order_updated_desc
when 'created_asc' then order_created_asc
when 'created_asc' then order_created_asc
when 'created_date' then order_created_desc
when 'created_desc' then order_created_desc
when 'id_desc' then order_id_desc
when 'id_asc' then order_id_asc
when 'id_asc' then order_id_asc
when 'id_desc' then order_id_desc
when 'name_asc' then order_name_asc
when 'name_desc' then order_name_desc
when 'updated_asc' then order_updated_asc
when 'updated_desc' then order_updated_desc
else
all
end
......
......@@ -116,7 +116,8 @@ class Issue < ActiveRecord::Base
def self.sort(method, excluded_labels: [])
case method.to_s
when 'due_date_asc' then order_due_date_asc
when 'due_date' then order_due_date_asc
when 'due_date_asc' then order_due_date_asc
when 'due_date_desc' then order_due_date_desc
else
super
......
- sorted_by = sort_options_hash[@sort]
- viewing_issues = controller.controller_name == 'issues' || controller.action_name == 'issues'
.dropdown.inline.prepend-left-10
%button.dropdown-toggle{ type: 'button', data: {toggle: 'dropdown' } }
- if @sort.present?
= sort_options_hash[@sort]
- else
= sort_title_recently_created
%button.dropdown-toggle{ type: 'button', data: { toggle: 'dropdown' } }
= sorted_by
= icon('chevron-down')
%ul.dropdown-menu.dropdown-menu-align-right.dropdown-menu-sort
%ul.dropdown-menu.dropdown-menu-align-right.dropdown-menu-selectable.dropdown-menu-sort
%li
= link_to page_filter_path(sort: sort_value_priority, label: true) do
= sort_title_priority
= link_to page_filter_path(sort: sort_value_label_priority, label: true) do
= sort_title_label_priority
= link_to page_filter_path(sort: sort_value_recently_created, label: true) do
= sort_title_recently_created
= link_to page_filter_path(sort: sort_value_oldest_created, label: true) do
= sort_title_oldest_created
= link_to page_filter_path(sort: sort_value_recently_updated, label: true) do
= sort_title_recently_updated
= link_to page_filter_path(sort: sort_value_oldest_updated, label: true) do
= sort_title_oldest_updated
= link_to page_filter_path(sort: sort_value_milestone_soon, label: true) do
= sort_title_milestone_soon
= link_to page_filter_path(sort: sort_value_milestone_later, label: true) do
= sort_title_milestone_later
- if viewing_issues
= link_to page_filter_path(sort: sort_value_due_date_soon, label: true) do
= sort_title_due_date_soon
= link_to page_filter_path(sort: sort_value_due_date_later, label: true) do
= sort_title_due_date_later
= link_to page_filter_path(sort: sort_value_upvotes, label: true) do
= sort_title_upvotes
= link_to page_filter_path(sort: sort_value_downvotes, label: true) do
= sort_title_downvotes
= sortable_item(sort_title_priority, page_filter_path(sort: sort_value_priority, label: true), sorted_by)
= sortable_item(sort_title_created_date, page_filter_path(sort: sort_value_created_date, label: true), sorted_by)
= sortable_item(sort_title_recently_updated, page_filter_path(sort: sort_value_recently_updated, label: true), sorted_by)
= sortable_item(sort_title_milestone, page_filter_path(sort: sort_value_milestone, label: true), sorted_by)
= sortable_item(sort_title_due_date, page_filter_path(sort: sort_value_due_date, label: true), sorted_by) if viewing_issues
= sortable_item(sort_title_popularity, page_filter_path(sort: sort_value_popularity, label: true), sorted_by)
= sortable_item(sort_title_label_priority, page_filter_path(sort: sort_value_label_priority, label: true), sorted_by)
---
title: Improve list of sorting options
merge_request: 14320
author: Vitaliy @blackst0ne Klachkov
type: added
......@@ -51,36 +51,34 @@ Feature: Project Issues
@javascript
Scenario: Visiting Issues after being sorted the list
Given I visit project "Shop" issues page
And I sort the list by "Oldest updated"
And I sort the list by "Last updated"
And I visit my project's home page
And I visit project "Shop" issues page
Then The list should be sorted by "Oldest updated"
Then The list should be sorted by "Last updated"
@javascript
Scenario: Visiting Merge Requests after being sorted the list
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 sort the list by "Last updated"
And I visit project "Shop" merge requests page
Then The list should be sorted by "Oldest updated"
Then The list should be sorted by "Last updated"
@javascript
Scenario: Visiting Merge Requests from a differente Project after sorting
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 sort the list by "Last updated"
And I visit dashboard merge requests page
Then The list should be sorted by "Oldest updated"
Then The list should be sorted by "Last updated"
@javascript
Scenario: Sort issues by upvotes/downvotes
Given project "Shop" have "Bugfix" open issue
And issue "Release 0.4" have 2 upvotes and 1 downvote
And issue "Tweet control" have 1 upvote and 2 downvotes
And I sort the list by "Most popular"
Then The list should be sorted by "Most popular"
And I sort the list by "Least popular"
Then The list should be sorted by "Least popular"
And I sort the list by "Popularity"
Then The list should be sorted by "Popularity"
# Markdown
......
......@@ -91,28 +91,26 @@ Feature: Project Merge Requests
@javascript
Scenario: Visiting Merge Requests after being sorted the list
Given I visit project "Shop" merge requests page
And I sort the list by "Oldest updated"
And I sort the list by "Last updated"
And I visit my project's home page
And I visit project "Shop" merge requests page
Then The list should be sorted by "Oldest updated"
Then The list should be sorted by "Last updated"
@javascript
Scenario: Visiting Merge Requests from a different Project after sorting
Given I visit project "Shop" merge requests page
And I sort the list by "Oldest updated"
And I sort the list by "Last updated"
And I visit dashboard merge requests page
Then The list should be sorted by "Oldest updated"
Then The list should be sorted by "Last updated"
@javascript
Scenario: Sort merge requests by upvotes/downvotes
Scenario: Sort merge requests by upvotes
Given project "Shop" have "Bug NS-05" open merge request with diffs inside
And project "Shop" have "Bug NS-06" open merge request
And merge request "Bug NS-04" have 2 upvotes and 1 downvote
And merge request "Bug NS-06" have 1 upvote and 2 downvotes
And I sort the list by "Most popular"
Then The list should be sorted by "Most popular"
And I sort the list by "Least popular"
Then The list should be sorted by "Least popular"
And I sort the list by "Popularity"
Then The list should be sorted by "Popularity"
@javascript
Scenario: I comment on a merge request diff
......
......@@ -223,7 +223,7 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps
end
end
step 'The list should be sorted by "Most popular"' do
step 'The list should be sorted by "Popularity"' do
page.within '.issues-list' do
page.within 'li.issue:nth-child(1)' do
expect(page).to have_content 'Release 0.4'
......
......@@ -222,7 +222,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
end
end
step 'The list should be sorted by "Most popular"' do
step 'The list should be sorted by "Popularity"' do
page.within '.mr-list' do
page.within 'li.merge-request:nth-child(1)' do
expect(page).to have_content 'Bug NS-04'
......
......@@ -109,10 +109,10 @@ module SharedIssuable
edit_issuable
end
step 'I sort the list by "Oldest updated"' do
step 'I sort the list by "Last updated"' do
find('button.dropdown-toggle').click
page.within('.content ul.dropdown-menu.dropdown-menu-align-right li') do
click_link "Oldest updated"
click_link "Last updated"
end
end
......@@ -124,16 +124,16 @@ module SharedIssuable
end
end
step 'I sort the list by "Most popular"' do
step 'I sort the list by "Popularity"' do
find('button.dropdown-toggle').click
page.within('.content ul.dropdown-menu.dropdown-menu-align-right li') do
click_link 'Most popular'
click_link 'Popularity'
end
end
step 'The list should be sorted by "Oldest updated"' do
expect(find('.issues-filters')).to have_content('Oldest updated')
step 'The list should be sorted by "Last updated"' do
expect(find('.issues-filters')).to have_content('Last updated')
end
step 'I click link "Next" in the sidebar' do
......
......@@ -90,17 +90,17 @@ feature 'Dashboard Issues filtering', :js do
context 'sorting' do
it 'shows sorted issues' do
sorting_by('Oldest updated')
sorting_by('Created date')
visit_issues
expect(find('.issues-filters')).to have_content('Oldest updated')
expect(find('.issues-filters')).to have_content('Created date')
end
it 'keeps sorting issues after visiting Projects Issues page' do
sorting_by('Oldest updated')
sorting_by('Created date')
visit project_issues_path(project)
expect(find('.issues-filters')).to have_content('Oldest updated')
expect(find('.issues-filters')).to have_content('Created date')
end
end
......
......@@ -112,19 +112,19 @@ feature 'Dashboard Merge Requests' do
end
it 'shows sorted merge requests' do
sorting_by('Oldest updated')
sorting_by('Created date')
visit merge_requests_dashboard_path(assignee_id: current_user.id)
expect(find('.issues-filters')).to have_content('Oldest updated')
expect(find('.issues-filters')).to have_content('Created date')
end
it 'keeps sorting merge requests after visiting Projects MR page' do
sorting_by('Oldest updated')
sorting_by('Created date')
visit project_merge_requests_path(project)
expect(find('.issues-filters')).to have_content('Oldest updated')
expect(find('.issues-filters')).to have_content('Created date')
end
end
end
......@@ -40,10 +40,10 @@ describe 'Projects > Issuables > Default sort order' do
context 'in the "merge requests / open" tab', js: true do
let(:issuable_type) { :merge_request }
it 'is "last created"' do
it 'is "created date"' do
visit_merge_requests_with_state(project, 'open')
expect(selected_sort_order).to eq('last created')
expect(selected_sort_order).to eq('created date')
expect(first_merge_request).to include(last_created_issuable.title)
expect(last_merge_request).to include(first_created_issuable.title)
end
......@@ -76,10 +76,10 @@ describe 'Projects > Issuables > Default sort order' do
context 'in the "merge requests / all" tab', js: true do
let(:issuable_type) { :merge_request }
it 'is "last created"' do
it 'is "created date"' do
visit_merge_requests_with_state(project, 'all')
expect(find('.issues-other-filters')).to have_content('Last created')
expect(find('.issues-other-filters')).to have_content('Created date')
expect(first_merge_request).to include(last_created_issuable.title)
expect(last_merge_request).to include(first_created_issuable.title)
end
......@@ -105,10 +105,10 @@ describe 'Projects > Issuables > Default sort order' do
context 'in the "issues" tab', js: true do
let(:issuable_type) { :issue }
it 'is "last created"' do
it 'is "created date"' do
visit_issues project
expect(find('.issues-other-filters')).to have_content('Last created')
expect(find('.issues-other-filters')).to have_content('Created date')
expect(first_issue).to include(last_created_issuable.title)
expect(last_issue).to include(first_created_issuable.title)
end
......@@ -117,10 +117,10 @@ describe 'Projects > Issuables > Default sort order' do
context 'in the "issues / open" tab', js: true do
let(:issuable_type) { :issue }
it 'is "last created"' do
it 'is "created date"' do
visit_issues_with_state(project, 'open')
expect(find('.issues-other-filters')).to have_content('Last created')
expect(find('.issues-other-filters')).to have_content('Created date')
expect(first_issue).to include(last_created_issuable.title)
expect(last_issue).to include(first_created_issuable.title)
end
......@@ -141,10 +141,10 @@ describe 'Projects > Issuables > Default sort order' do
context 'in the "issues / all" tab', js: true do
let(:issuable_type) { :issue }
it 'is "last created"' do
it 'is "created date"' do
visit_issues_with_state(project, 'all')
expect(find('.issues-other-filters')).to have_content('Last created')
expect(find('.issues-other-filters')).to have_content('Created date')
expect(first_issue).to include(last_created_issuable.title)
expect(last_issue).to include(first_created_issuable.title)
end
......@@ -157,26 +157,12 @@ describe 'Projects > Issuables > Default sort order' do
visit_issues(project, sort: 'id_desc')
end
it 'shows the sort order as last created' do
expect(find('.issues-other-filters')).to have_content('Last created')
it 'shows the sort order as created date' do
expect(find('.issues-other-filters')).to have_content('Created date')
expect(first_issue).to include(last_created_issuable.title)
expect(last_issue).to include(first_created_issuable.title)
end
end
context 'when the sort in the URL is id_asc' do
let(:issuable_type) { :issue }
before do
visit_issues(project, sort: 'id_asc')
end
it 'shows the sort order as oldest created' do
expect(find('.issues-other-filters')).to have_content('Oldest created')
expect(first_issue).to include(first_created_issuable.title)
expect(last_issue).to include(last_created_issuable.title)
end
end
end
def selected_sort_order
......
......@@ -405,20 +405,18 @@ describe 'Filter issues', js: true do
end
context 'sorting' do
it 'sorts by oldest updated' do
create(:issue,
it 'sorts by created date' do
new_issue = create(:issue,
title: '3 days ago',
project: project,
author: user,
created_at: 3.days.ago,
updated_at: 3.days.ago)
created_at: 3.days.ago)
old_issue = create(:issue,
create(:issue,
title: '5 days ago',
project: project,
author: user,
created_at: 5.days.ago,
updated_at: 5.days.ago)
created_at: 5.days.ago)
input_filtered_search('days ago')
......@@ -427,10 +425,10 @@ describe 'Filter issues', js: true do
sort_toggle = find('.filtered-search-wrapper .dropdown-toggle')
sort_toggle.click
find('.filtered-search-wrapper .dropdown-menu li a', text: 'Oldest updated').click
find('.filtered-search-wrapper .dropdown-menu li a', text: 'Created date').click
wait_for_requests
expect(find('.issues-list .issue:first-of-type .issue-title-text a')).to have_content(old_issue.title)
expect(find('.issues-list .issue:first-of-type .issue-title-text a')).to have_content(new_issue.title)
end
end
end
......
......@@ -190,19 +190,12 @@ describe 'Issues' do
let(:later_due_milestone) { create(:milestone, due_date: '2013-12-12') }
it 'sorts by newest' do
visit project_issues_path(project, sort: sort_value_recently_created)
visit project_issues_path(project, sort: sort_value_created_date)
expect(first_issue).to include('foo')
expect(last_issue).to include('baz')
end
it 'sorts by oldest' do
visit project_issues_path(project, sort: sort_value_oldest_created)
expect(first_issue).to include('baz')
expect(last_issue).to include('foo')
end
it 'sorts by most recently updated' do
baz.updated_at = Time.now + 100
baz.save
......@@ -211,36 +204,22 @@ describe 'Issues' do
expect(first_issue).to include('baz')
end
it 'sorts by least recently updated' do
baz.updated_at = Time.now - 100
baz.save
visit project_issues_path(project, sort: sort_value_oldest_updated)
expect(first_issue).to include('baz')
end
describe 'sorting by due date' do
before do
foo.update(due_date: 1.day.from_now)
bar.update(due_date: 6.days.from_now)
end
it 'sorts by recently due date' do
visit project_issues_path(project, sort: sort_value_due_date_soon)
it 'sorts by due date' do
visit project_issues_path(project, sort: sort_value_due_date)
expect(first_issue).to include('foo')
end
it 'sorts by least recently due date' do
visit project_issues_path(project, sort: sort_value_due_date_later)
expect(first_issue).to include('bar')
end
it 'sorts by least recently due date by excluding nil due dates' do
it 'sorts by due date by excluding nil due dates' do
bar.update(due_date: nil)
visit project_issues_path(project, sort: sort_value_due_date_later)
visit project_issues_path(project, sort: sort_value_due_date)
expect(first_issue).to include('foo')
end
......@@ -339,19 +318,12 @@ describe 'Issues' do
bar.save
end
it 'sorts by recently due milestone' do
visit project_issues_path(project, sort: sort_value_milestone_soon)
it 'sorts by milestone' do
visit project_issues_path(project, sort: sort_value_milestone)
expect(first_issue).to include('foo')
expect(last_issue).to include('baz')
end
it 'sorts by least recently due milestone' do
visit project_issues_path(project, sort: sort_value_milestone_later)
expect(first_issue).to include('bar')
expect(last_issue).to include('baz')
end
end
describe 'combine filter and sort' do
......@@ -365,13 +337,11 @@ describe 'Issues' do
end
it 'sorts with a filter applied' do
visit project_issues_path(project,
sort: sort_value_oldest_created,
assignee_id: user2.id)
visit project_issues_path(project, sort: sort_value_created_date, assignee_id: user2.id)
expect(first_issue).to include('bar')
expect(last_issue).to include('foo')
expect(page).not_to have_content 'baz'
expect(first_issue).to include('foo')
expect(last_issue).to include('bar')
expect(page).not_to have_content('baz')
end
end
end
......
......@@ -277,9 +277,9 @@ describe 'Filter merge requests' do
expect_mr_list_count(2)
click_button 'Last created'
click_button 'Created date'
page.within '.dropdown-menu-sort' do
click_link 'Oldest created'
click_link 'Priority'
end
wait_for_requests
......
......@@ -52,21 +52,13 @@ describe 'Projects > Merge requests > User lists merge requests' do
end
it 'sorts by newest' do
visit_merge_requests(project, sort: sort_value_recently_created)
visit_merge_requests(project, sort: sort_value_created_date)
expect(first_merge_request).to include('fix')
expect(last_merge_request).to include('merge-test')
expect(count_merge_requests).to eq(3)
end
it 'sorts by oldest' do
visit_merge_requests(project, sort: sort_value_oldest_created)
expect(first_merge_request).to include('merge-test')
expect(last_merge_request).to include('fix')
expect(count_merge_requests).to eq(3)
end
it 'sorts by last updated' do
visit_merge_requests(project, sort: sort_value_recently_updated)
......@@ -74,33 +66,19 @@ describe 'Projects > Merge requests > User lists merge requests' do
expect(count_merge_requests).to eq(3)
end
it 'sorts by oldest updated' do
visit_merge_requests(project, sort: sort_value_oldest_updated)
expect(first_merge_request).to include('markdown')
expect(count_merge_requests).to eq(3)
end
it 'sorts by milestone due soon' do
visit_merge_requests(project, sort: sort_value_milestone_soon)
it 'sorts by milestone' do
visit_merge_requests(project, sort: sort_value_milestone)
expect(first_merge_request).to include('fix')
expect(count_merge_requests).to eq(3)
end
it 'sorts by milestone due later' do
visit_merge_requests(project, sort: sort_value_milestone_later)
expect(first_merge_request).to include('markdown')
expect(count_merge_requests).to eq(3)
end
it 'filters on one label and sorts by due soon' do
it 'filters on one label and sorts by due date' do
label = create(:label, project: project)
create(:label_link, label: label, target: @fix)
visit_merge_requests(project, label_name: [label.name],
sort: sort_value_due_date_soon)
sort: sort_value_due_date)
expect(first_merge_request).to include('fix')
expect(count_merge_requests).to eq(1)
......@@ -115,9 +93,9 @@ describe 'Projects > Merge requests > User lists merge requests' do
create(:label_link, label: label2, target: @fix)
end
it 'sorts by due soon' do
it 'sorts by due date' do
visit_merge_requests(project, label_name: [label.name, label2.name],
sort: sort_value_due_date_soon)
sort: sort_value_due_date)
expect(first_merge_request).to include('fix')
expect(count_merge_requests).to eq(1)
......@@ -127,7 +105,7 @@ describe 'Projects > Merge requests > User lists merge requests' do
it 'sorts by due soon' do
visit_merge_requests(project, label_name: [label.name, label2.name],
assignee_id: user.id,
sort: sort_value_due_date_soon)
sort: sort_value_due_date)
expect(first_merge_request).to include('fix')
expect(count_merge_requests).to eq(1)
......@@ -137,7 +115,7 @@ describe 'Projects > Merge requests > User lists merge requests' do
visit project_merge_requests_path(project,
label_name: [label.name, label2.name],
assignee_id: user.id,
sort: sort_value_milestone_soon)
sort: sort_value_milestone)
expect(first_merge_request).to include('fix')
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