From 9c803084beb0b00be1e0bd17a516507e0f054b98 Mon Sep 17 00:00:00 2001 From: Travis Grathwell Date: Sat, 5 Sep 2015 22:43:24 -0700 Subject: [PATCH] Consolidate EventsController's event json generation in the 'index' action The index action asks an EventList object to find the specific events and produce json from them if desired. Removes /events/past_events.json and /events/all_events.json Adds a 'type' to /events.json, which can be 'all', 'past', or 'upcoming'. Default is 'upcoming', which is much speedier than trying to jsonify 200+ events. --- app/controllers/events_controller.rb | 44 +++------------ app/models/event.rb | 4 -- app/models/external_event.rb | 4 ++ app/services/event_list.rb | 39 +++++++++++++ config/routes.rb | 2 - spec/controllers/events_controller_spec.rb | 65 ++++++++++------------ 6 files changed, 81 insertions(+), 77 deletions(-) create mode 100644 app/services/event_list.rb diff --git a/app/controllers/events_controller.rb b/app/controllers/events_controller.rb index 276fe9eaf..10e6c4793 100644 --- a/app/controllers/events_controller.rb +++ b/app/controllers/events_controller.rb @@ -1,17 +1,19 @@ class EventsController < ApplicationController - before_filter :authenticate_user!, except: [:index, :feed, :past_events, :all_events, :show, :levels] - before_filter :find_event, except: [:index, :feed, :past_events, :all_events, :create, :new] - before_filter :validate_organizer!, except: [:index, :feed, :past_events, :all_events, :create, :show, :new, :levels] + before_filter :authenticate_user!, except: [:index, :feed, :show, :levels] + before_filter :find_event, except: [:index, :feed, :create, :new] + before_filter :validate_organizer!, except: [:index, :feed, :create, :show, :new, :levels] before_filter :set_time_zone, only: [:create, :update] def index - @events = Event.upcoming.published_or_organized_by(current_user).includes(:event_sessions, :location, :chapter) - @event_chapters = @events.map { |e| e.chapter }.compact.uniq respond_to do |format| format.html do - @past_events = sort_by_starts_at(combined_past_events) + @events = Event.upcoming.published_or_organized_by(current_user).includes(:event_sessions, :location, :chapter) + @event_chapters = @events.map { |e| e.chapter }.compact.uniq + @past_events = EventList.new('past').combined_events + end + format.json do + render json: EventList.new(params[:type]) end - format.json { render json: @events } end end @@ -24,18 +26,6 @@ def feed end end - def past_events - respond_to do |format| - format.json { render json: sort_by_starts_at(combined_past_events_for_json) } - end - end - - def all_events - respond_to do |format| - format.json { render json: sort_by_starts_at(combined_all_events) } - end - end - def show if user_signed_in? && !@event.historical? @organizer = @event.organizer?(current_user) || current_user.admin? @@ -128,22 +118,6 @@ def find_event @event = Event.find(params[:id]) end - def combined_past_events_for_json - Event.for_json.past.published + ExternalEvent.past - end - - def combined_past_events - Event.includes(:location).past.published + ExternalEvent.past - end - - def combined_all_events - Event.for_json.published + ExternalEvent.all - end - - def sort_by_starts_at(events) - events.sort_by { |e| e.starts_at.to_time } - end - def allow_insecure? request.get? && request.format.json? end diff --git a/app/models/event.rb b/app/models/event.rb index df255c598..6ebd30e7b 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -185,10 +185,6 @@ def rsvps_with_checkins end end - def self.for_json - includes(:location, :event_sessions, :organizers, :legacy_organizers) - end - def self.published where(published: true) end diff --git a/app/models/external_event.rb b/app/models/external_event.rb index 21abc0d86..dac9b27f4 100644 --- a/app/models/external_event.rb +++ b/app/models/external_event.rb @@ -9,6 +9,10 @@ def self.past where('ends_at < ?', Time.now.utc) end + def self.upcoming + where('ends_at >= ?', Time.now.utc) + end + def title name end diff --git a/app/services/event_list.rb b/app/services/event_list.rb new file mode 100644 index 000000000..dab612a23 --- /dev/null +++ b/app/services/event_list.rb @@ -0,0 +1,39 @@ +class EventList + UPCOMING = 'upcoming'.freeze, + PAST = 'past'.freeze, + ALL = 'all'.freeze, + + def initialize(type = UPCOMING) + @type = type + end + + def as_json(options = {}) + combined_events.sort_by { |e| e.starts_at.to_time }.as_json + end + + def combined_events + bridgetroll_events.includes(:location, :event_sessions, :organizers, :legacy_organizers) + external_events + end + + private + + def bridgetroll_events + if @type == PAST + Event.past.published + elsif @type == ALL + Event.published + else + Event.upcoming.published + end + end + + def external_events + if @type == PAST + ExternalEvent.past + elsif @type == ALL + ExternalEvent.all + else + ExternalEvent.upcoming + end + end +end \ No newline at end of file diff --git a/config/routes.rb b/config/routes.rb index 013ab50e8..89f46d8d6 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -63,8 +63,6 @@ end collection do - get "past_events" - get "all_events" resources :unpublished_events, only: [:index], controller: "events/unpublished_events" do post "publish" post "flag" diff --git a/spec/controllers/events_controller_spec.rb b/spec/controllers/events_controller_spec.rb index c6d613a47..2c3cb05f9 100644 --- a/spec/controllers/events_controller_spec.rb +++ b/spec/controllers/events_controller_spec.rb @@ -46,6 +46,35 @@ end end + describe "GET index (json)" do + before do + @future_event = create(:event, title: 'FutureBridge', starts_at: 5.days.from_now, ends_at: 6.days.from_now, time_zone: 'Alaska') + @future_external_event = create(:external_event, name: 'FutureExternalBridge', starts_at: 3.days.from_now, ends_at: 4.days.from_now) + @past_event = create(:event, title: 'PastBridge', time_zone: 'Alaska') + @past_event.update_attributes(starts_at: 5.days.ago, ends_at: 4.days.ago) + @past_external_event = create(:external_event, name: 'PastExternalBridge', starts_at: 3.days.ago, ends_at: 2.days.ago) + @unpublished_event = create(:event, starts_at: 5.days.from_now, ends_at: 6.days.from_now, published: false) + end + + it 'can render published past events as json' do + get :index, format: 'json', type: 'past' + result_titles = JSON.parse(response.body).map{ |e| e['title'] } + result_titles.should == [@past_event, @past_external_event].map(&:title) + end + + it 'can render all published events as json' do + get :index, format: 'json', type: 'all' + result_titles = JSON.parse(response.body).map{ |e| e['title'] } + result_titles.should == [@past_event, @past_external_event, @future_external_event, @future_event].map(&:title) + end + + it 'can render only upcoming published events as json' do + get :index, format: 'json', type: 'upcoming' + result_titles = JSON.parse(response.body).map{ |e| e['title'] } + result_titles.should == [@future_external_event, @future_event].map(&:title) + end + end + describe "GET show" do let!(:event) { create(:event, title: 'DonutBridge') } @@ -429,42 +458,6 @@ def make_request end end - describe "GET past_events" do - before do - @future_event = create(:event, title: 'FutureBridge', starts_at: 5.days.from_now, ends_at: 6.days.from_now, time_zone: 'Alaska') - @future_external_event = create(:external_event, name: 'FutureExternalBridge', starts_at: 3.days.from_now, ends_at: 4.days.from_now) - @past_event = create(:event, title: 'PastBridge', time_zone: 'Alaska') - @past_event.update_attributes(starts_at: 5.days.ago, ends_at: 4.days.ago) - @past_external_event = create(:external_event, name: 'PastExternalBridge', starts_at: 3.days.ago, ends_at: 2.days.ago) - @unpublished_event = create(:event, starts_at: 5.days.from_now, ends_at: 6.days.from_now, published: false) - end - - it 'renders published past events as json' do - get :past_events, format: 'json' - response.should be_success - - result_titles = JSON.parse(response.body).map{ |e| e['title'] } - result_titles.should == [@past_event, @past_external_event].map(&:title) - end - end - - describe "GET all_events" do - before do - @future_event = create(:event, title: 'FutureBridge', starts_at: 5.days.from_now, ends_at: 6.days.from_now, time_zone: 'Alaska') - @future_external_event = create(:external_event, name: 'FutureExternalBridge', starts_at: 3.days.from_now, ends_at: 2.days.from_now) - @past_event = create(:event, title: 'PastBridge', time_zone: 'Alaska') - @past_event.update_attributes(starts_at: 5.days.ago, ends_at: 4.days.ago) - @past_external_event = create(:external_event, name: 'PastExternalBridge', starts_at: 3.days.ago, ends_at: 2.days.ago) - @unpublished_event = create(:event, starts_at: 5.days.from_now, ends_at: 6.days.from_now, published: false) - end - - it 'renders all published events as json' do - get :all_events, format: 'json' - result_titles = JSON.parse(response.body).map{ |e| e['title'] } - result_titles.should == [@past_event, @past_external_event, @future_external_event, @future_event].map(&:title) - end - end - describe "GET feed" do let!(:event) { create(:event, title: 'DonutBridge') } let!(:other_event) { create(:event, title: 'C5 Event!') }