Merge pull request #9 from af83/3268-vehicle-journey-times--order-vehicle-journeys-display-journey-with-earliest-stop-first--rb20170504170900

3268 vehicle journey times  order vehicle journeys display journey with earliest stop first  rb20170504170900

4 files changed, 128 insertions, 16 deletions

diff --git a/app/controllers/vehicle_journeys_controller.rb b/app/controllers/vehicle_journeys_controller.rb
index c084b592a..f692f3628 100644
--- a/app/controllers/vehicle_journeys_controller.rb
+++ b/app/controllers/vehicle_journeys_controller.rb
@@ -77,14 +77,22 @@ class VehicleJourneysController < ChouetteController
 
   protected
   def collection
-    scope = route.vehicle_journeys.joins(:journey_pattern).joins('LEFT JOIN "vehicle_journey_at_stops" ON "vehicle_journey_at_stops"."vehicle_journey_id" = "vehicle_journeys"."id"')
+    scope = route.vehicle_journeys
+      .joins(:journey_pattern)
+      .joins('
+        LEFT JOIN "vehicle_journey_at_stops"
+          ON "vehicle_journey_at_stops"."vehicle_journey_id" = "vehicle_journeys"."id"
+          AND "vehicle_journey_at_stops"."stop_point_id" =
+            "journey_patterns"."departure_stop_point_id"
+      ')
+      .order("vehicle_journey_at_stops.departure_time")
 
     @q = scope.search filtered_ransack_params
     grouping = ransack_periode_filter
     @q.build_grouping(grouping) if grouping
 
     @ppage = 20
-    @vehicle_journeys = @q.result(distinct: true).paginate(:page => params[:page], :per_page => @ppage)
+    @vehicle_journeys = @q.result.paginate(:page => params[:page], :per_page => @ppage)
     @footnotes = route.line.footnotes.to_json
     @matrix    = resource_class.matrix(@vehicle_journeys)
     @vehicle_journeys
diff --git a/app/models/chouette/vehicle_journey.rb b/app/models/chouette/vehicle_journey.rb
index 4d7d596d8..297e462f0 100644
--- a/app/models/chouette/vehicle_journey.rb
+++ b/app/models/chouette/vehicle_journey.rb
@@ -209,5 +209,18 @@ module Chouette
       end
     end
 
+    def self.with_stops
+      self
+        .joins(:journey_pattern)
+        .joins('
+          LEFT JOIN "vehicle_journey_at_stops"
+            ON "vehicle_journey_at_stops"."vehicle_journey_id" =
+              "vehicle_journeys"."id"
+            AND "vehicle_journey_at_stops"."stop_point_id" =
+              "journey_patterns"."departure_stop_point_id"
+        ')
+        .order("vehicle_journey_at_stops.departure_time")
+    end
+
   end
 end
diff --git a/spec/factories/chouette_vehicle_journey.rb b/spec/factories/chouette_vehicle_journey.rb
index b7c5e37d5..9ba660800 100644
--- a/spec/factories/chouette_vehicle_journey.rb
+++ b/spec/factories/chouette_vehicle_journey.rb
@@ -3,29 +3,31 @@ FactoryGirl.define do
   factory :vehicle_journey_common, :class => Chouette::VehicleJourney do
     sequence(:objectid) { |n| "test:VehicleJourney:#{n}" }
 
-    factory :vehicle_journey do
+    factory :vehicle_journey_empty do
       association :journey_pattern, :factory => :journey_pattern
 
       after(:build) do |vehicle_journey|
         vehicle_journey.route = vehicle_journey.journey_pattern.route
       end
 
-      after(:create) do |vehicle_journey|
-        vehicle_journey.journey_pattern.stop_points.each_with_index do |stop_point, index|
-          vehicle_journey.vehicle_journey_at_stops << create(:vehicle_journey_at_stop,
-                 :vehicle_journey => vehicle_journey,
-                 :stop_point      => stop_point,
-                 :arrival_time    => '2000-01-01 01:00:00 UTC',
-                 :departure_time  => '2000-01-01 03:00:00 UTC')
+      factory :vehicle_journey do
+        after(:create) do |vehicle_journey|
+          vehicle_journey.journey_pattern.stop_points.each_with_index do |stop_point, index|
+            vehicle_journey.vehicle_journey_at_stops << create(:vehicle_journey_at_stop,
+                   :vehicle_journey => vehicle_journey,
+                   :stop_point      => stop_point,
+                   :arrival_time    => '2000-01-01 01:00:00 UTC',
+                   :departure_time  => '2000-01-01 03:00:00 UTC')
+          end
         end
-      end
 
-      factory :vehicle_journey_odd do
-        association :journey_pattern, :factory => :journey_pattern_odd
-      end
+        factory :vehicle_journey_odd do
+          association :journey_pattern, :factory => :journey_pattern_odd
+        end
 
-      factory :vehicle_journey_even do
-        association :journey_pattern, :factory => :journey_pattern_even
+        factory :vehicle_journey_even do
+          association :journey_pattern, :factory => :journey_pattern_even
+        end
       end
     end
   end
diff --git a/spec/models/chouette/vehicle_journey_spec.rb b/spec/models/chouette/vehicle_journey_spec.rb
index 3d3a948bc..b22183ab6 100644
--- a/spec/models/chouette/vehicle_journey_spec.rb
+++ b/spec/models/chouette/vehicle_journey_spec.rb
@@ -221,6 +221,95 @@ describe Chouette::VehicleJourney, :type => :model do
     end
   end
 
+  describe ".with_stops" do
+    def initialize_stop_times(vehicle_journey, &block)
+      vehicle_journey
+        .vehicle_journey_at_stops
+        .each_with_index do |at_stop, index|
+          at_stop.update(
+            departure_time: at_stop.departure_time + block.call(index),
+            arrival_time: at_stop.arrival_time + block.call(index)
+          )
+        end
+    end
+
+    it "selects vehicle journeys including stops in order or earliest departure time" do
+      # Create later vehicle journey to give it a later id, such that it should
+      # appear last if the order in the query isn't right.
+      journey_late = create(:vehicle_journey)
+      journey_early = create(
+        :vehicle_journey,
+        route: journey_late.route,
+        journey_pattern: journey_late.journey_pattern
+      )
+
+      initialize_stop_times(journey_early) do |index|
+        (index + 5).minutes
+      end
+      initialize_stop_times(journey_late) do |index|
+        (index + 65).minutes
+      end
+
+      expected_journey_order = [journey_early, journey_late]
+
+      expect(
+        journey_late
+          .route
+          .vehicle_journeys
+          .with_stops
+          .to_a
+      ).to eq(expected_journey_order)
+    end
+
+    it "orders journeys with nil times at the end" do
+      journey_nil = create(:vehicle_journey_empty)
+      journey = create(
+        :vehicle_journey,
+        route: journey_nil.route,
+        journey_pattern: journey_nil.journey_pattern
+      )
+
+      expected_journey_order = [journey, journey_nil]
+
+      expect(
+        journey
+          .route
+          .vehicle_journeys
+          .with_stops
+          .to_a
+      ).to eq(expected_journey_order)
+    end
+
+    it "journeys that skip the first stop(s) get ordered by the time of the \
+        first stop that they make" do
+      journey_missing_stop = create(:vehicle_journey)
+      journey_early = create(
+      :vehicle_journey,
+        route: journey_missing_stop.route,
+        journey_pattern: journey_missing_stop.journey_pattern
+      )
+
+      initialize_stop_times(journey_early) do |index|
+        (index + 5).minutes
+      end
+      initialize_stop_times(journey_missing_stop) do |index|
+        (index + 65).minutes
+      end
+
+      journey_missing_stop.vehicle_journey_at_stops.first.destroy
+
+      expected_journey_order = [journey_early, journey_missing_stop]
+
+      expect(
+        journey_missing_stop
+          .route
+          .vehicle_journeys
+          .with_stops
+          .to_a
+      ).to eq(expected_journey_order)
+    end
+  end
+
   subject { create(:vehicle_journey_odd) }
   describe "in_relation_to_a_journey_pattern methods" do
     let!(:route) { create(:route)}