[bug] canceled reservation are not removed from statistics (fix #133)

CHANGELOG.md

@@ -11,6 +11,7 @@
 - Fix a bug: some users may not appear in the admin's general listing
 - Fix a bug: Availabilities export report an erroneous number of reservations for machine availabilities (#131)
 - Fix a bug: close period reminder is sent before the first invoice's first anniversary
+- Fix a bug: Canceled reservations are not removed from statistics (#133)
 - Improved translations syntax according to YML specifications
 - Refactored some Ruby code to match style guide
 - [TODO DEPLOY] `rake fablab:fix:users_group_ids`

app/controllers/api/slots_controller.rb

@@ -19,7 +19,7 @@ class API::SlotsController < API::ApiController
 
   def cancel
     authorize @slot
-    @slot.update_attributes(canceled_at: DateTime.now)
+    SlotService.new.cancel(@slot)
   end
 
   private

app/models/availability.rb

@@ -40,8 +40,8 @@ class Availability < ActiveRecord::Base
   validate :should_be_associated
 
   ## elastic callbacks
-  after_save { AvailabilityIndexerWorker.perform_async(:index, self.id) }
-  after_destroy { AvailabilityIndexerWorker.perform_async(:delete, self.id) }
+  after_save { AvailabilityIndexerWorker.perform_async(:index, id) }
+  after_destroy { AvailabilityIndexerWorker.perform_async(:delete, id) }
 
   # elastic mapping
   settings index: { number_of_replicas: 0 } do
@@ -113,7 +113,7 @@ class Availability < ActiveRecord::Base
     return false if nb_total_places.blank?
 
     if available_type == 'training' || available_type == 'space'
-      nb_total_places <= slots.to_a.select {|s| s.canceled_at == nil }.size
+      nb_total_places <= slots.to_a.select { |s| s.canceled_at.nil? }.size
     elsif available_type == 'event'
       event.nb_free_places.zero?
     end
@@ -132,6 +132,7 @@ class Availability < ActiveRecord::Base
     end
   end
 
+  # the resulting JSON will be indexed in ElasticSearch, as /fablab/availabilities
   def as_indexed_json
     json = JSON.parse(to_json)
     json['hours_duration'] = (end_at - start_at) / (60 * 60)

app/models/project.rb

@@ -75,6 +75,7 @@ class Project < ActiveRecord::Base
     end
   end
 
+  # the resulting JSON will be indexed in ElasticSearch, as /fablab/projects
   def as_indexed_json
     ApplicationController.new.view_context.render(
       partial: 'api/projects/indexed',

app/services/custom_aggregation_service.rb

@@ -1,20 +1,24 @@
+# frozen_string_literal: true
+
 require 'json'
 
+# Custom aggregations provides a way to aggregate data in a non-standard way to enhance statistics.
+# These aggregations will run in ElasticSearch so they must belongs to its syntax.
 class CustomAggregationService
 
   ##
   # Run any additional custom aggregations related to the given statistic type, if any
   ##
   def call(statistic_index, statistic_type, start_date, end_date, custom_query, results)
-    if statistic_type and start_date and end_date
+    if statistic_type && start_date && end_date
       stat_index = StatisticIndex.find_by(es_type_key: statistic_index)
       stat_type = StatisticType.find_by(statistic_index_id: stat_index.id, key: statistic_type)
       client = Elasticsearch::Model.client
       stat_type.statistic_custom_aggregations.each do |custom|
 
-        query = sprintf(custom.query, {aggs_name: custom.field, start_date: start_date, end_date: end_date})
+        query = sprintf(custom.query, aggs_name: custom.field, start_date: start_date, end_date: end_date)
 
-        if custom_query and !custom_query.empty?
+        if custom_query && !custom_query.empty?
           # Here, a custom query was provided with the original request (eg: filter by subtype)
           # so we try to apply this custom filter to the current custom aggregation.
           #
@@ -42,4 +46,4 @@ class CustomAggregationService
     end
     results
   end
-end
+end

app/services/slot_service.rb

@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+# helpers for managing slots (reservations sub-units)
+class SlotService
+  def cancel(slot)
+    # first we mark ths slot as cancelled in DB, to free a ticket
+    slot.update_attributes(canceled_at: DateTime.now)
+
+    # then we try to remove this reservation from ElasticSearch, to keep the statistics up-to-date
+    model_name = slot.reservation.reservable.class.name
+    client = Elasticsearch::Model.client
+
+    model = "Stats::#{model_name}".constantize
+    client.delete_by_query(
+      index: model.index_name,
+      type: model.document_type,
+      conflicts: 'proceed',
+      body: { query: { match: { reservationId: slot.reservation.id } } }
+    )
+  end
+end

app/services/statistic_service.rb

@@ -157,7 +157,8 @@ class StatisticService
   def reservations_machine_list(options = default_options)
     result = []
     Reservation
-      .where("reservable_type = 'Machine' AND reservations.created_at >= :start_date AND reservations.created_at <= :end_date", options)
+      .where("reservable_type = 'Machine' AND slots.canceled_at IS NULL AND " \
+             'reservations.created_at >= :start_date AND reservations.created_at <= :end_date', options)
       .eager_load(:slots, statistic_profile: [:group], invoice: [:invoice_items])
       .each do |r|
       next unless r.reservable
@@ -179,7 +180,8 @@ class StatisticService
   def reservations_space_list(options = default_options)
     result = []
     Reservation
-      .where("reservable_type = 'Space' AND reservations.created_at >= :start_date AND reservations.created_at <= :end_date", options)
+      .where("reservable_type = 'Space' AND slots.canceled_at IS NULL AND " \
+             'reservations.created_at >= :start_date AND reservations.created_at <= :end_date', options)
       .eager_load(:slots, statistic_profile: [:group], invoice: [:invoice_items])
       .each do |r|
       next unless r.reservable
@@ -201,7 +203,8 @@ class StatisticService
   def reservations_training_list(options = default_options)
     result = []
     Reservation
-      .where("reservable_type = 'Training' AND reservations.created_at >= :start_date AND reservations.created_at <= :end_date", options)
+      .where("reservable_type = 'Training' AND slots.canceled_at IS NULL AND " \
+             'reservations.created_at >= :start_date AND reservations.created_at <= :end_date', options)
       .eager_load(:slots, statistic_profile: [:group], invoice: [:invoice_items])
       .each do |r|
       next unless r.reservable
@@ -225,7 +228,8 @@ class StatisticService
   def reservations_event_list(options = default_options)
     result = []
     Reservation
-      .where("reservable_type = 'Event' AND reservations.created_at >= :start_date AND reservations.created_at <= :end_date", options)
+      .where("reservable_type = 'Event' AND slots.canceled_at IS NULL AND " \
+             'reservations.created_at >= :start_date AND reservations.created_at <= :end_date', options)
       .eager_load(:slots, statistic_profile: [:group], invoice: [:invoice_items])
       .each do |r|
       next unless r.reservable