[bug#131] Availabilities export report an erroneous number of reservations for machine availabilities

CHANGELOG.md

@@ -1,10 +1,14 @@
 # Changelog Fab Manager
 
+- Configurable privacy policy and data protection officer
+- Alert users on privacy policy update
 - Fix a bug: (spanish) some translations are not loaded correctly
 - Fix a bug: some users may not appear in the admin's general listing
 - Fix a bug: updating a setting does not chain new values
+- Fix a bug: Availabilities export report an erroneous number of reservations for machine availabilities
 - Fix a security issue: updated to jquery 3.4.1 to fix [CVE-2019-11358](https://nvd.nist.gov/vuln/detail/CVE-2019-11358)
 - Improved translations syntax according to YML specifications
+- Refactored some Ruby code to match style guide
 - [TODO DEPLOY] `rake fablab:fix:users_group_ids`
 
 ## v3.1.1 2019 April 8

app/controllers/api/exports_controller.rb

@@ -37,7 +37,7 @@ class API::ExportsController < API::ApiController
     elsif params[:category] == 'availabilities'
       case params[:type]
       when 'index'
-        export = export.where('created_at > ?', Availability.maximum('updated_at'))
+        export = export.where('created_at > ?', [Availability.maximum('updated_at'), Reservation.maximum('updated_at')].max)
       else
         raise ArgumentError, "Unknown type availabilities/#{params[:type]}"
       end

app/helpers/availability_helper.rb

@@ -34,7 +34,7 @@ module AvailabilityHelper
   def space_slot_border_color(slot)
     if slot.is_reserved
       IS_RESERVED_BY_CURRENT_USER
-    elsif slot.is_complete?
+    elsif slot.complete?
       IS_COMPLETED
     else
       SPACE_COLOR

app/models/slot.rb

@@ -1,3 +1,7 @@
+# frozen_string_literal: true
+
+# Time range of duration defined by ApplicationHelper::SLOT_DURATION, slicing an Availability.
+# During a slot a Reservation is possible
 class Slot < ActiveRecord::Base
   include NotifyWith::NotificationAttachedObject
 
@@ -22,11 +26,12 @@ class Slot < ActiveRecord::Base
     super
   end
 
-  def is_complete?
+  def complete?
     reservations.length >= availability.nb_total_places
   end
 
   private
+
   def notify_member_and_admin_slot_is_modified
     NotificationCenter.call type: 'notify_member_slot_is_modified',
                             receiver: reservation.user,
@@ -47,7 +52,8 @@ class Slot < ActiveRecord::Base
 
   def can_be_modified?
     return false if (start_at - Time.now) / 1.day < 1
-    return true
+
+    true
   end
 
   def dates_were_modified?

app/services/availabilities/availabilities_service.rb

@@ -59,7 +59,7 @@ class Availabilities::AvailabilitiesService
       end
     end
     slots.each do |s|
-      s.title = I18n.t('availabilities.not_available') if s.is_complete? && !s.is_reserved
+      s.title = I18n.t('availabilities.not_available') if s.complete? && !s.is_reserved
     end
     slots
   end

app/services/availabilities_export_service.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'abstract_controller'
 require 'action_controller'
 require 'action_view'
@@ -6,6 +8,7 @@ require 'active_record'
 # require any helpers
 require './app/helpers/application_helper'
 
+# Retrieve all availabilities and their related objects and write the result as a table in an excel file
 class AvailabilitiesExportService
 
   # export all availabilities
@@ -14,7 +17,7 @@ class AvailabilitiesExportService
 
     ActionController::Base.prepend_view_path './app/views/'
     # place data in view_assigns
-    view_assigns = {availabilities: @availabilities}
+    view_assigns = { availabilities: @availabilities }
     av = ActionView::Base.new(ActionController::Base.view_paths, view_assigns)
     av.class_eval do
       # include any needed helpers (for the view)
@@ -23,7 +26,7 @@ class AvailabilitiesExportService
 
     content = av.render template: 'exports/availabilities_index.xlsx.axlsx'
     # write content to file
-    File.open(export.file,"w+b") {|f| f.puts content }
+    File.open(export.file, 'w+b') { |f| f.puts content }
   end
 
-end
+end

app/views/api/availabilities/public.json.jbuilder

@@ -61,7 +61,7 @@ json.array!(@availabilities) do |availability|
     elsif availability.try(:space)
       json.space_id availability.space.id
       json.borderColor space_slot_border_color(availability)
-      json.is_completed availability.is_complete?
+      json.is_completed availability.complete?
     else
       json.title 'Unknown slot'
     end

app/views/api/availabilities/spaces.json.jbuilder

@@ -5,7 +5,7 @@ json.array!(@slots) do |slot|
   json.start slot.start_at.iso8601
   json.end slot.end_at.iso8601
   json.is_reserved slot.is_reserved
-  json.is_completed slot.is_complete?
+  json.is_completed slot.complete?
   json.backgroundColor 'white'
   json.borderColor space_slot_border_color(slot)
 

app/views/exports/availabilities_index.xlsx.axlsx

@@ -1,7 +1,9 @@
+# frozen_string_literal: true
+
 wb = xlsx_package.workbook
 
-header = wb.styles.add_style :b => true, :bg_color => Stylesheet.primary.upcase.gsub('#', 'FF'), :fg_color => 'FFFFFFFF'
-date = wb.styles.add_style :format_code => Rails.application.secrets.excel_date_format
+header = wb.styles.add_style b: true, bg_color: Stylesheet.primary.upcase.gsub('#', 'FF'), fg_color: 'FFFFFFFF'
+date = wb.styles.add_style format_code: Rails.application.secrets.excel_date_format
 
 ## Machines slots
 wb.add_worksheet(name: t('export_availabilities.machines')) do |sheet|
@@ -10,7 +12,7 @@ wb.add_worksheet(name: t('export_availabilities.machines')) do |sheet|
   # heading labels
   columns = [t('export_availabilities.date'), t('export_availabilities.day_of_week'), t('export_availabilities.slot'),
              t('export_availabilities.machine'), t('export_availabilities.reservations')]
-  sheet.add_row columns, :style => header
+  sheet.add_row columns, style: header
 
   # data rows
   @availabilities.where(available_type: 'machines').order(:start_at).each do |a|
@@ -19,21 +21,21 @@ wb.add_worksheet(name: t('export_availabilities.machines')) do |sheet|
         start_at = a.start_at + (i * ApplicationHelper::SLOT_DURATION).minutes
         end_at = a.start_at + (i * ApplicationHelper::SLOT_DURATION).minutes + ApplicationHelper::SLOT_DURATION.minutes
         reservations = 0
-        if a.slots and a.slots.map(&:start_at).include? start_at
-          reservations = 1
+        if a.slots&.map(&:start_at)&.include? start_at
+          reservations = Reservation.where(reservable: m).includes(:slots).where('slots.id' => a.slots, 'slots.start_at' => start_at).count
         end
 
         data = [
-            start_at.to_date,
-            I18n.l(start_at, format: '%A').capitalize,
-            print_slot(start_at, end_at),
-            m.name,
-            reservations
+          start_at.to_date,
+          I18n.l(start_at, format: '%A').capitalize,
+          print_slot(start_at, end_at),
+          m.name,
+          reservations
         ]
         styles = [date, nil, nil, nil, nil]
-        types = [:date, :string, :string, :string, :integer]
+        types = %i[date string string string integer]
 
-        sheet.add_row data, :style =>  styles, :types => types
+        sheet.add_row data, style: styles, types: types
 
       end
     end
@@ -49,22 +51,22 @@ wb.add_worksheet(name: t('export_availabilities.trainings')) do |sheet|
   columns = [t('export_availabilities.date'), t('export_availabilities.day_of_week'), t('export_availabilities.slot'),
              t('export_availabilities.training'), t('export_availabilities.reservations'),
              t('export_availabilities.available_seats')]
-  sheet.add_row columns, :style => header
+  sheet.add_row columns, style: header
 
   # data rows
   @availabilities.where(available_type: 'training').order(:start_at).each do |a|
     data = [
-        a.start_at.to_date,
-        I18n.l(a.start_at, format: '%A').capitalize,
-        print_slot(a.start_at, a.end_at),
-        a.trainings.first.name,
-        a.reservations.count,
-        a.nb_total_places
+      a.start_at.to_date,
+      I18n.l(a.start_at, format: '%A').capitalize,
+      print_slot(a.start_at, a.end_at),
+      a.trainings.first.name,
+      a.reservations.count,
+      a.nb_total_places
     ]
     styles = [date, nil, nil, nil, nil, nil]
-    types = [:date, :string, :string, :string, :integer, :integer]
+    types = %i[date string string string integer integer]
 
-    sheet.add_row data, :style => styles, :types => types
+    sheet.add_row data, style: styles, types: types
   end
 end
 
@@ -77,7 +79,7 @@ if Rails.application.secrets.fablab_without_spaces != 'false'
     columns = [t('export_availabilities.date'), t('export_availabilities.day_of_week'), t('export_availabilities.slot'),
                t('export_availabilities.space'), t('export_availabilities.reservations'),
                t('export_availabilities.available_seats')]
-    sheet.add_row columns, :style => header
+    sheet.add_row columns, style: header
 
     # data rows
     @availabilities.where(available_type: 'space').order(:start_at).each do |a|
@@ -87,17 +89,17 @@ if Rails.application.secrets.fablab_without_spaces != 'false'
         reservations = a.slots.where(start_at: start_at).count
 
         data = [
-            start_at.to_date,
-            I18n.l(start_at, format: '%A').capitalize,
-            print_slot(start_at, end_at),
-            a.spaces.first.name,
-            reservations,
-            a.nb_total_places
+          start_at.to_date,
+          I18n.l(start_at, format: '%A').capitalize,
+          print_slot(start_at, end_at),
+          a.spaces.first.name,
+          reservations,
+          a.nb_total_places
         ]
         styles = [date, nil, nil, nil, nil, nil]
-        types = [:date, :string, :string, :string, :integer, :integer]
+        types = %i[date string string string integer integer]
 
-        sheet.add_row data, :style =>  styles, :types => types
+        sheet.add_row data, style: styles, types: types
       end
     end
   end
@@ -117,16 +119,16 @@ wb.add_worksheet(name: t('export_availabilities.events')) do |sheet|
   # data rows
   @availabilities.where(available_type: 'event').order(:start_at).each do |a|
     data = [
-        a.start_at.to_date,
-        I18n.l(a.start_at, format: '%A').capitalize,
-        print_slot(a.start_at, a.end_at),
-        a.event.title,
-        a.reservations.map(&:nb_reserve_places).reduce(:+) || 0,
-        a.nb_total_places
+      a.start_at.to_date,
+      I18n.l(a.start_at, format: '%A').capitalize,
+      print_slot(a.start_at, a.end_at),
+      a.event.title,
+      a.reservations.map(&:nb_reserve_places).reduce(:+) || 0,
+      a.nb_total_places
     ]
     styles = [date, nil, nil, nil, nil, nil]
-    types = [:date, :string, :string, :string, :integer, :integer]
+    types = %i[date string string string integer integer]
 
-    sheet.add_row data, :style => styles, :types => types
+    sheet.add_row data, style: styles, types: types
   end
 end

app/workers/availabilities_export_worker.rb

@@ -1,27 +1,25 @@
+# frozen_string_literal: true
+
+# Asynchronously export all the availabilities to an excel sheet
 class AvailabilitiesExportWorker
   include Sidekiq::Worker
 
   def perform(export_id)
     export = Export.find(export_id)
 
-    unless export.user.admin?
-      raise SecurityError, 'Not allowed to export'
-    end
+    raise SecurityError, 'Not allowed to export' unless export.user.admin?
 
-    unless export.category == 'availabilities'
-      raise KeyError, 'Wrong worker called'
-    end
+    raise KeyError, 'Wrong worker called' unless export.category == 'availabilities'
 
     service = AvailabilitiesExportService.new
     method_name = "export_#{export.export_type}"
 
-    if %w(index).include?(export.export_type) and service.respond_to?(method_name)
-      service.public_send(method_name, export)
+    return unless %w[index].include?(export.export_type) && service.respond_to?(method_name)
 
-      NotificationCenter.call type: :notify_admin_export_complete,
-                              receiver: export.user,
-                              attached_object: export
-    end
+    service.public_send(method_name, export)
 
+    NotificationCenter.call type: :notify_admin_export_complete,
+                            receiver: export.user,
+                            attached_object: export
   end
 end