From 6e974250f9d283022d2421370e5e97ff6f26a3be Mon Sep 17 00:00:00 2001
From: Justin DiRose <justin@justindirose.com>
Date: Wed, 4 Nov 2020 16:46:55 -0600
Subject: [PATCH] FIX: Sort query was ambiguous (#15)

After implementing changes to tagging, this introduced ambiguity in the SQL query for sorting by activity. This commit fixes the ambiguity by explicitly defining the table from where the columns are used. Also adds tests to check for regressions in the future.
---
 lib/knowledge_explorer/query.rb               |  8 +--
 .../knowledge_explorer_controller_spec.rb     | 58 +++++++++++++++++++
 2 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/lib/knowledge_explorer/query.rb b/lib/knowledge_explorer/query.rb
index 9bbb6af..3348bee 100644
--- a/lib/knowledge_explorer/query.rb
+++ b/lib/knowledge_explorer/query.rb
@@ -77,15 +77,15 @@ module KnowledgeExplorer
 
       if @filters[:order] == "title"
         if @filters[:ascending].present?
-          results = results.reorder('title')
+          results = results.reorder('topics.title')
         else
-          results = results.reorder('title DESC')
+          results = results.reorder('topics.title DESC')
         end
       elsif @filters[:order] == "activity"
         if @filters[:ascending].present?
-          results = results.reorder('last_posted_at')
+          results = results.reorder('topics.last_posted_at')
         else
-          results = results.reorder('last_posted_at DESC')
+          results = results.reorder('topics.last_posted_at DESC')
         end
       end
 
diff --git a/spec/requests/knowledge_explorer_controller_spec.rb b/spec/requests/knowledge_explorer_controller_spec.rb
index 594fcac..74fdb70 100644
--- a/spec/requests/knowledge_explorer_controller_spec.rb
+++ b/spec/requests/knowledge_explorer_controller_spec.rb
@@ -113,6 +113,64 @@ describe KnowledgeExplorer::KnowledgeExplorerController do
       end
     end
 
+    context 'when ordering results' do
+      context 'by title' do
+        it 'should return the list ordered descending' do
+          get "/docs.json?order=title"
+
+          expect(response.status).to eq(200)
+
+          json = response.parsed_body
+          topics = json['topics']['topic_list']['topics']
+
+          expect(topics[0]['id']).to eq(topic2.id)
+          expect(topics[1]['id']).to eq(topic.id)
+        end
+
+        it 'should return the list ordered ascending with an additional parameter' do
+          get "/docs.json?order=title&ascending=true"
+
+          expect(response.status).to eq(200)
+
+          json = response.parsed_body
+          topics = json['topics']['topic_list']['topics']
+
+          expect(topics[0]['id']).to eq(topic.id)
+          expect(topics[1]['id']).to eq(topic2.id)
+        end
+      end
+
+      context 'by date' do
+        before do
+          topic2.update(last_posted_at: Time.zone.now + 100)
+        end
+
+        it 'should return the list ordered descending' do
+          get "/docs.json?order=activity"
+
+          expect(response.status).to eq(200)
+
+          json = response.parsed_body
+          topics = json['topics']['topic_list']['topics']
+
+          expect(topics[0]['id']).to eq(topic.id)
+          expect(topics[1]['id']).to eq(topic2.id)
+        end
+
+        it 'should return the list ordered ascending with an additional parameter' do
+          get "/docs.json?order=activity&ascending=true"
+
+          expect(response.status).to eq(200)
+
+          json = response.parsed_body
+          topics = json['topics']['topic_list']['topics']
+
+          expect(topics[0]['id']).to eq(topic2.id)
+          expect(topics[1]['id']).to eq(topic.id)
+        end
+      end
+    end
+
     context 'when searching' do
       before do
         SearchIndexer.enable