Commit 8bcc47ac authored by George Koltsov's avatar George Koltsov Committed by Kamil Trzciński

Add SortingPreference concern

Sorting preference functionality has been extracted
from `IssuableCollections` to a new `SortingPreference`
concern in order to reuse this functionality in projects
(and groups in the future).
parent 2c4fa09b
...@@ -2,8 +2,8 @@ ...@@ -2,8 +2,8 @@
module IssuableCollections module IssuableCollections
extend ActiveSupport::Concern extend ActiveSupport::Concern
include CookiesHelper
include SortingHelper include SortingHelper
include SortingPreference
include Gitlab::IssuableMetadata include Gitlab::IssuableMetadata
include Gitlab::Utils::StrongMemoize include Gitlab::Utils::StrongMemoize
...@@ -127,47 +127,8 @@ module IssuableCollections ...@@ -127,47 +127,8 @@ module IssuableCollections
'opened' 'opened'
end end
def set_sort_order def legacy_sort_cookie_name
set_sort_order_from_user_preference || set_sort_order_from_cookie || default_sort_order 'issuable_sort'
end
def set_sort_order_from_user_preference
return unless current_user
return unless issuable_sorting_field
user_preference = current_user.user_preference
sort_param = params[:sort]
sort_param ||= user_preference[issuable_sorting_field]
return sort_param if Gitlab::Database.read_only?
if user_preference[issuable_sorting_field] != sort_param
user_preference.update(issuable_sorting_field => sort_param)
end
sort_param
end
# Implement issuable_sorting_field method on controllers
# to choose which column to store the sorting parameter.
def issuable_sorting_field
nil
end
def set_sort_order_from_cookie
sort_param = params[:sort] if params[:sort].present?
# fallback to legacy cookie value for backward compatibility
sort_param ||= cookies['issuable_sort']
sort_param ||= cookies[remember_sorting_key]
sort_value = update_cookie_value(sort_param)
set_secure_cookie(remember_sorting_key, sort_value)
sort_value
end
def remember_sorting_key
@remember_sorting_key ||= "#{collection_type.downcase}_sort"
end end
def default_sort_order def default_sort_order
...@@ -178,17 +139,6 @@ module IssuableCollections ...@@ -178,17 +139,6 @@ module IssuableCollections
end end
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 'downvotes_asc' then sort_value_popularity
when 'downvotes_desc' then sort_value_popularity
else value
end
end
def finder def finder
@finder ||= issuable_finder_for(finder_type) @finder ||= issuable_finder_for(finder_type)
end end
......
...@@ -32,7 +32,7 @@ module IssuableCollectionsAction ...@@ -32,7 +32,7 @@ module IssuableCollectionsAction
private private
def issuable_sorting_field def sorting_field
case action_name case action_name
when 'issues' when 'issues'
Issue::SORTING_PREFERENCE_FIELD Issue::SORTING_PREFERENCE_FIELD
......
# frozen_string_literal: true
module SortingPreference
include SortingHelper
include CookiesHelper
def set_sort_order
set_sort_order_from_user_preference || set_sort_order_from_cookie || params[:sort] || default_sort_order
end
# Implement sorting_field method on controllers
# to choose which column to store the sorting parameter.
def sorting_field
nil
end
# Implement default_sort_order method on controllers
# to choose which default sort should be applied if
# sort param is not provided.
def default_sort_order
nil
end
# Implement legacy_sort_cookie_name method on controllers
# to set sort from cookie for backwards compatibility.
def legacy_sort_cookie_name
nil
end
private
def set_sort_order_from_user_preference
return unless current_user
return unless sorting_field
user_preference = current_user.user_preference
sort_param = params[:sort]
sort_param ||= user_preference[sorting_field]
return sort_param if Gitlab::Database.read_only?
if user_preference[sorting_field] != sort_param
user_preference.update(sorting_field => sort_param)
end
sort_param
end
def set_sort_order_from_cookie
return unless legacy_sort_cookie_name
sort_param = params[:sort] if params[:sort].present?
# fallback to legacy cookie value for backward compatibility
sort_param ||= cookies[legacy_sort_cookie_name]
sort_param ||= cookies[remember_sorting_key]
sort_value = update_cookie_value(sort_param)
set_secure_cookie(remember_sorting_key, sort_value)
sort_value
end
# Convert sorting_field to legacy cookie name for backwards compatibility
# :merge_requests_sort => 'mergerequest_sort'
# :issues_sort => 'issue_sort'
def remember_sorting_key
@remember_sorting_key ||= sorting_field
.to_s
.split('_')[0..-2]
.map(&:singularize)
.join('')
.concat('_sort')
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 'downvotes_asc' then sort_value_popularity
when 'downvotes_desc' then sort_value_popularity
else value
end
end
end
...@@ -4,10 +4,12 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController ...@@ -4,10 +4,12 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController
include ParamsBackwardCompatibility include ParamsBackwardCompatibility
include RendersMemberAccess include RendersMemberAccess
include OnboardingExperimentHelper include OnboardingExperimentHelper
include SortingHelper
include SortingPreference
prepend_before_action(only: [:index]) { authenticate_sessionless_user!(:rss) } prepend_before_action(only: [:index]) { authenticate_sessionless_user!(:rss) }
before_action :set_non_archived_param before_action :set_non_archived_param
before_action :default_sorting before_action :set_sorting
before_action :projects, only: [:index] before_action :projects, only: [:index]
skip_cross_project_access_check :index, :starred skip_cross_project_access_check :index, :starred
...@@ -59,11 +61,6 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController ...@@ -59,11 +61,6 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController
end end
end end
def default_sorting
params[:sort] ||= 'latest_activity_desc'
@sort = params[:sort]
end
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def load_projects(finder_params) def load_projects(finder_params)
@total_user_projects_count = ProjectsFinder.new(params: { non_public: true }, current_user: current_user).execute @total_user_projects_count = ProjectsFinder.new(params: { non_public: true }, current_user: current_user).execute
...@@ -88,4 +85,17 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController ...@@ -88,4 +85,17 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController
Events::RenderService.new(current_user).execute(@events, atom_request: request.format.atom?) Events::RenderService.new(current_user).execute(@events, atom_request: request.format.atom?)
end end
def set_sorting
params[:sort] = set_sort_order
@sort = params[:sort]
end
def default_sort_order
sort_value_latest_activity
end
def sorting_field
Project::SORTING_PREFERENCE_FIELD
end
end end
...@@ -3,12 +3,13 @@ ...@@ -3,12 +3,13 @@
class Explore::ProjectsController < Explore::ApplicationController class Explore::ProjectsController < Explore::ApplicationController
include ParamsBackwardCompatibility include ParamsBackwardCompatibility
include RendersMemberAccess include RendersMemberAccess
include SortingHelper
include SortingPreference
before_action :set_non_archived_param before_action :set_non_archived_param
before_action :set_sorting
def index def index
params[:sort] ||= 'latest_activity_desc'
@sort = params[:sort]
@projects = load_projects @projects = load_projects
respond_to do |format| respond_to do |format|
...@@ -23,7 +24,6 @@ class Explore::ProjectsController < Explore::ApplicationController ...@@ -23,7 +24,6 @@ class Explore::ProjectsController < Explore::ApplicationController
def trending def trending
params[:trending] = true params[:trending] = true
@sort = params[:sort]
@projects = load_projects @projects = load_projects
respond_to do |format| respond_to do |format|
...@@ -67,4 +67,17 @@ class Explore::ProjectsController < Explore::ApplicationController ...@@ -67,4 +67,17 @@ class Explore::ProjectsController < Explore::ApplicationController
prepare_projects_for_rendering(projects) prepare_projects_for_rendering(projects)
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
def set_sorting
params[:sort] = set_sort_order
@sort = params[:sort]
end
def default_sort_order
sort_value_latest_activity
end
def sorting_field
Project::SORTING_PREFERENCE_FIELD
end
end end
...@@ -190,7 +190,7 @@ class Projects::IssuesController < Projects::ApplicationController ...@@ -190,7 +190,7 @@ class Projects::IssuesController < Projects::ApplicationController
protected protected
def issuable_sorting_field def sorting_field
Issue::SORTING_PREFERENCE_FIELD Issue::SORTING_PREFERENCE_FIELD
end end
......
...@@ -219,7 +219,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo ...@@ -219,7 +219,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
alias_method :issuable, :merge_request alias_method :issuable, :merge_request
alias_method :awardable, :merge_request alias_method :awardable, :merge_request
def issuable_sorting_field def sorting_field
MergeRequest::SORTING_PREFERENCE_FIELD MergeRequest::SORTING_PREFERENCE_FIELD
end end
......
...@@ -55,6 +55,8 @@ class Project < ApplicationRecord ...@@ -55,6 +55,8 @@ class Project < ApplicationRecord
VALID_MIRROR_PORTS = [22, 80, 443].freeze VALID_MIRROR_PORTS = [22, 80, 443].freeze
VALID_MIRROR_PROTOCOLS = %w(http https ssh git).freeze VALID_MIRROR_PROTOCOLS = %w(http https ssh git).freeze
SORTING_PREFERENCE_FIELD = :projects_sort
cache_markdown_field :description, pipeline: :description cache_markdown_field :description, pipeline: :description
delegate :feature_available?, :builds_enabled?, :wiki_enabled?, delegate :feature_available?, :builds_enabled?, :wiki_enabled?,
......
---
title: Add persistance to last choice of projects sorting on projects dashboard page
merge_request: 31669
author:
type: added
# frozen_string_literal: true
class AddProjectsSortingFieldToUserPreferences < ActiveRecord::Migration[5.2]
DOWNTIME = false
def up
add_column :user_preferences, :projects_sort, :string, limit: 64
end
def down
remove_column :user_preferences, :projects_sort
end
end
...@@ -3411,6 +3411,7 @@ ActiveRecord::Schema.define(version: 2019_08_15_093949) do ...@@ -3411,6 +3411,7 @@ ActiveRecord::Schema.define(version: 2019_08_15_093949) do
t.string "epics_sort" t.string "epics_sort"
t.integer "roadmap_epics_state" t.integer "roadmap_epics_state"
t.string "roadmaps_sort" t.string "roadmaps_sort"
t.string "projects_sort", limit: 64
t.index ["user_id"], name: "index_user_preferences_on_user_id", unique: true t.index ["user_id"], name: "index_user_preferences_on_user_id", unique: true
end end
......
...@@ -24,78 +24,6 @@ describe IssuableCollections do ...@@ -24,78 +24,6 @@ describe IssuableCollections do
controller controller
end end
describe '#set_sort_order_from_user_preference' do
describe 'when sort param given' do
let(:params) { { sort: 'updated_desc' } }
context 'when issuable_sorting_field is defined' do
before do
controller.class.define_method(:issuable_sorting_field) { :issues_sort}
end
it 'sets user_preference with the right value' do
controller.send(:set_sort_order_from_user_preference)
expect(user.user_preference.reload.issues_sort).to eq('updated_desc')
end
end
context 'when no issuable_sorting_field is defined on the controller' do
it 'does not touch user_preference' do
allow(user).to receive(:user_preference)
controller.send(:set_sort_order_from_user_preference)
expect(user).not_to have_received(:user_preference)
end
end
end
context 'when a user sorting preference exists' do
let(:params) { {} }
before do
controller.class.define_method(:issuable_sorting_field) { :issues_sort }
end
it 'returns the set preference' do
user.user_preference.update(issues_sort: 'updated_asc')
sort_preference = controller.send(:set_sort_order_from_user_preference)
expect(sort_preference).to eq('updated_asc')
end
end
end
describe '#set_set_order_from_cookie' do
describe 'when sort param given' do
let(:cookies) { {} }
let(:params) { { sort: 'downvotes_asc' } }
it 'sets the cookie with the right values and flags' do
allow(controller).to receive(:cookies).and_return(cookies)
controller.send(:set_sort_order_from_cookie)
expect(cookies['issue_sort']).to eq({ value: 'popularity', secure: false, httponly: false })
end
end
describe 'when cookie exists' do
let(:cookies) { { 'issue_sort' => 'id_asc' } }
let(:params) { {} }
it 'sets the cookie with the right values and flags' do
allow(controller).to receive(:cookies).and_return(cookies)
controller.send(:set_sort_order_from_cookie)
expect(cookies['issue_sort']).to eq({ value: 'created_asc', secure: false, httponly: false })
end
end
end
describe '#page_count_for_relation' do describe '#page_count_for_relation' do
let(:params) { { state: 'opened' } } let(:params) { { state: 'opened' } }
......
# frozen_string_literal: true
require 'spec_helper'
describe SortingPreference do
let(:user) { create(:user) }
let(:controller_class) do
Class.new do
def self.helper_method(name); end
include SortingPreference
include SortingHelper
end
end
let(:controller) { controller_class.new }
before do
allow(controller).to receive(:params).and_return(ActionController::Parameters.new(params))
allow(controller).to receive(:current_user).and_return(user)
allow(controller).to receive(:legacy_sort_cookie_name).and_return('issuable_sort')
allow(controller).to receive(:sorting_field).and_return(:issues_sort)
end
describe '#set_sort_order_from_user_preference' do
subject { controller.send(:set_sort_order_from_user_preference) }
context 'when sort param given' do
let(:params) { { sort: 'updated_desc' } }
context 'when sorting_field is defined' do
it 'sets user_preference with the right value' do
is_expected.to eq('updated_desc')
end
end
context 'when no sorting_field is defined on the controller' do
before do
allow(controller).to receive(:sorting_field).and_return(nil)
end
it 'does not touch user_preference' do
expect(user).not_to receive(:user_preference)
subject
end
end
end
context 'when a user sorting preference exists' do
let(:params) { {} }
before do
user.user_preference.update!(issues_sort: 'updated_asc')
end
it 'returns the set preference' do
is_expected.to eq('updated_asc')
end
end
end
describe '#set_set_order_from_cookie' do
subject { controller.send(:set_sort_order_from_cookie) }
before do
allow(controller).to receive(:cookies).and_return(cookies)
end
context 'when sort param given' do
let(:cookies) { {} }
let(:params) { { sort: 'downvotes_asc' } }
it 'sets the cookie with the right values and flags' do
subject
expect(cookies['issue_sort']).to eq(value: 'popularity', secure: false, httponly: false)
end
end
context 'when cookie exists' do
let(:cookies) { { 'issue_sort' => 'id_asc' } }
let(:params) { {} }
it 'sets the cookie with the right values and flags' do
subject
expect(cookies['issue_sort']).to eq(value: 'created_asc', secure: false, httponly: false)
end
end
end
end
...@@ -40,6 +40,14 @@ describe Dashboard::ProjectsController do ...@@ -40,6 +40,14 @@ describe Dashboard::ProjectsController do
expect(assigns(:projects)).to eq([project, project2]) expect(assigns(:projects)).to eq([project, project2])
end end
context 'project sorting' do
let(:project) { create(:project) }
it_behaves_like 'set sort order from user preference' do
let(:sorting_param) { 'created_asc' }
end
end
end end
end end
......
...@@ -3,56 +3,91 @@ ...@@ -3,56 +3,91 @@
require 'spec_helper' require 'spec_helper'
describe Explore::ProjectsController do describe Explore::ProjectsController do
describe 'GET #index.json' do shared_examples 'explore projects' do
render_views describe 'GET #index.json' do
render_views
before do before do
get :index, format: :json get :index, format: :json
end
it { is_expected.to respond_with(:success) }
end end
it { is_expected.to respond_with(:success) } describe 'GET #trending.json' do
end render_views
describe 'GET #trending.json' do before do
render_views get :trending, format: :json
end
before do it { is_expected.to respond_with(:success) }
get :trending, format: :json end
describe 'GET #starred.json' do
render_views
before do
get :starred, format: :json
end
it { is_expected.to respond_with(:success) }
end end
it { is_expected.to respond_with(:success) } describe 'GET #trending' do
context 'sorting by update date' do
let(:project1) { create(:project, :public, updated_at: 3.days.ago) }
let(:project2) { create(:project, :public, updated_at: 1.day.ago) }
before do
create(:trending_project, project: project1)
create(:trending_project, project: project2)
end
it 'sorts by last updated' do
get :trending, params: { sort: 'updated_desc' }
expect(assigns(:projects)).to eq [project2, project1]
end
it 'sorts by oldest updated' do
get :trending, params: { sort: 'updated_asc' }
expect(assigns(:projects)).to eq [project1, project2]
end
end
end
end end
describe 'GET #starred.json' do context 'when user is signed in' do
render_views let(:user) { create(:user) }
before do before do
get :starred, format: :json sign_in(user)
end end
it { is_expected.to respond_with(:success) } include_examples 'explore projects'
end
describe 'GET #trending' do context 'user preference sorting' do
context 'sorting by update date' do let(:project) { create(:project) }
let(:project1) { create(:project, :public, updated_at: 3.days.ago) }
let(:project2) { create(:project, :public, updated_at: 1.day.ago) }
before do it_behaves_like 'set sort order from user preference' do
create(:trending_project, project: project1) let(:sorting_param) { 'created_asc' }
create(:trending_project, project: project2)
end end
end
end
it 'sorts by last updated' do context 'when user is not signed in' do
get :trending, params: { sort: 'updated_desc' } include_examples 'explore projects'
expect(assigns(:projects)).to eq [project2, project1] context 'user preference sorting' do
end let(:project) { create(:project) }
let(:sorting_param) { 'created_asc' }
it 'sorts by oldest updated' do it 'does not set sort order from user preference' do
get :trending, params: { sort: 'updated_asc' } expect_any_instance_of(UserPreference).not_to receive(:update)
expect(assigns(:projects)).to eq [project1, project2] get :index, params: { sort: sorting_param }
end end
end end
end end
......
...@@ -2,14 +2,14 @@ ...@@ -2,14 +2,14 @@
shared_examples 'set sort order from user preference' do shared_examples 'set sort order from user preference' do
describe '#set_sort_order_from_user_preference' do describe '#set_sort_order_from_user_preference' do
# There is no issuable_sorting_field defined in any CE controllers yet, # There is no sorting_field defined in any CE controllers yet,
# however any other field present in user_preferences table can be used for testing. # however any other field present in user_preferences table can be used for testing.
context 'when database is in read-only mode' do context 'when database is in read-only mode' do
it 'does not update user preference' do it 'does not update user preference' do
allow(Gitlab::Database).to receive(:read_only?).and_return(true) allow(Gitlab::Database).to receive(:read_only?).and_return(true)
expect_any_instance_of(UserPreference).not_to receive(:update).with({ controller.send(:issuable_sorting_field) => sorting_param }) expect_any_instance_of(UserPreference).not_to receive(:update).with({ controller.send(:sorting_field) => sorting_param })
get :index, params: { namespace_id: project.namespace, project_id: project, sort: sorting_param } get :index, params: { namespace_id: project.namespace, project_id: project, sort: sorting_param }
end end
...@@ -19,7 +19,7 @@ shared_examples 'set sort order from user preference' do ...@@ -19,7 +19,7 @@ shared_examples 'set sort order from user preference' do
it 'updates user preference' do it 'updates user preference' do
allow(Gitlab::Database).to receive(:read_only?).and_return(false) allow(Gitlab::Database).to receive(:read_only?).and_return(false)
expect_any_instance_of(UserPreference).to receive(:update).with({ controller.send(:issuable_sorting_field) => sorting_param }) expect_any_instance_of(UserPreference).to receive(:update).with({ controller.send(:sorting_field) => sorting_param })
get :index, params: { namespace_id: project.namespace, project_id: project, sort: sorting_param } get :index, params: { namespace_id: project.namespace, project_id: project, sort: sorting_param }
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