Merge pull request #259 from fnf-org/kig/events-speedup

A slew of performance related changes.

.envrc

@@ -72,5 +72,7 @@ export RUBY_CFLAGS="$CFLAGS"
 
 [[ -f .envrc.local ]] && source .envrc.local
 
-
+if [[ $(uname -s) == "Darwin" ]]; then
+  export BULLET_ENABLED=true
+fi
 

.rubocop.yml

@@ -17,7 +17,7 @@ RSpecRails/InferredSpecType:
   Enabled: false
 
 Metrics/ClassLength:
-  Max: 300
+  Enabled: false
 
 Metrics/ModuleLength:
   Enabled: false

.rubocop_todo.yml

@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2024-05-20 21:02:26 UTC using RuboCop version 1.63.5.
+# on 2024-08-02 21:17:29 UTC using RuboCop version 1.65.0.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
@@ -16,11 +16,11 @@ Lint/UnmodifiedReduceAccumulator:
   Exclude:
     - 'app/helpers/shifts_helper.rb'
 
-# Offense count: 6
+# Offense count: 7
 # Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns, inherit_mode.
 # AllowedMethods: refine
 Metrics/BlockLength:
-  Max: 47
+  Max: 63
 
 # Offense count: 12
 # Configuration parameters: AllowedMethods, AllowedPatterns.
@@ -30,7 +30,7 @@ Metrics/CyclomaticComplexity:
 # Offense count: 50
 # Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns.
 Metrics/MethodLength:
-  Max: 60
+  Max: 46
 
 # Offense count: 10
 # Configuration parameters: AllowedMethods, AllowedPatterns.
@@ -44,7 +44,7 @@ Naming/MethodParameterName:
   Exclude:
     - 'spec/support/time_extensions.rb'
 
-# Offense count: 22
+# Offense count: 26
 # Configuration parameters: Prefixes, AllowedPatterns.
 # Prefixes: when, with, without
 RSpec/ContextWording:
@@ -59,10 +59,14 @@ RSpec/DescribeClass:
   Exclude:
     - 'spec/lib/fnf/music_submissions_spec.rb'
 
-# Offense count: 29
+# Offense count: 1
+RSpec/MultipleExpectations:
+  Max: 3
+
+# Offense count: 23
 # Configuration parameters: AllowSubject.
 RSpec/MultipleMemoizedHelpers:
-  Max: 11
+  Max: 7
 
 # Offense count: 2
 # Configuration parameters: EnforcedStyle, IgnoreSharedExamples.
@@ -72,7 +76,7 @@ RSpec/NamedSubject:
     - 'spec/lib/fnf/csv_reader_spec.rb'
     - 'spec/models/ticket_request_spec.rb'
 
-# Offense count: 86
+# Offense count: 84
 # Configuration parameters: AllowedGroups.
 RSpec/NestedGroups:
   Max: 6
@@ -91,7 +95,7 @@ RSpec/RepeatedExampleGroupBody:
   Exclude:
     - 'spec/models/event_spec.rb'
 
-# Offense count: 9
+# Offense count: 12
 # Configuration parameters: Database, Include.
 # SupportedDatabases: mysql, postgresql
 # Include: db/**/*.rb
@@ -124,7 +128,7 @@ Rails/HelperInstanceVariable:
   Exclude:
     - 'app/helpers/shifts_helper.rb'
 
-# Offense count: 23
+# Offense count: 24
 Rails/I18nLocaleTexts:
   Exclude:
     - 'app/controllers/events_controller.rb'
@@ -148,7 +152,7 @@ Rails/NotNullColumn:
     - 'db/migrate/20130226221916_add_user_to_ticket_request.rb'
     - 'db/migrate/20130311213508_add_event_id_to_ticket_request.rb'
 
-# Offense count: 9
+# Offense count: 22
 # Configuration parameters: Include.
 # Include: db/**/*.rb
 Rails/ReversibleMigration:
@@ -182,21 +186,13 @@ Rails/ThreeStateBooleanColumn:
     - 'db/migrate/20140616030905_change_camping_type_on_ticket_requests.rb'
     - 'db/migrate/20150609064608_add_agrees_terms_to_ticket_requests.rb'
 
-# Offense count: 2
-# Configuration parameters: Include.
-# Include: app/models/**/*.rb
-Rails/UniqueValidationWithoutIndex:
-  Exclude:
-    - 'app/models/payment.rb'
-    - 'app/models/site_admin.rb'
-
 # Offense count: 2
 # This cop supports safe autocorrection (--autocorrect).
 Rake/Desc:
   Exclude:
     - 'Rakefile'
 
-# Offense count: 87
+# Offense count: 94
 # Configuration parameters: AllowedConstants.
 Style/Documentation:
   Enabled: false
@@ -209,18 +205,16 @@ Style/FrozenStringLiteralComment:
   Exclude:
     - 'app/models/application_record.rb'
 
-# Offense count: 6
+# Offense count: 2
 # This cop supports safe autocorrection (--autocorrect).
 # Configuration parameters: MinBodyLength, AllowConsecutiveConditionals.
 Style/GuardClause:
   Exclude:
     - 'app/controllers/application_controller.rb'
