diff options
author | Mario de la Ossa <mariodelaossa@gmail.com> | 2019-01-06 18:00:48 -0600 |
---|---|---|
committer | Mario de la Ossa <mariodelaossa@gmail.com> | 2019-01-28 12:48:05 -0600 |
commit | 49c74068ae7f0017ebeb8e7daa2c556fef3124e3 (patch) | |
tree | 76200baeb51199910ce81b77736da2491bcd33f5 | |
parent | 958a819fce709419c0be76cec8a20c8e8417ab84 (diff) | |
download | gitlab-ce-49c74068ae7f0017ebeb8e7daa2c556fef3124e3.tar.gz |
Save sorting preference for Issues/MRs in BE
In order to let users' sorting preferences transfer between devices, we
save the preference for issues and MRs (one preference for issues, one
for MRs) in the backend inside the UserPreference object
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 |