diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 5c782634..a71cab2b 100644 --- a/app/controllers/application_controller.rb +++ b/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 diff --git a/app/controllers/ticket_requests_controller.rb b/app/controllers/ticket_requests_controller.rb index 96b963b9..4ceef9ed 100644 --- a/app/controllers/ticket_requests_controller.rb +++ b/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,7 +67,7 @@ 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 @@ -73,10 +75,12 @@ def new @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,7 +208,7 @@ 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 @@ -206,5 +216,5 @@ def permitted_params early_arrival_passes late_departure_passes guests]) end end + # rubocop: enable Metrics/ClassLength -1 diff --git a/app/models/ticket_request.rb b/app/models/ticket_request.rb index dfd5b235..a4c0ee3d 100644 --- a/app/models/ticket_request.rb +++ b/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 diff --git a/spec/controllers/ticket_requests_controller_spec.rb b/spec/controllers/ticket_requests_controller_spec.rb index 0d0767d4..aeffe76d 100644 --- a/spec/controllers/ticket_requests_controller_spec.rb +++ b/spec/controllers/ticket_requests_controller_spec.rb @@ -3,8 +3,8 @@ require 'rails_helper' describe TicketRequestsController, type: :controller do - let!(:user) { create(:user) } - let!(:event) { create(:event) } + let(:user) { create(:user) } + let(:event) { create(:event) } let(:viewer) { nil } @@ -41,7 +41,7 @@ describe 'GET #show' do subject do get :show, params: { event_id: ticket_request.event.to_param, - id: ticket_request.to_param } + id: ticket_request.to_param } end let(:ticket_request) { create(:ticket_request, user:) } @@ -94,7 +94,7 @@ describe 'GET #edit' do subject do get :edit, params: { event_id: ticket_request.event.to_param, - id: ticket_request.to_param } + id: ticket_request.to_param } end let(:ticket_request) { create(:ticket_request, event:) } @@ -130,74 +130,117 @@ end describe 'POST #create' do - let(:make_request) { ->(params = {}) { post :create, params: { event_id: event.id, ticket_request: ticket_request_params.merge(params) } } } + let(:event) { create(:event, tickets_require_approval: true) } let(:ticket_request_params) { build(:ticket_request, event:).as_json } - it 'has a ticket request hash' do - expect(ticket_request_params).to be_a(Hash) - end + let(:post_params) { { event_id: event.id, ticket_request: ticket_request_params } } - context 'when event ticket sales are closed' do - it 'has no error message before the request' do - Timecop.freeze(event.end_time + 1.hour) do - make_request.call - expect(flash[:error]).to be('Sorry, but ticket sales have closed') - end - end + let(:make_request) do + lambda { |params = {}| + post_params[:ticket_request].merge!(params) unless params.empty? + post :create, params: post_params + } end - context 'when viewer already signed in' do - subject(:response) { make_request[user_id: viewer.id] } + describe 'without Ventable callbacks', :ventable_disabled do - let(:viewer) { create(:user) } + describe 'ticket_request_params' do + subject { ticket_request_params } - it 'creates a ticket request' do - expect { response }.to(change(TicketRequest, :count)) + it { is_expected.to be_a(Hash) } + it { is_expected.to include('event_id' => event.id) } + it { is_expected.not_to include('ticket_request' => ['status']) } end - it 'assigned the ticket request to the viewer' do - expect { subject }.to change { viewer.reload.reload.ticket_requests }.by(1) + context 'when event ticket sales are closed' do + it 'has no error message before the request' do + Timecop.freeze(event.end_time + 1.hour) do + make_request.call + expect(flash[:error]).to be('Sorry, but ticket sales have closed') + end + end end - end - context 'when viewer is not signed in' do - let(:email) { Faker::Internet.email } - let(:password) { Faker::Internet.password } + context 'when viewer already signed in' do + subject { make_request.call } - let(:user_attributes) do - { - name: Faker::Name.name, - email:, - password: - } - end + let(:viewer) { user } + + before { TicketRequest.where(user_id: viewer.id).destroy_all } + + describe '#create HTTP status' do + it { succeeds } + + it 'has no errors' do + expect(flash[:error]).to be_nil + end + end - let(:valid_params) { { event_id: event.to_param, user_attributes: } } + describe 'database state' do + subject(:response) { make_request[] } - describe '#create' do - subject { make_request[user_attributes:] } + it 'creates a ticket request' do + expect { subject }.to(change(TicketRequest, :count)) + end - let(:users_request_count) { -> { User.find_by(email:).ticket_requests.count } } + it 'assigned the ticket request to the viewer' do + expect { subject }.to change { viewer.ticket_requests.count }.by(1) + end + end + end + + # rubocop: disable RSpec/MultipleMemoizedHelpers + context 'when viewer is not signed in' do + before { make_request[user_attributes:] } + + let(:email) { Faker::Internet.email } + let(:name) { Faker::Internet.name } + let(:password) { Faker::Internet.password } + + let(:user_attributes) do + { + name:, + email:, + password:, + password_confirmation: password + } + end + + let(:created_user) { User.find_by(email:) } + let(:users_request_count) { -> { created_user&.ticket_requests&.count } } - its(:current_user) { is_expected.not_to be_nil } + it { succeeds } - it 'creates a ticket request' do - expect { subject }.to have_changed(TicketRequest, :count).by(1) + it 'has no errors' do + expect(flash[:error]).to be_nil end - it 'assigns the ticket request to the created user' do - expect(users_request_count[]).to be(1) + describe 'ticket requests count' do + it 'creates a ticket request' do + expect(subject).to have_changed(TicketRequest, :count).by(1) + end + + it 'assigns the ticket request to the created user' do + expect(users_request_count[]).to be(1) + end end - it 'signs in the created user' do - expect(controller.current_user).to eql(User.find_by(email:)) + describe '#current_user' do + subject { controller&.current_user } + + it { is_expected.not_to be_nil } + + it { is_expected.to eql(created_user) } end - end - it 'creates a user with the specified email address' do - User.find_by(email:).should_not be_nil + describe 'created user' do + subject { created_user } + + it { is_expected.not_to be_nil } + end end end + # rubocop: enable RSpec/MultipleMemoizedHelpers end end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 02f41a67..5818ba64 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -25,6 +25,14 @@ def example_with_timeout(example) config.after { StripeMock.stop } + config.before :example, :ventable_disabled do + Ventable.disable + end + + config.after :example, :ventable_disabled do + Ventable.enable + end + config.around { |example| example_with_timeout(example) } config.use_transactional_fixtures = true