-    - 'app/controllers/ticket_requests_controller.rb'
-    - 'app/models/event.rb'
 
-# Offense count: 27
+# Offense count: 34
 # This cop supports safe autocorrection (--autocorrect).
 # Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, AllowedPatterns.
 # URISchemes: http, https
 Layout/LineLength:
-  Max: 252
+  Max: 154

Gemfile

@@ -101,6 +101,7 @@ end
 
 group :development do
   gem 'asciidoctor'
+  gem 'bullet', require: false
   # gem 'rack-mini-profiler'
   gem 'better_errors'
   gem 'binding_of_caller'

Gemfile.lock

@@ -105,6 +105,9 @@ GEM
     brakeman (6.1.2)
       racc
     builder (3.3.0)
+    bullet (7.2.0)
+      activesupport (>= 3.0.0)
+      uniform_notifier (~> 1.11)
     capistrano (3.19.1)
       airbrussh (>= 1.0.0)
       i18n
@@ -436,6 +439,7 @@ GEM
       concurrent-ruby (~> 1.0)
     unaccent (0.4.0)
     unicode-display_width (2.5.0)
+    uniform_notifier (1.16.0)
     ventable (1.3.1)
       activesupport (>= 5)
     warden (1.2.9)
@@ -474,6 +478,7 @@ DEPENDENCIES
   binding_of_caller
   bootsnap
   brakeman
+  bullet
   capistrano
   capistrano-faster-assets
   capistrano-rails

Makefile

@@ -42,7 +42,7 @@ reset:		## Complete reset of the databases and runs the rspec and rubocop
 		@printf "\n$(bg_purple)  👉  $(purple)$(clear)  $(yellow)Creating local databases...$(clear)\n"
 		@bundle exec rake db:create:all
 		@printf "\n$(bg_purple)  👉  $(purple)$(clear)  $(yellow)Migrating development...$(clear)\n"
-		@bundle exec rake db:migrate
+		@bundle exec rake db:migrate:with_data
 		@printf "\n$(bg_purple)  👉  $(purple)$(clear)  $(yellow)Seeding development...$(clear)\n"
 		@bundle exec rake db:seed
 		@printf "\n$(bg_purple)  👉  $(purple)$(clear)  $(yellow)Cloning to test DB...$(clear)\n"
@@ -84,7 +84,7 @@ db-create:  	node_modules ## Create if necessary, migrate and seed the database
 		@printf "\n$(bg_purple)  👉  $(purple)$(clear)  $(yellow)Creating Database:    [$(DEV_DB)]$(clear)\n"
 		@bin/rails db:create
 		@printf "\n$(bg_purple)  👉  $(purple)$(clear)  $(yellow)Migrating Databases:  [$(DEV_DB)]$(clear)\n"
-		@bin/rails db:migrate
+		@bin/rails db:migrate:with_data
 		@printf "\n$(bg_purple)  👉  $(purple)$(clear)  $(yellow)Seeding dev database: [$(DEV_DB)]$(clear)\n"
 		@bin/rails db:seed
 
@@ -109,7 +109,7 @@ dev:          	gems node_modules db-create foreman ## Start the development envi
 
 
 ci: 		## Run all tests and linters as if on CI
-		bin/rails db:migrate
+		bin/rails db:migrate:with_data
 		bin/rails db:test:prepare
 		bundle exec rspec
 		bundle exec rubocop

Procfile.dev

@@ -1,5 +1,5 @@
 # vim: ft=yaml
-web: bin/rails server -p 8080
+web: BULLET_ENABLED=true bin/rails server -p 8080
 js: yarn watch:js
 css: yarn watch:css
 browser: sleep 4 && open http://tickets-local.fnf.org:8080/ && while true; do sleep 100; echo .; done

app/controllers/application_controller.rb

@@ -33,13 +33,22 @@ def stripe_publishable_api_key
 
   def set_event
     event_id = permitted_params[:event_id].to_i
-    Rails.logger.debug { "#set_event() => event_id = #{event_id}, params[:event_id] => #{permitted_params[:event_id]}" }
+    event_slug = permitted_params[:event_id].delete("#{event_id}-")
 
-    @event = Event.where(id: event_id).first
-    if @event.nil?
-      flash.now[:error] = "Event with id #{event_id} was not found."
+    Rails.logger.debug { "#set_event() => event_id = #{event_id}, event_slug = #{event_slug} params[:event_id] => #{permitted_params[:event_id]}" }
+
+    event_not_found = lambda do |eid, flash|
+      flash.now[:error] = "Event with id #{eid} was not found."
       raise ArgumentError, flash.now[:error]
     end
+
+    @event = Event.where(id: event_id).first
+
+    if @event.slug != event_slug
+      Rails.logger.warn("Event slug mismatch: [#{event_slug}] != [#{@event&.slug}]")
+    end
+
+    event_not_found[event_id, flash] if @event.nil?
   end
 
   def ticket_request_id

app/controllers/events_controller.rb

@@ -10,9 +10,9 @@ class EventsController < ApplicationController
 
   def index
     if current_user.site_admin?
