Refactor sorting logic in Issues and Projects controllers to use sanitize_sort method

Author: andrew

app/controllers/api/v1/issues_controller.rb

@@ -10,21 +10,12 @@ def index
 
     scope = scope.where('issues.created_at > ?', 1.day.ago) if params[:recent].present?
 
-    # Define allowed sort fields with database expressions
-    allowed_sort_fields = {
-      'created_at' => 'issues.created_at',
-      'updated_at' => 'issues.updated_at',
-      'stars' => "CAST(projects.repository->>'stargazers_count' AS INTEGER)"
-    }
-
     if params[:sort].present? || params[:order].present?
-      sort_key = params[:sort].presence || 'created_at'
-      sort_field = allowed_sort_fields[sort_key] || 'issues.created_at'
-      
+      sort = sanitize_sort(Issue.sortable_columns, default: 'created_at')
       if params[:order] == 'asc'
-        scope = scope.order(Arel.sql(sort_field).asc.nulls_last)
+        scope = scope.order(sort.asc.nulls_last)
       else
-        scope = scope.order(Arel.sql(sort_field).desc.nulls_last)
+        scope = scope.order(sort.desc.nulls_last)
       end
     else
       scope = scope.order('issues.created_at DESC')
@@ -39,11 +30,11 @@ def openclimateaction
     scope = Project.where(id: project_ids).active.reviewed.includes(:climatetriage_issues)
 
     if params[:sort].present? || params[:order].present?
-      sort = params[:sort].presence || 'projects.updated_at'
+      sort = sanitize_sort(Project.sortable_columns, default: 'projects.updated_at')
       if params[:order] == 'asc'
-        scope = scope.order(Arel.sql(sort).asc.nulls_last)
+        scope = scope.order(sort.asc.nulls_last)
       else
-        scope = scope.order(Arel.sql(sort).desc.nulls_last)
+        scope = scope.order(sort.desc.nulls_last)
       end
     else
       scope = scope.order('projects.updated_at DESC')

app/controllers/api/v1/projects_controller.rb

@@ -8,11 +8,11 @@ def index
     @projects = @projects.where(reviewed: true) if params[:reviewed] == 'true'
 
     if params[:sort].present? || params[:order].present?
-      sort = params[:sort].presence || 'projects.updated_at'
+      sort = sanitize_sort(Project.sortable_columns, default: 'projects.updated_at')
       if params[:order] == 'asc'
-        @projects = @projects.order(Arel.sql(sort).asc.nulls_last)
+        @projects = @projects.order(sort.asc.nulls_last)
       else
-        @projects = @projects.order(Arel.sql(sort).desc.nulls_last)
+        @projects = @projects.order(sort.desc.nulls_last)
       end
     end
 
@@ -64,11 +64,11 @@ def esd
     @projects = Project.all.where.not(last_synced_at: nil).where(esd: true)
 
     if params[:sort].present? || params[:order].present?
-      sort = params[:sort].presence || 'projects.updated_at'
+      sort = sanitize_sort(Project.sortable_columns, default: 'projects.updated_at')
       if params[:order] == 'asc'
-        @projects = @projects.order(Arel.sql(sort).asc.nulls_last)
+        @projects = @projects.order(sort.asc.nulls_last)
       else
-        @projects = @projects.order(Arel.sql(sort).desc.nulls_last)
+        @projects = @projects.order(sort.desc.nulls_last)
       end
     end
 

app/controllers/application_controller.rb

@@ -8,6 +8,12 @@ def default_url_options(options = {})
     Rails.env.production? ? { :protocol => "https" }.merge(options) : options
   end
 
+  def sanitize_sort(allowed_columns, default: 'updated_at')
+    sort_param = params[:sort].presence || default
+    sql = allowed_columns[sort_param] || allowed_columns[default] || default
+    Arel.sql(sql)
+  end
+
   def set_cache_headers(browser_ttl: 5.minutes, cdn_ttl: 6.hours)
     return unless request.get?
     response.headers['Cache-Control'] = "public, max-age=#{browser_ttl.to_i}, s-maxage=#{cdn_ttl.to_i}, stale-while-revalidate=#{cdn_ttl.to_i}, stale-if-error=#{1.day.to_i}"

app/controllers/issues_controller.rb

@@ -7,21 +7,12 @@ def index
     scope = scope.joins(:project).merge(Project.language(params[:language])) if params[:language].present?
     scope = scope.joins(:project).merge(Project.keyword(params[:keyword])) if params[:keyword].present?
 
-    # Define allowed sort fields with database expressions
-    allowed_sort_fields = {
-      'created_at' => 'issues.created_at',
-      'updated_at' => 'issues.updated_at',
-      'stars' => "CAST(projects.repository->>'stargazers_count' AS INTEGER)"
-    }
-
     if params[:sort].present? || params[:order].present?
-      sort_key = params[:sort].presence || 'created_at'
-      sort_field = allowed_sort_fields[sort_key] || 'issues.created_at'
-      
+      sort = sanitize_sort(Issue.sortable_columns, default: 'created_at')
       if params[:order] == 'asc'
-        scope = scope.order(Arel.sql(sort_field).asc.nulls_last)
+        scope = scope.order(sort.asc.nulls_last)
       else
-        scope = scope.order(Arel.sql(sort_field).desc.nulls_last)
+        scope = scope.order(sort.desc.nulls_last)
       end
     else
       scope = scope.order('issues.created_at DESC')

app/models/issue.rb

@@ -1,4 +1,12 @@
 class Issue < ApplicationRecord
+  def self.sortable_columns
+    {
+      'created_at' => 'issues.created_at',
+      'updated_at' => 'issues.updated_at',
+      'stars' => "CAST(projects.repository->>'stargazers_count' AS INTEGER)",
+    }
+  end
+
   belongs_to :project
 
   scope :past_year, -> { where('created_at > ?', 1.year.ago) }

app/models/project.rb

@@ -3,6 +3,18 @@
 class Project < ApplicationRecord
   include EcosystemApiClient
 
+  def self.sortable_columns
+    {
+      'projects.updated_at' => 'projects.updated_at',
+      'projects.created_at' => 'projects.created_at',
+      'updated_at' => 'updated_at',
+      'created_at' => 'created_at',
+      'last_synced_at' => 'last_synced_at',
+      'score' => 'score',
+      'name' => 'name',
+    }
+  end
+
   SEARCH_TSVECTOR = <<~SQL.squish
     to_tsvector('english',
       coalesce(projects.name, '') || ' ' ||