From a6a4031aa2b10202ae9899efa1607a059b886730 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Oliver=20G=C3=BCnther?= <mail@oliverguenther.de>
Date: Fri, 15 Nov 2019 11:16:27 +0100
Subject: [PATCH] [31667] Fix ambiguous reference to project_id when group/sort

Properly reference project_id to avoid ambiguous references to
project_id when grouping by project_id and sorting by priority

https://community.openproject.com/wp/31667
---
 .../work_packages/columns/property_column.rb  | 18 +++++------
 spec/models/query/results_spec.rb             | 32 +++++++++++++++++++
 2 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/app/models/queries/work_packages/columns/property_column.rb b/app/models/queries/work_packages/columns/property_column.rb
index 51d4f50f6c..8b88cb7f5b 100644
--- a/app/models/queries/work_packages/columns/property_column.rb
+++ b/app/models/queries/work_packages/columns/property_column.rb
@@ -43,7 +43,7 @@ class Queries::WorkPackages::Columns::PropertyColumn < Queries::WorkPackages::Co
     project: {
       association: 'project',
       sortable: "name",
-      groupable: 'project_id'
+      groupable: "#{WorkPackage.table_name}.project_id"
     },
     subject: {
       sortable: "#{WorkPackage.table_name}.subject"
@@ -51,7 +51,7 @@ class Queries::WorkPackages::Columns::PropertyColumn < Queries::WorkPackages::Co
     type: {
       association: 'type',
       sortable: "position",
-      groupable: 'type_id'
+      groupable: "#{WorkPackage.table_name}.type_id"
     },
     parent: {
       association: 'ancestors_relations',
@@ -62,35 +62,35 @@ class Queries::WorkPackages::Columns::PropertyColumn < Queries::WorkPackages::Co
       association: 'status',
       sortable: "position",
       highlightable: true,
-      groupable: 'status_id'
+      groupable: "#{WorkPackage.table_name}.status_id"
     },
     priority: {
       association: 'priority',
       sortable: "position",
       default_order: 'desc',
       highlightable: true,
-      groupable: 'priority_id'
+      groupable: "#{WorkPackage.table_name}.priority_id"
     },
     author: {
       association: 'author',
       sortable: ["lastname",
                  "firstname",
                  "id"],
-      groupable: 'author_id'
+      groupable: "#{WorkPackage.table_name}.author_id"
     },
     assigned_to: {
       association: 'assigned_to',
       sortable: ["lastname",
                  "firstname",
                  "id"],
-      groupable: 'assigned_to_id'
+      groupable: "#{WorkPackage.table_name}.assigned_to_id"
     },
     responsible: {
       association: 'responsible',
       sortable: ["lastname",
                  "firstname",
                  "id"],
-      groupable: 'responsible_id'
+      groupable: "#{WorkPackage.table_name}.responsible_id"
     },
     updated_at: {
       sortable: "#{WorkPackage.table_name}.updated_at",
@@ -99,13 +99,13 @@ class Queries::WorkPackages::Columns::PropertyColumn < Queries::WorkPackages::Co
     category: {
       association: 'category',
       sortable: "name",
-      groupable: 'category_id'
+      groupable: "#{WorkPackage.table_name}.category_id"
     },
     fixed_version: {
       association: 'fixed_version',
       sortable: ["name"],
       default_order: 'desc',
-      groupable: 'fixed_version_id'
+      groupable: "#{WorkPackage.table_name}.fixed_version_id"
     },
     start_date: {
       # Put empty start_dates in the far future rather than in the far past
diff --git a/spec/models/query/results_spec.rb b/spec/models/query/results_spec.rb
index 1c4f3063eb..86d3421696 100644
--- a/spec/models/query/results_spec.rb
+++ b/spec/models/query/results_spec.rb
@@ -604,6 +604,38 @@ describe ::Query::Results, type: :model, with_mail: false do
       end
     end
 
+    context 'sorting by priority, grouping by project' do
+      let(:prio_low) { FactoryBot.create :issue_priority, position: 1 }
+      let(:prio_high) { FactoryBot.create :issue_priority, position: 0 }
+      let(:group_by) { 'project' }
+
+      before do
+        allow(User).to receive(:current).and_return(user_1)
+
+        work_package1.priority = prio_low
+        work_package2.priority = prio_high
+
+        work_package1.save(validate: false)
+        work_package2.save(validate: false)
+      end
+
+      it 'properly selects project_id (Regression #31667)' do
+        query.sort_criteria = [['priority', 'asc']]
+
+        expect(query_results.sorted_work_packages)
+          .to match [work_package1, work_package2]
+
+        query.sort_criteria = [['priority', 'desc']]
+
+        expect(query_results.sorted_work_packages)
+          .to match [work_package2, work_package1]
+
+        group_count = query_results.work_package_count_by_group
+
+        expect(group_count).to eq({ project_1 => 2 })
+      end
+    end
+
     context 'sorting by author and responsible, grouping by assigned_to' do
       let(:group_by) { 'assigned_to' }
       let(:sort_by) { [['author', 'asc'], ['responsible', 'desc']] }