Just 5 specs left to fix

app/controllers/application_controller.rb

@@ -20,6 +20,12 @@ def require_event_admin
     redirect_to new_event_ticket_request_path(@event) unless @event.admin?(current_user)
   end
 
+  def require_logged_in_user
+    unless current_user&.id
+      redirect_to new_user_session_path
+    end
+  end
+
   def configure_permitted_parameters
     devise_parameter_sanitizer.permit(:sign_up, keys: [:name])
   end

app/controllers/ticket_requests_controller.rb

@@ -6,10 +6,12 @@
 # Manage all pages related to ticket requests.
 # rubocop: disable Metrics/ClassLength
 class TicketRequestsController < ApplicationController
-  before_action :set_event
   before_action :authenticate_user!, except: %i[new create]
+
+  before_action :set_event
+
   before_action :require_event_admin, except: %i[new create show edit update]
-  before_action :set_ticket_request, except: %i[index new create download]
+  before_action :set_ticket_request, except: %i[new create index download]
 
   def index
     @ticket_requests = TicketRequest
@@ -41,7 +43,7 @@ def index
   def download
     temp_csv = Tempfile.new('csv')
 
-    raise(ArgumentError, 'Blank temp_csv') unless temp_csv&.path
+    raise ArgumentError('Tempfile is nil') if temp_csv.nil? || temp_csv.path.nil?
 
     CSV.open(temp_csv.path, 'wb') do |csv|
       csv << (%w[name email] + TicketRequest.columns.map(&:name))
@@ -65,18 +67,20 @@ def show
 
   def new
     if signed_in?
-      existing_request = TicketRequest.where(user_id: current_user, event_id: @event).first
+      existing_request = TicketRequest.where(user_id: current_user, event_id: @event).order(:created_at).first
 
       return redirect_to event_ticket_request_path(@event, existing_request) if existing_request
     end
 
     @user = current_user if signed_in?
     @ticket_request = TicketRequest.new
 
-    last_ticket_request = TicketRequest.where(user_id: @user).order(:created_at).last
-    if last_ticket_request
-      %w[address_line1 address_line2 city state zip_code country_code].each do |field|
-        @ticket_request.send(:"#{field}=", last_ticket_request.send(field))
+    if @user
+      last_ticket_request = TicketRequest.where(user_id: @user&.id).order(:created_at).last
+      if last_ticket_request
+        %w[address_line1 address_line2 city state zip_code country_code].each do |field|
+          @ticket_request.send(:"#{field}=", last_ticket_request.send(field))
+        end
       end
     end
   end
@@ -105,20 +109,26 @@ def create
     end
 
     @ticket_request = TicketRequest.new(tr_params)
-    if @ticket_request.save
-      FnF::Events::TicketRequestEvent.new(
-        user: current_user,
-        target: @ticket_request
-      ).fire!
-
-      sign_in(@ticket_request.user) unless signed_in?
-
-      if @event.tickets_require_approval || @ticket_request.free?
-        redirect_to event_ticket_request_path(@event, @ticket_request)
+    @ticket_request.validate
+    if @ticket_request.valid?
+      if @ticket_request.save
+        FnF::Events::TicketRequestEvent.new(
+          user: current_user,
+          target: @ticket_request
+        ).fire!
+
+        sign_in(@ticket_request.user) unless signed_in?
+
+        if @event.tickets_require_approval || @ticket_request.free?
+          redirect_to event_ticket_request_path(@event, @ticket_request)
+        else
+          redirect_to new_payment_url(ticket_request_id: @ticket_request)
+        end
       else
-        redirect_to new_payment_url(ticket_request_id: @ticket_request)
+        render action: 'new'
       end
     else
+      flash[:error] = @ticket_request.errors.full_messages.join('. ')
       render action: 'new'
     end
   end
@@ -198,13 +208,13 @@ def set_ticket_request
   def permitted_params
     params.permit(:event_id, :id,
                   ticket_request: %i[user_id adults kids cabins needs_assistance
-                                     notes status special_price event_id
+                                     notes special_price event_id
                                      user_attributes user donation role role_explanation
                                      car_camping car_camping_explanation previous_contribution
                                      address_line1 address_line2 city state zip_code
                                      country_code admin_notes agrees_to_terms
                                      early_arrival_passes late_departure_passes guests])
   end
 end
+
 # rubocop: enable Metrics/ClassLength
-1

app/models/ticket_request.rb

@@ -1,5 +1,8 @@
 # frozen_string_literal: true
 
+#
+# rubocop: disable Metrics/ClassLength
+
 # == Schema Information
 #
 # Table name: ticket_requests
@@ -35,6 +38,9 @@
 #  user_id                 :integer          not null
 #
 class TicketRequest < ApplicationRecord
+  include ActiveModel::Validations
+  include ActiveModel::Validations::Callbacks
+
   STATUSES = [
     STATUS_PENDING = 'P',
     STATUS_AWAITING_PAYMENT = 'A',
@@ -59,9 +65,13 @@ class TicketRequest < ApplicationRecord
     ROLE_OTHER => 'Other'
   }.freeze
 
-  belongs_to :user
-  belongs_to :event
-  has_one :payment
+  belongs_to :user, inverse_of: :ticket_requests
+  belongs_to :event, inverse_of: :ticket_requests
+
+  has_one :payment, inverse_of: :ticket_request
+
+  # Serialize guest emails as an array in a text field.
+  # TODO: This should probably be switched to a separate table or at least a JSONB format column.
   serialize :guests, Array
 
   attr_accessible :user_id, :adults, :kids, :cabins, :needs_assistance,
@@ -77,7 +87,11 @@ class TicketRequest < ApplicationRecord
 
   accepts_nested_attributes_for :user
 
-  validates :user, presence: { unless: -> { user.try(:new_record?) } }
+  before_validation :set_defaults
+
+  validates :user, presence: { unless: -> { user&.id&.nil? } }
+
+  validates :event, presence: { unless: -> { event.try(:new_record?) } }
 
   validates :status, presence: true, inclusion: { in: STATUSES }
 
@@ -121,19 +135,6 @@ class TicketRequest < ApplicationRecord
   scope :declined, -> { where(status: STATUS_DECLINED) }
   scope :not_declined, -> { where.not(status: STATUS_DECLINED) }
 
-  before_validation do
-    if event
-      self.status ||= if event.tickets_require_approval
-                        STATUS_PENDING
-                      else
-                        STATUS_AWAITING_PAYMENT
-                      end
-    end
-
-    # Remove empty guests
-    self.guests = Array(guests).map { |guest| guest.strip.presence }.compact
-  end
-
   def can_view?(user)
     self.user == user || event.admin?(user)
   end
@@ -241,4 +242,25 @@ def all_guests_specified?
   def country_name
     ISO3166::Country[country_code]
   end
+
+  private
+
+  def set_defaults
+    self.status = nil if status.blank?
+    self.status ||= if event
+                      if event.tickets_require_approval
+                        STATUS_PENDING
+                      else
+                        STATUS_AWAITING_PAYMENT
+                      end
+                    else
+                      STATUS_PENDING
+                    end
+
+    # Remove empty guests
+    # Note that guests are serialized as an array field.
+    self.guests = Array(guests).map { |guest| guest&.strip }.select(&:present?).compact
+  end
 end
+
+# rubocop: enable Metrics/ClassLength