From 7ce816e146ee59dd9b9c6e383cad1ec1eb775894 Mon Sep 17 00:00:00 2001 From: Siddhant Bajaj Date: Mon, 6 Mar 2017 21:45:48 +0530 Subject: [PATCH 1/3] Fixed DeleteEventSchedules issue Issues with DeleteEventSchedules method in conference controller: 1.It deletes EventSchedules of all the conferences that are not in the hours range. Instead it should delete EventSchedules of those events only that belong to that particular conference only. 2.If we set invalid start or end hour attribute of a conference then also EventSchedules gets deleted even though conference is not successfully updated. Fixed both the issues and added test for the same. --- .../admin/conferences_controller.rb | 10 ---------- app/models/conference.rb | 15 +++++++++++++++ spec/models/conference_spec.rb | 19 +++++++++++++++++++ 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/app/controllers/admin/conferences_controller.rb b/app/controllers/admin/conferences_controller.rb index 6aafe6c5..50360a64 100644 --- a/app/controllers/admin/conferences_controller.rb +++ b/app/controllers/admin/conferences_controller.rb @@ -82,7 +82,6 @@ module Admin short_title = @conference.short_title @conference.assign_attributes(conference_params) send_mail_on_conf_update = @conference.notify_on_dates_changed? - delete_event_schedules if @conference.start_hour_changed? || @conference.end_hour_changed? if @conference.update_attributes(conference_params) ConferenceDateUpdateMailJob.perform_later(@conference) if send_mail_on_conf_update @@ -189,14 +188,5 @@ module Admin :targets, :targets_attributes, :campaigns, :campaigns_attributes, :registration_limit) end - - def delete_event_schedules - event_schedules = EventSchedule.select do |e| - e.start_time.strftime('%H').to_i < @conference.start_hour || - e.end_time.strftime('%H').to_i > @conference.end_hour || - (e.end_time.strftime('%H').to_i == @conference.end_hour && e.end_time.strftime('%M').to_i > 0) - end - event_schedules.each(&:destroy) - end end end diff --git a/app/models/conference.rb b/app/models/conference.rb index f4d55d2e..224c1dfb 100644 --- a/app/models/conference.rb +++ b/app/models/conference.rb @@ -67,6 +67,7 @@ class Conference < ActiveRecord::Base before_create :create_email_settings after_create :create_free_ticket + after_update :delete_event_schedules ## # Checks if the user is registered to the conference @@ -80,6 +81,20 @@ class Conference < ActiveRecord::Base user.present? && registrations.where(user_id: user.id).count > 0 end + ## + # Delete all EventSchedules that are not in the hours range + # After the conference has been successfully updated + def delete_event_schedules + if start_hour_changed? || end_hour_changed? + event_schedules = program.event_schedules.select do |event_schedule| + event_schedule.start_time.hour < start_hour || + event_schedule.end_time.hour > end_hour || + (event_schedule.end_time.hour == end_hour && event_schedule.end_time.minute > 0) + end + event_schedules.each(&:destroy) + end + end + ## # Checks if the registration for the conference is currently open # diff --git a/spec/models/conference_spec.rb b/spec/models/conference_spec.rb index 47d64164..b5a6d5f0 100755 --- a/spec/models/conference_spec.rb +++ b/spec/models/conference_spec.rb @@ -1639,4 +1639,23 @@ describe Conference do expect(free_ticket.price_cents).to eq(0) end end + + describe 'after_update' do + let(:conference) { create(:conference) } + let(:scheduled_event_before_conference) { create(:event_scheduled, program: conference.program, hour: conference.start_date + conference.start_hour.hours) } + let(:scheduled_event_after_conference) { create(:event_scheduled, program: conference.program, hour: conference.start_date + conference.end_hour.hours - 1.hour) } + let!(:scheduled_event_during_conference) { create(:event_scheduled, program: conference.program, hour: conference.start_date + conference.start_hour.hours + 3.hours) } + + it 'delete event schedules that are not in hour ranges, when conference start hour is updated' do + scheduled_event_before_conference + conference.start_hour = conference.start_hour + 1 + expect{ conference.save }.to change{ EventSchedule.count }.from(2).to(1) + end + + it 'delete event schedules that are not in hour ranges, when conference end hour is updated' do + scheduled_event_after_conference + conference.end_hour = conference.end_hour - 2 + expect{ conference.save }.to change{ EventSchedule.count }.from(2).to(1) + end + end end From 7ff34c5b23655af18deaf919eb687ccf948f803d Mon Sep 17 00:00:00 2001 From: Siddhant Bajaj Date: Wed, 12 Apr 2017 23:06:17 +0530 Subject: [PATCH 2/3] Added relation between EventSchedule and program model Program has many event_schedules through schedules --- app/models/program.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/models/program.rb b/app/models/program.rb index ff0ab094..e8114f99 100644 --- a/app/models/program.rb +++ b/app/models/program.rb @@ -10,6 +10,7 @@ class Program < ActiveRecord::Base has_many :tracks, dependent: :destroy has_many :difficulty_levels, dependent: :destroy has_many :schedules, dependent: :destroy + has_many :event_schedules, through: :schedules belongs_to :selected_schedule, class_name: 'Schedule' has_many :events, dependent: :destroy do def require_registration From dc865abaaa50a4aa3142603f1dd810a8db6d3acc Mon Sep 17 00:00:00 2001 From: Siddhant Bajaj Date: Tue, 25 Apr 2017 16:36:43 +0530 Subject: [PATCH 3/3] Excluded conference_spec from block cop Fixes #1333 --- .rubocop.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.rubocop.yml b/.rubocop.yml index c2e973f0..9410af03 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -120,6 +120,10 @@ Metrics/ClassLength: Exclude: - 'app/models/conference.rb' +Metrics/BlockLength: + Exclude: + - 'spec/models/conference_spec.rb' + #################### Lint ############################### # Wrap your assignment in condition if you mean it, otherwise it is most likely equality check