-      @events = Event.order(start_time: :desc)
+      @events = Event.includes(:ticket_requests).order(start_time: :desc)
     elsif current_user.event_admin?
-      @events = current_user.events_administrated.order(:start_time)
+      @events = current_user.events_administrated.includes(:ticket_requests).order(:start_time)
     else
       redirect_to :root
     end

app/models/ticket_request.rb

@@ -101,11 +101,11 @@ def csv_columns
   include ActiveModel::Validations::Callbacks
 
   STATUSES = [
-    STATUS_PENDING          = 'P',
+    STATUS_PENDING = 'P',
     STATUS_AWAITING_PAYMENT = 'A',
-    STATUS_DECLINED         = 'D',
-    STATUS_COMPLETED        = 'C',
-    STATUS_REFUNDED         = 'R'
+    STATUS_DECLINED = 'D',
+    STATUS_COMPLETED = 'C',
+    STATUS_REFUNDED = 'R'
   ].freeze
 
   STATUS_NAMES = {
@@ -133,7 +133,7 @@ def csv_columns
   }.freeze
 
   belongs_to :user, inverse_of: :ticket_requests
-  belongs_to :event, inverse_of: :ticket_requests
+  belongs_to :event, inverse_of: :ticket_requests, touch: true
 
   has_one :payment, inverse_of: :ticket_request
 
@@ -268,11 +268,7 @@ def refund
 
   # calculate the total price for this ticket request
   def price
-    return special_price if special_price
-
-    total = tickets_price
-    total += calculate_addons_price
-    total
+    @price ||= special_price.presence || (tickets_price + calculate_addons_price)
   end
 
   def tickets_price
@@ -282,14 +278,17 @@ def tickets_price
   end
 
   def calculate_addons_price
-    return 0 unless ticket_request_event_addons?
-
-    tr_addons_price = 0
-    ticket_request_event_addons.each do |tr_event_addon|
-      tr_addons_price += tr_event_addon.calculate_cost
-    end
+    @calculate_addons_price ||=
+      if ticket_request_event_addons?
+        tr_addons_price = 0
+        ticket_request_event_addons.each do |tr_event_addon|
+          tr_addons_price += tr_event_addon.calculate_cost
+        end
 
-    tr_addons_price
+        tr_addons_price
+      else
+        0
+      end
   end
 
   def cost
@@ -354,27 +353,28 @@ def build_ticket_request_event_addons_from_params(build_params)
   end
 
   def active_addons
-    ticket_request_event_addons.where('quantity > ?', 0)
+    @active_addons ||= ticket_request_event_addons.where('quantity > ?', 0)
   end
 
   def active_addons_sum
-    active_addons.sum(&:quantity)
+    @active_addons_sum ||= active_addons.sum(&:quantity)
   end
 
   def active_sorted_addons
-    active_addons.sort_by { |e| [e.category, e.price, e.name] }
+    @active_sorted_addons ||= active_addons.sort_by { |e| [e.category, e.price, e.name] }
   end
 
   def active_addon_pass_sum
-    active_addon_sum_quantity_by_category(Addon::CATEGORY_PASS)
+    @active_addon_pass_sum ||= active_addon_sum_quantity_by_category(Addon::CATEGORY_PASS)
   end
 
   def active_addon_camp_sum
-    active_addon_sum_quantity_by_category(Addon::CATEGORY_CAMP)
+    @active_addon_camp_sum ||= active_addon_sum_quantity_by_category(Addon::CATEGORY_CAMP)
   end
 
   def active_addon_sum_quantity_by_category(category)
-    active_addons.select { |addon| addon.category == category }.sum(&:quantity)
+    @active_addon_sum_quantity_by_category ||= {}
+    @active_addon_sum_quantity_by_category[category] ||= active_addons.select { |addon| addon.category == category }.sum(&:quantity)
   end
 
   def ticket_request_event_addons?

app/views/events/_event.html.haml

@@ -0,0 +1,21 @@
+- status_widget = event.status
+
+%tr
+  %td.p-1.align-content-center
+    .span.me-1
+      = link_to event, class: "text-nowrap btn btn-#{status_widget.css_class} btn-sm btn-event-name" do
+        = event.name.truncate(35, separator: /\s/)
+  %td.p-1.optional.align-content-center.mx-4.text-nowrap
+    %span{class: "text-#{status_widget.css_class}"}= status_widget.name
+  %td.p-1.text-end.optional.align-content-center
+    = number_with_delimiter(event.ticket_requests.count)
+  %td.p-1.text-end.align-content-center
+    = number_with_delimiter(event.ticket_requests.sum(&:guest_count))
+  %td.p-1.text-end.optional.align-content-center
+    = number_to_currency(event.ticket_requests.completed.sum(&:cost))
+  %td.p-1.text-end.optional.align-content-center
+    = number_to_currency(event.ticket_requests.awaiting_payment.sum(&:cost))
+  %td.text-nowrap.small.align-content-center.event-dates.optional-small
+    = TimeHelper.for_display(event.start_time.localtime)
+    %br
+    = TimeHelper.for_display(event.end_time.localtime)