RoutingConstraintZones#index: Fix sort in table

Sorting by stop point count and route name wasn't working. Only sorting
by name worked.

This was because these HTML table columns get their data from methods on
the `RoutingConstraintZone` model (`#stop_points_count` and
`#route_name`). Those methods can't be sorted on in the database.
Furthermore, when setting up the order ActiveRecord call, only database
column names on `routing_constraint_zones` were taken into account. When
a `sort` parameter specified an identifier that wasn't a column name,
the `name` column would be used to sort the list.

In this change, we enable sorting by stop point count and by route name.
The SQL work for ordering comes from a couple of new scope methods
defined on the model. Xin explained that he had previously used scope
methods to enable exotic sorting of Referentials on the
Workbenches#show page. I couldn't think of a better way to deal with
these, and it seems like a reasonable thing to do here, so I went with
the same setup.

In `#sort_column`, tack on our custom sorting keys to the list of
accepted keys. This borrows the pattern used in other `#sort_column`
methods in other controllers. Ended up using "route" as the route name
sorting key because that's what gets passed from the frontend.

Eliminated the `if` condition in `#collection` that defaults to `name`
sorting because it was redundant. The `#sort_column` method already
defaults to sorting by name if no `params[:sort]` is passed in. This
makes the method a lot cleaner, nice.

Add a new private method `#sort_collection` that determines what method
to call to sort the query. When using real table column names, it calls
`#order` like before. But now, if the user asked to sort by stop point
count or route name, we delegate instead to our scope methods,
`#order_by_stop_points_count` and `#order_by_route_name`.

Refs #4130

2 files changed, 33 insertions, 7 deletions

diff --git a/app/controllers/routing_constraint_zones_controller.rb b/app/controllers/routing_constraint_zones_controller.rb
index 6c3cb8a29..6541eb492 100644
--- a/app/controllers/routing_constraint_zones_controller.rb
+++ b/app/controllers/routing_constraint_zones_controller.rb
@@ -58,23 +58,40 @@ class RoutingConstraintZonesController < ChouetteController
     @q = current_referential.routing_constraint_zones.search(params[:q])
 
     @routing_constraint_zones ||= begin
-      if sort_column && sort_direction
-        routing_constraint_zones = @q.result(distinct: true).order(sort_column + ' ' + sort_direction)
-      else
-        routing_constraint_zones = @q.result(distinct: true).order(:name)
-      end
-      routing_constraint_zones = routing_constraint_zones.paginate(page: params[:page], per_page: 10)
+      routing_constraint_zones = sort_collection
+      routing_constraint_zones = routing_constraint_zones.paginate(
+        page: params[:page],
+        per_page: 10
+      )
     end
   end
 
   private
   def sort_column
-    (Chouette::RoutingConstraintZone.column_names).include?(params[:sort]) ? params[:sort] : 'name'
+    (
+      Chouette::RoutingConstraintZone.column_names +
+      [
+        'stop_points_count',
+        'route'
+      ]
+    ).include?(params[:sort]) ? params[:sort] : 'name'
   end
   def sort_direction
     %w[asc desc].include?(params[:direction]) ?  params[:direction] : 'asc'
   end
 
+  def sort_collection
+    sort_by = sort_column
+
+    if sort_by == 'stop_points_count'
+      @q.result.order_by_stop_points_count(sort_direction)
+    elsif sort_by == 'route'
+      @q.result.order_by_route_name(sort_direction)
+    else
+      @q.result.order(sort_column + ' ' + sort_direction)
+    end
+  end
+
   def routing_constraint_zone_params
     params.require(:routing_constraint_zone).permit(
       :name,
diff --git a/app/models/chouette/routing_constraint_zone.rb b/app/models/chouette/routing_constraint_zone.rb
index f9bdec7e4..a649b0b8e 100644
--- a/app/models/chouette/routing_constraint_zone.rb
+++ b/app/models/chouette/routing_constraint_zone.rb
@@ -6,6 +6,15 @@ class Chouette::RoutingConstraintZone < Chouette::TridentActiveRecord
   # validates :stop_point_ids, length: { minimum: 2, too_short: I18n.t('activerecord.errors.models.routing_constraint_zone.attributes.stop_points.not_enough_stop_points') }
   validate :stop_points_belong_to_route, :not_all_stop_points_selected
 
+  scope :order_by_stop_points_count, ->(direction) do
+    order("array_length(stop_point_ids, 1) #{direction}")
+  end
+
+  scope :order_by_route_name, ->(direction) do
+    joins(:route)
+      .order("routes.name #{direction}")
+  end
+
   def stop_points_belong_to_route
     errors.add(:stop_point_ids, I18n.t('activerecord.errors.models.routing_constraint_zone.attributes.stop_points.stop_points_not_from_route')) unless stop_points.all? { |sp| route.stop_points.include? sp }
   end