diff options
17 files changed, 151 insertions, 45 deletions
diff --git a/app/controllers/concerns/issuable_collections.rb b/app/controllers/concerns/issuable_collections.rb index 789e0dc736e..07d0bf16d93 100644 --- a/app/controllers/concerns/issuable_collections.rb +++ b/app/controllers/concerns/issuable_collections.rb @@ -129,13 +129,13 @@ module IssuableCollections return sort_param if Gitlab::Database.read_only? if user_preference[issuable_sorting_field] != sort_param - user_preference.update_attribute(issuable_sorting_field, sort_param) + user_preference.update(issuable_sorting_field => sort_param) end sort_param end - # Implement default_sorting_field method on controllers + # Implement issuable_sorting_field method on controllers # to choose which column to store the sorting parameter. def issuable_sorting_field nil diff --git a/app/controllers/concerns/issues_action.rb b/app/controllers/concerns/issuable_collections_action.rb index a75590457d6..18ed4027eac 100644 --- a/app/controllers/concerns/issues_action.rb +++ b/app/controllers/concerns/issuable_collections_action.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -module IssuesAction +module IssuableCollectionsAction extend ActiveSupport::Concern include IssuableCollections include IssuesCalendar @@ -18,6 +18,12 @@ module IssuesAction format.atom { render layout: 'xml.atom' } end end + + def merge_requests + @merge_requests = issuables_collection.page(params[:page]) + + @issuable_meta_data = issuable_meta_data(@merge_requests, collection_type) + end # rubocop:enable Gitlab/ModuleWithInstanceVariables def issues_calendar @@ -26,8 +32,29 @@ module IssuesAction private + def issuable_sorting_field + case action_name + when 'issues' + Issue::SORTING_PREFERENCE_FIELD + when 'merge_requests' + MergeRequest::SORTING_PREFERENCE_FIELD + else + nil + end + end + def finder_type - (super if defined?(super)) || - (IssuesFinder if %w(issues issues_calendar).include?(action_name)) + case action_name + when 'issues', 'issues_calendar' + IssuesFinder + when 'merge_requests' + MergeRequestsFinder + else + nil + end + end + + def finder_options + super.merge(non_archived: true) end end diff --git a/app/controllers/concerns/merge_requests_action.rb b/app/controllers/concerns/merge_requests_action.rb deleted file mode 100644 index ed10f32512e..00000000000 --- a/app/controllers/concerns/merge_requests_action.rb +++ /dev/null @@ -1,25 +0,0 @@ -# frozen_string_literal: true - -module MergeRequestsAction - extend ActiveSupport::Concern - include IssuableCollections - - # rubocop:disable Gitlab/ModuleWithInstanceVariables - def merge_requests - @merge_requests = issuables_collection.page(params[:page]) - - @issuable_meta_data = issuable_meta_data(@merge_requests, collection_type) - end - # rubocop:enable Gitlab/ModuleWithInstanceVariables - - private - - def finder_type - (super if defined?(super)) || - (MergeRequestsFinder if action_name == 'merge_requests') - end - - def finder_options - super.merge(non_archived: true) - end -end diff --git a/app/controllers/dashboard_controller.rb b/app/controllers/dashboard_controller.rb index be2d9512c01..75329b05a6f 100644 --- a/app/controllers/dashboard_controller.rb +++ b/app/controllers/dashboard_controller.rb @@ -1,8 +1,7 @@ # frozen_string_literal: true class DashboardController < Dashboard::ApplicationController - include IssuesAction - include MergeRequestsAction + include IssuableCollectionsAction prepend_before_action(only: [:issues]) { authenticate_sessionless_user!(:rss) } prepend_before_action(only: [:issues_calendar]) { authenticate_sessionless_user!(:ics) } diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index c5d8ac2ed77..15aadf3f74b 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -2,8 +2,7 @@ class GroupsController < Groups::ApplicationController include API::Helpers::RelatedResourcesHelpers - include IssuesAction - include MergeRequestsAction + include IssuableCollectionsAction include ParamsBackwardCompatibility include PreviewMarkdown diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index e3e60665506..fd5f3eeaa99 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -191,6 +191,10 @@ class Projects::IssuesController < Projects::ApplicationController protected + def issuable_sorting_field + Issue::SORTING_PREFERENCE_FIELD + end + # rubocop: disable CodeReuse/ActiveRecord def issue return @issue if defined?(@issue) diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index accc37557b0..bc0a3d3526d 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -230,6 +230,10 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo alias_method :issuable, :merge_request alias_method :awardable, :merge_request + def issuable_sorting_field + MergeRequest::SORTING_PREFERENCE_FIELD + end + def merge_params params.permit(merge_params_attributes) end diff --git a/app/models/issue.rb b/app/models/issue.rb index 5c4ecbfdf4e..182c5d3d4b0 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -26,6 +26,8 @@ class Issue < ActiveRecord::Base DueThisMonth = DueDateStruct.new('Due This Month', 'month').freeze DueNextMonthAndPreviousTwoWeeks = DueDateStruct.new('Due Next Month And Previous Two Weeks', 'next_month_and_previous_two_weeks').freeze + SORTING_PREFERENCE_FIELD = :issues_sort + belongs_to :project belongs_to :moved_to, class_name: 'Issue' belongs_to :closed_by, class_name: 'User' diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 7206d858dae..84cb8e1c50b 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -21,6 +21,8 @@ class MergeRequest < ActiveRecord::Base self.reactive_cache_refresh_interval = 10.minutes self.reactive_cache_lifetime = 10.minutes + SORTING_PREFERENCE_FIELD = :merge_requests_sort + ignore_column :locked_at, :ref_fetched, :deleted_at diff --git a/changelogs/unreleased/50352-sort-save.yml b/changelogs/unreleased/50352-sort-save.yml new file mode 100644 index 00000000000..cd046c8b785 --- /dev/null +++ b/changelogs/unreleased/50352-sort-save.yml @@ -0,0 +1,5 @@ +--- +title: Save issues/merge request sorting options to backend +merge_request: 24198 +author: +type: added diff --git a/db/migrate/20190116234221_add_sorting_fields_to_user_preference.rb b/db/migrate/20190116234221_add_sorting_fields_to_user_preference.rb new file mode 100644 index 00000000000..7bf581fe9b0 --- /dev/null +++ b/db/migrate/20190116234221_add_sorting_fields_to_user_preference.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddSortingFieldsToUserPreference < ActiveRecord::Migration[5.0] + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + def up + add_column :user_preferences, :issues_sort, :string + add_column :user_preferences, :merge_requests_sort, :string + end + + def down + remove_column :user_preferences, :issues_sort + remove_column :user_preferences, :merge_requests_sort + end +end diff --git a/db/schema.rb b/db/schema.rb index 859f007dbe1..7c1733becb9 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -2146,6 +2146,8 @@ ActiveRecord::Schema.define(version: 20190124200344) do t.integer "merge_request_notes_filter", limit: 2, default: 0, null: false t.datetime_with_timezone "created_at", null: false t.datetime_with_timezone "updated_at", null: false + t.string "issues_sort" + t.string "merge_requests_sort" t.index ["user_id"], name: "index_user_preferences_on_user_id", unique: true, using: :btree end diff --git a/spec/controllers/concerns/issuable_collections_spec.rb b/spec/controllers/concerns/issuable_collections_spec.rb index 5a3a7a15f5a..307c5d60c57 100644 --- a/spec/controllers/concerns/issuable_collections_spec.rb +++ b/spec/controllers/concerns/issuable_collections_spec.rb @@ -17,10 +17,55 @@ describe IssuableCollections do controller = klass.new allow(controller).to receive(:params).and_return(ActionController::Parameters.new(params)) + allow(controller).to receive(:current_user).and_return(user) controller 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) { {} } diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index a2c3bb2919d..8ea5b4ea09c 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -42,7 +42,9 @@ describe Projects::IssuesController do it_behaves_like "issuables list meta-data", :issue - it_behaves_like 'set sort order from user preference' + it_behaves_like 'set sort order from user preference' do + let(:sorting_param) { 'updated_asc' } + end it "returns index" do get :index, params: { namespace_id: project.namespace, project_id: project } diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 53d5bf752ef..01a27f0429b 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -153,7 +153,9 @@ describe Projects::MergeRequestsController do it_behaves_like "issuables list meta-data", :merge_request - it_behaves_like 'set sort order from user preference' + it_behaves_like 'set sort order from user preference' do + let(:sorting_param) { 'updated_asc' } + end context 'when page param' do let(:last_page) { project.merge_requests.page().total_pages } diff --git a/spec/models/user_preference_spec.rb b/spec/models/user_preference_spec.rb index 2898613545c..b2ef17a81d4 100644 --- a/spec/models/user_preference_spec.rb +++ b/spec/models/user_preference_spec.rb @@ -3,9 +3,10 @@ require 'spec_helper' describe UserPreference do + let(:user_preference) { create(:user_preference) } + describe '#set_notes_filter' do let(:issuable) { build_stubbed(:issue) } - let(:user_preference) { create(:user_preference) } shared_examples 'setting system notes' do it 'returns updated discussion filter' do @@ -50,4 +51,26 @@ describe UserPreference do end end end + + describe 'sort_by preferences' do + shared_examples_for 'a sort_by preference' do + it 'allows nil sort fields' do + user_preference.update(attribute => nil) + + expect(user_preference).to be_valid + end + end + + context 'merge_requests_sort attribute' do + let(:attribute) { :merge_requests_sort } + + it_behaves_like 'a sort_by preference' + end + + context 'issues_sort attribute' do + let(:attribute) { :issues_sort } + + it_behaves_like 'a sort_by preference' + end + end end diff --git a/spec/support/shared_examples/controllers/set_sort_order_from_user_preference_shared_examples.rb b/spec/support/shared_examples/controllers/set_sort_order_from_user_preference_shared_examples.rb index d86838719d4..98ab04c5636 100644 --- a/spec/support/shared_examples/controllers/set_sort_order_from_user_preference_shared_examples.rb +++ b/spec/support/shared_examples/controllers/set_sort_order_from_user_preference_shared_examples.rb @@ -2,18 +2,12 @@ shared_examples '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, # however any other field present in user_preferences table can be used for testing. - let(:sorting_field) { :issue_notes_filter } - let(:sorting_param) { 'any' } - - before do - allow(controller).to receive(:issuable_sorting_field).and_return(sorting_field) - end context 'when database is in read-only mode' do it 'it does not update user preference' do allow(Gitlab::Database).to receive(:read_only?).and_return(true) - expect_any_instance_of(UserPreference).not_to receive(:update_attribute).with(sorting_field, sorting_param) + expect_any_instance_of(UserPreference).not_to receive(:update).with({ controller.send(:issuable_sorting_field) => sorting_param }) get :index, params: { namespace_id: project.namespace, project_id: project, sort: sorting_param } end @@ -23,7 +17,7 @@ shared_examples 'set sort order from user preference' do it 'updates user preference' do allow(Gitlab::Database).to receive(:read_only?).and_return(false) - expect_any_instance_of(UserPreference).to receive(:update_attribute).with(sorting_field, sorting_param) + expect_any_instance_of(UserPreference).to receive(:update).with({ controller.send(:issuable_sorting_field) => sorting_param }) get :index, params: { namespace_id: project.namespace, project_id: project, sort: sorting_param } end |