diff options
author | Douwe Maan <douwe@gitlab.com> | 2016-03-23 10:35:54 +0000 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2016-03-23 10:35:54 +0000 |
commit | 703f7c5d579a6ab6d7592c24f8e3f2813d9d1703 (patch) | |
tree | 831b73106a0e4fce56bdea27b23229f7856d8f0f | |
parent | 3b39ce325263028553c6019cc894024c207644ea (diff) | |
parent | 100e3e7601dd4d3033cd8292814273dd0354722e (diff) | |
download | gitlab-ce-703f7c5d579a6ab6d7592c24f8e3f2813d9d1703.tar.gz |
Merge branch 'fix-sorting-by-votes-on-groups-page' into 'master'
Fix sorting issues/mrs by votes on the groups page
Closes #14394
The `non_archived` scope applied here https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/controllers/concerns/issues_action.rb#L5 overrides the previous `ORDER BY` applied inside the IssuesFinder, with the default scope of the Project model, resulting in SQL errors.
```ruby
Issue.reorder(created_at: :desc).joins(:project).to_sql
=> "SELECT issues.*
FROM issues INNER JOIN projects ON projects.id = issues.project_id
ORDER BY issues.created_at DESC"
Issue.reorder(created_at: :desc).joins(:project).merge(Project.non_archived).to_sql
=> "SELECT issues.*
FROM issues INNER JOIN projects ON projects.id = issues.project_id
WHERE projects.archived = 'f'
ORDER BY projects.id DESC"
Issue.reorder(created_at: :desc).joins(:project).merge(Project.non_archived.only(:where)).to_sql
=> "SELECT issues.*
FROM issues INNER JOIN projects ON projects.id = issues.project_id
WHERE projects.archived = 'f'
ORDER BY issues.created_at DESC"
```
/cc @yorickpeterse
See merge request !3333
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | app/models/concerns/issuable.rb | 2 | ||||
-rw-r--r-- | spec/controllers/groups_controller_spec.rb | 59 |
3 files changed, 59 insertions, 3 deletions
diff --git a/CHANGELOG b/CHANGELOG index 1cc8f377a6a..924358a4f0b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -70,6 +70,7 @@ v 8.6.0 - Canceled builds are now ignored in compound build status if marked as `allowed to fail` - Trigger a todo for mentions on commits page - Let project owners and admins soft delete issues and merge requests + - Fix sorting issues by votes on the groups issues page results in SQL errors v 8.5.8 - Bump Git version requirement to 2.7.4 diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 476e1ce7af0..cf5b2c71675 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -41,7 +41,7 @@ module Issuable scope :join_project, -> { joins(:project) } scope :references_project, -> { references(:project) } - scope :non_archived, -> { join_project.merge(Project.non_archived) } + scope :non_archived, -> { join_project.merge(Project.non_archived.only(:where)) } delegate :name, :email, diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index 938e97298b6..465531b2b36 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -1,10 +1,15 @@ require 'rails_helper' describe GroupsController do - describe 'GET index' do + let(:user) { create(:user) } + let(:group) { create(:group) } + let(:project) { create(:project, namespace: group) } + let!(:group_member) { create(:group_member, group: group, user: user) } + + describe 'GET #index' do context 'as a user' do it 'redirects to Groups Dashboard' do - sign_in(create(:user)) + sign_in(user) get :index @@ -20,4 +25,54 @@ describe GroupsController do end end end + + describe 'GET #issues' do + let(:issue_1) { create(:issue, project: project) } + let(:issue_2) { create(:issue, project: project) } + + before do + create_list(:upvote_note, 3, project: project, noteable: issue_2) + create_list(:upvote_note, 2, project: project, noteable: issue_1) + create_list(:downvote_note, 2, project: project, noteable: issue_2) + + sign_in(user) + end + + context 'sorting by votes' do + it 'sorts most popular issues' do + get :issues, id: group.to_param, sort: 'upvotes_desc' + expect(assigns(:issues)).to eq [issue_2, issue_1] + end + + it 'sorts least popular issues' do + get :issues, id: group.to_param, sort: 'downvotes_desc' + expect(assigns(:issues)).to eq [issue_2, issue_1] + end + end + end + + describe 'GET #merge_requests' do + let(:merge_request_1) { create(:merge_request, source_project: project) } + let(:merge_request_2) { create(:merge_request, :simple, source_project: project) } + + before do + create_list(:upvote_note, 3, project: project, noteable: merge_request_2) + create_list(:upvote_note, 2, project: project, noteable: merge_request_1) + create_list(:downvote_note, 2, project: project, noteable: merge_request_2) + + sign_in(user) + end + + context 'sorting by votes' do + it 'sorts most popular merge requests' do + get :merge_requests, id: group.to_param, sort: 'upvotes_desc' + expect(assigns(:merge_requests)).to eq [merge_request_2, merge_request_1] + end + + it 'sorts least popular merge requests' do + get :merge_requests, id: group.to_param, sort: 'downvotes_desc' + expect(assigns(:merge_requests)).to eq [merge_request_2, merge_request_1] + end + end + end end |