From d4613cdbcc5a2d96afc5b332cf426746f9de99af Mon Sep 17 00:00:00 2001 From: Sylvain Date: Mon, 24 Oct 2022 15:56:04 +0200 Subject: [PATCH] (quality) refactored EventService to match rubocop rules --- CHANGELOG.md | 1 + .../javascript/controllers/admin/events.js | 2 +- app/services/event_service.rb | 184 +++++++++--------- 3 files changed, 98 insertions(+), 89 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3951096ee..0a427c6b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog Fab-manager - Allow searching by username (#401) +- Fix a bug: adding a new event without updating the dates results in internal server error (undefined method `div' for nil) - Fix a bug: portuguese time formatting (#405) - Fix a bug: admin users groups being overriden by SSO group_id (#404) - Fix a bug: no statistics on trainings and spaces reservations diff --git a/app/frontend/src/javascript/controllers/admin/events.js b/app/frontend/src/javascript/controllers/admin/events.js index ddacc3919..a26668edb 100644 --- a/app/frontend/src/javascript/controllers/admin/events.js +++ b/app/frontend/src/javascript/controllers/admin/events.js @@ -584,7 +584,7 @@ Application.Controllers.controller('NewEventController', ['$scope', '$state', 'C end_date: new Date(), start_time: new Date(), end_time: new Date(), - all_day: 'false', + all_day: true, recurrence: 'none', category_id: null, prices: [] diff --git a/app/services/event_service.rb b/app/services/event_service.rb index c27c6b47d..4e5eba4b1 100644 --- a/app/services/event_service.rb +++ b/app/services/event_service.rb @@ -31,14 +31,15 @@ class EventService def date_range(starting, ending, all_day) start_date = Time.zone.parse(starting[:date]) end_date = Time.zone.parse(ending[:date]) - start_time = Time.parse(starting[:time]) if starting[:time] - end_time = Time.parse(ending[:time]) if ending[:time] - if all_day + start_time = starting[:time] ? Time.zone.parse(starting[:time]) : nil + end_time = ending[:time] ? Time.zone.parse(ending[:time]) : nil + if all_day || start_time.nil? || end_time.nil? start_at = DateTime.new(start_date.year, start_date.month, start_date.day, 0, 0, 0, start_date.zone) end_at = DateTime.new(end_date.year, end_date.month, end_date.day, 23, 59, 59, end_date.zone) else - start_at = DateTime.new(start_date.year, start_date.month, start_date.day, start_time&.hour, start_time&.min, start_time&.sec, start_date.zone) - end_at = DateTime.new(end_date.year, end_date.month, end_date.day, end_time&.hour, end_time&.min, end_time&.sec, end_date.zone) + start_at = DateTime.new(start_date.year, start_date.month, start_date.day, start_time.hour, start_time.min, start_time.sec, + start_date.zone) + end_at = DateTime.new(end_date.year, end_date.month, end_date.day, end_time.hour, end_time.min, end_time.sec, end_date.zone) end { start_at: start_at, end_at: end_at } end @@ -59,16 +60,13 @@ class EventService ) .references(:availabilities, :events) when 'all' - Event.where( - 'recurrence_id = ?', - event.recurrence_id - ) + Event.where(recurrence_id: event.recurrence_id) else [] end events.each do |e| - # here we use double negation because safe_destroy can return either a boolean (false) or an Availability (in case of delete success) + # we use double negation because safe_destroy can return either a boolean (false) or an Availability (in case of delete success) results.push status: !!e.safe_destroy, event: e # rubocop:disable Style/DoubleNegation end results @@ -89,99 +87,41 @@ class EventService .references(:availabilities, :events) when 'all' Event.includes(:availability, :event_price_categories, :event_files) - .where( - 'recurrence_id = ?', - event.recurrence_id - ) + .where(recurrence_id: event.recurrence_id) else [] end - update_events(event, events, event_params) + update_occurrences(event, events, event_params) end private - def update_events(event, events, event_params) + def update_occurrences(base_event, occurrences, event_params) results = { events: [], slots: [] } - events.each do |e| - next unless e.id != event.id + original_slots_ids = base_event.availability.slots.map(&:id) - start_at = event_params['availability_attributes']['start_at'] - end_at = event_params['availability_attributes']['end_at'] - event_price_categories_attributes = event_params['event_price_categories_attributes'] - event_files_attributes = event_params['event_files_attributes'] - e_params = event_params.merge( - availability_id: e.availability_id, - availability_attributes: { - id: e.availability_id, - start_at: e.availability.start_at.change(hour: start_at.hour, min: start_at.min), - end_at: e.availability.end_at.change(hour: end_at.hour, min: end_at.min), - available_type: e.availability.available_type - } - ) - epc_attributes = [] - event_price_categories_attributes&.each do |epca| - epc = e.event_price_categories.find_by(price_category_id: epca['price_category_id']) - if epc - epc_attributes.push( - id: epc.id, - price_category_id: epc.price_category_id, - amount: epca['amount'], - _destroy: epca['_destroy'] - ) - elsif epca['id'].present? - event_price = event.event_price_categories.find(epca['id']) - epc_attributes.push( - price_category_id: epca['price_category_id'], - amount: event_price.amount, - _destroy: '' - ) - end - end - unless epc_attributes.empty? - e_params = e_params.merge( - event_price_categories_attributes: epc_attributes - ) - end + occurrences.each do |occurrence| + next unless occurrence.id != base_event.id - ef_attributes = [] - event_files_attributes&.each do |efa| - if efa['id'].present? - event_file = event.event_files.find(efa['id']) - ef = e.event_files.find_by(attachment: event_file.attachment.file.filename) - if ef - ef_attributes.push( - id: ef.id, - attachment: efa['attachment'], - _destroy: efa['_destroy'] - ) - end - else - ef_attributes.push(efa) - end - end - e_params = e_params.merge( - event_files_attributes: ef_attributes - ) - original_slots_ids = event.availability.slots.map(&:id) + e_params = occurrence_params(base_event, occurrence, event_params) begin - results[:events].push status: !!e.update(e_params.permit!), event: e # rubocop:disable Style/DoubleNegation - rescue StandardError => err - results[:events].push status: false, event: e, error: err.try(:record).try(:class).try(:name), message: err.message + results[:events].push status: !!occurrence.update(e_params.permit!), event: occurrence # rubocop:disable Style/DoubleNegation + rescue StandardError => e + results[:events].push status: false, event: occurrence, error: e.try(:record).try(:class).try(:name), message: e.message end - results[:slots].concat(update_slots(e.availability_id, original_slots_ids)) + results[:slots].concat(update_slots(occurrence.availability_id, original_slots_ids)) end - original_slots_ids = event.availability.slots.map(&:id) + begin - event_params[:availability_attributes][:id] = event.availability_id - results[:events].push status: !!event.update(event_params), event: event # rubocop:disable Style/DoubleNegation - rescue StandardError => err - results[:events].push status: false, event: event, error: err.try(:record).try(:class).try(:name), message: err.message + event_params[:availability_attributes][:id] = base_event.availability_id + results[:events].push status: !!base_event.update(event_params), event: base_event # rubocop:disable Style/DoubleNegation + rescue StandardError => e + results[:events].push status: false, event: base_event, error: e.try(:record).try(:class).try(:name), message: e.message end - results[:slots].concat(update_slots(event.availability_id, original_slots_ids)) + results[:slots].concat(update_slots(base_event.availability_id, original_slots_ids)) results end @@ -190,13 +130,81 @@ class EventService avail = Availability.find(availability_id) Slot.where(id: original_slots_ids).each do |slot| results.push( - status: !!slot.update_attributes(availability_id: availability_id, start_at: avail.start_at, end_at: avail.end_at), # rubocop:disable Style/DoubleNegation + status: !!slot.update(availability_id: availability_id, start_at: avail.start_at, end_at: avail.end_at), # rubocop:disable Style/DoubleNegation slot: slot ) - rescue StandardError => err - results.push status: false, slot: s, error: err.try(:record).try(:class).try(:name), message: err.message + rescue StandardError => e + results.push status: false, slot: s, error: e.try(:record).try(:class).try(:name), message: e.message end results end + + def occurrence_params(base_event, occurrence, event_params) + start_at = event_params['availability_attributes']['start_at'] + end_at = event_params['availability_attributes']['end_at'] + e_params = event_params.merge( + availability_id: occurrence.availability_id, + availability_attributes: { + id: occurrence.availability_id, + start_at: occurrence.availability.start_at.change(hour: start_at.hour, min: start_at.min), + end_at: occurrence.availability.end_at.change(hour: end_at.hour, min: end_at.min), + available_type: occurrence.availability.available_type + } + ) + epc_attributes = price_categories_attributes(base_event, occurrence, event_params) + unless epc_attributes.empty? + e_params = e_params.merge( + event_price_categories_attributes: epc_attributes + ) + end + + ef_attributes = file_attributes(base_event, occurrence, event_params) + e_params.merge( + event_files_attributes: ef_attributes + ) + end + + def price_categories_attributes(base_event, occurrence, event_params) + epc_attributes = [] + event_params['event_price_categories_attributes']&.each do |epca| + epc = occurrence.event_price_categories.find_by(price_category_id: epca['price_category_id']) + if epc + epc_attributes.push( + id: epc.id, + price_category_id: epc.price_category_id, + amount: epca['amount'], + _destroy: epca['_destroy'] + ) + elsif epca['id'].present? + event_price = base_event.event_price_categories.find(epca['id']) + epc_attributes.push( + price_category_id: epca['price_category_id'], + amount: event_price.amount, + _destroy: '' + ) + end + end + epc_attributes + end + + def file_attributes(base_event, occurrence, event_params) + ef_attributes = [] + event_params['event_files_attributes']&.each do |efa| + if efa['id'].present? + event_file = base_event.event_files.find(efa['id']) + ef = occurrence.event_files.find_by(attachment: event_file.attachment.file.filename) + if ef + ef_attributes.push( + id: ef.id, + attachment: efa['attachment'], + _destroy: efa['_destroy'] + ) + end + else + ef_attributes.push(efa) + end + end + ef_attributes + end end end