Commit 83961f97 authored by Douwe Maan's avatar Douwe Maan Committed by Rémy Coutable

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
parent e257e81c
...@@ -4,6 +4,7 @@ v 8.6.1 (unreleased) ...@@ -4,6 +4,7 @@ v 8.6.1 (unreleased)
- Add option to reload the schema before restoring a database backup. !2807 - Add option to reload the schema before restoring a database backup. !2807
- Display navigation controls on mobile. !3214 - Display navigation controls on mobile. !3214
- Fixed bug where participants would not work correctly on merge requests. !3329 - Fixed bug where participants would not work correctly on merge requests. !3329
- Fix sorting issues by votes on the groups issues page results in SQL errors. !3333
- Restrict notifications for confidential issues. !3334 - Restrict notifications for confidential issues. !3334
- Do not allow to move issue if it has not been persisted. !3340 - Do not allow to move issue if it has not been persisted. !3340
- Add a confirmation step before deleting an issuable. !3341 - Add a confirmation step before deleting an issuable. !3341
......
...@@ -41,7 +41,7 @@ module Issuable ...@@ -41,7 +41,7 @@ module Issuable
scope :join_project, -> { joins(:project) } scope :join_project, -> { joins(:project) }
scope :references_project, -> { references(: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, delegate :name,
:email, :email,
......
require 'rails_helper' require 'rails_helper'
describe GroupsController do 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 context 'as a user' do
it 'redirects to Groups Dashboard' do it 'redirects to Groups Dashboard' do
sign_in(create(:user)) sign_in(user)
get :index get :index
...@@ -20,4 +25,54 @@ describe GroupsController do ...@@ -20,4 +25,54 @@ describe GroupsController do
end end
end 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 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