From bfe944e19872b616667845d4ed08866483dff79a Mon Sep 17 00:00:00 2001 From: Sydney Young Date: Tue, 9 Aug 2016 15:42:33 -0400 Subject: [PATCH 1/2] Add option to disable requests Resolves #1624 - Adds a Service Object for reservation creation --- app/controllers/app_configs_controller.rb | 3 +- app/controllers/reservations_controller.rb | 135 +++++---- app/views/app_configs/_form.erb | 1 + app/views/cart_js/reservation_form.js.erb | 7 +- app/views/reservations/_new_request.html.erb | 8 +- .../reservations/_new_reservation.html.erb | 14 +- .../reservations/_requests_disabled.html.erb | 49 +++ app/views/reservations/new.html.erb | 8 +- app/views/reservations/new_request.html.erb | 3 + .../reservations/requests_disabled.html.erb | 3 + ...337_add_disable_requests_to_app_configs.rb | 5 + db/schema.rb | 3 +- lib/reservation_creator.rb | 52 ++++ spec/controllers/contact_controller_spec.rb | 2 +- .../reservations_controller_spec.rb | 282 ++++++------------ spec/factories/app_configs.rb | 1 + spec/lib/reservation_creator_spec.rb | 55 ++++ spec/mailers/user_mailer_spec.rb | 6 +- spec/support/app_config_helpers.rb | 2 +- 19 files changed, 361 insertions(+), 278 deletions(-) create mode 100644 app/views/reservations/_requests_disabled.html.erb create mode 100644 app/views/reservations/new_request.html.erb create mode 100644 app/views/reservations/requests_disabled.html.erb create mode 100644 db/migrate/20160809194337_add_disable_requests_to_app_configs.rb create mode 100644 lib/reservation_creator.rb create mode 100644 spec/lib/reservation_creator_spec.rb diff --git a/app/controllers/app_configs_controller.rb b/app/controllers/app_configs_controller.rb index eac7fe25a..f98877e92 100644 --- a/app/controllers/app_configs_controller.rb +++ b/app/controllers/app_configs_controller.rb @@ -70,6 +70,7 @@ def app_config_params :checkout_persons_can_edit, :enable_renewals, :override_on_create, :override_at_checkout, :require_phone, :notify_admin_on_create, :disable_user_emails, - :autodeactivate_on_archive, :requests_affect_availability) + :autodeactivate_on_archive, :requests_affect_availability, + :disable_requests) end end diff --git a/app/controllers/reservations_controller.rb b/app/controllers/reservations_controller.rb index 1e3337131..87340adaa 100644 --- a/app/controllers/reservations_controller.rb +++ b/app/controllers/reservations_controller.rb @@ -102,80 +102,43 @@ def view_all_dates def show end - def new # rubocop:disable MethodLength + def new if cart.items.empty? flash[:error] = 'You need to add items to your cart before making a '\ 'reservation.' redirect_loc = (request.env['HTTP_REFERER'].present? ? :back : root_path) redirect_to redirect_loc - else - # error handling - @errors = cart.validate_all - unless @errors.empty? - if can? :override, :reservation_errors - flash[:error] = 'Are you sure you want to continue? Please review '\ - 'the errors below.' - else - flash[:error] = 'Please review the errors below. If uncorrected, '\ - 'any reservations with errors will be filed as a request, and '\ - 'subject to administrator approval.' - end - end + return + end + # error handling + @errors = cart.validate_all + if @errors.empty? # this is used to initialize each reservation later @reservation = Reservation.new(start_date: cart.start_date, due_date: cart.due_date, reserver_id: cart.reserver_id) + else + handle_new_reservation_errors end end - def create # rubocop:disable all + def create + # store information about the cart because it is purged if reservations + # are successfully created @errors = cart.validate_all - notes = params[:reservation][:notes] - requested = !@errors.empty? && (cannot? :override, :reservation_errors) - - # check for missing notes and validation errors - if !@errors.blank? && notes.blank? - # there were errors but they didn't fill out the notes - flash[:error] = 'Please give a short justification for this '\ - "reservation #{requested ? 'request' : 'override'}" - @notes_required = true - if AppConfig.get(:request_text).empty? - @request_text = 'Please give a short justification for this '\ - 'equipment request.' - else - @request_text = AppConfig.get(:request_text) - end - render(:new) && return - end - - Reservation.transaction do - begin - start_date = cart.start_date - reserver = cart.reserver_id - notes = format_errors(@errors) + notes.to_s - if requested - flash[:notice] = cart.request_all(current_user, - params[:reservation][:notes]) - else - flash[:notice] = cart.reserve_all(current_user, - params[:reservation][:notes]) - end - - if (cannot? :manage, Reservation) || (requested == true) - redirect_to(catalog_path) && return - end - if start_date == Time.zone.today - flash[:notice] += ' Are you simultaneously checking out equipment '\ - 'for someone? Note that only the reservation has been made. '\ - 'Don\'t forget to continue to checkout.' - end - redirect_to(manage_reservations_for_user_path(reserver)) && return - rescue ActiveRecord::RecordNotSaved, ActiveRecord::RecordInvalid => e - redirect_to catalog_path, flash: { error: 'Oops, something went '\ - "wrong with making your reservation.
#{e.message}".html_safe } - raise ActiveRecord::Rollback - end + start_date = cart.start_date + reserver_id = cart.reserver_id + creator = + ReservationCreator.new(cart: cart, current_user: current_user, + override: can?(:override, :reservation_errors), + notes: params[:reservation][:notes]) + result = creator.create! + if result[:error] + handle_create_errors(result[:error]) + else + handle_create_success(result[:result], start_date, reserver_id, + creator.request?) end end @@ -511,6 +474,58 @@ def archive # rubocop:disable all private + def handle_new_reservation_errors + if can? :override, :reservation_errors + flash[:error] = 'Are you sure you want to continue? Please review '\ + 'the errors below.' + render :new + elsif AppConfig.check(:disable_requests) + flash[:error] = 'Please review the errors below.' + render :requests_disabled + else + flash[:error] = 'Please review the errors below. If uncorrected, '\ + 'any reservations with errors will be filed as a request, and '\ + 'subject to administrator approval.' + render :new_request + end + end + + def handle_create_errors(errors) + case errors + when 'needs notes' + flash[:error] = 'Please give a short justification for this reservation' + @notes_required = true + @request_text = if AppConfig.get(:request_text).empty? + 'Please give a short justification for this '\ + 'equipment request.' + else + AppConfig.get(:request_text) + end + render :new_request + when 'requests disabled' + flash[:error] = 'Unable to create reservation' + render :requests_disabled + else + redirect_to catalog_path, + flash: { error: 'Oops, something went wrong with making '\ + "your reservation.
#{errors}".html_safe } + end + end + + def handle_create_success(messages, start_date, reserver_id, requested) + flash[:notice] = messages + if can?(:manage, Reservation) && !requested + if start_date == Time.zone.today + flash[:notice] += ' Are you simultaneously checking out equipment '\ + 'for someone? Note that only the reservation has been made. '\ + 'Don\'t forget to continue to checkout.' + end + redirect_to(manage_reservations_for_user_path(reserver_id)) + else + redirect_to(catalog_path) + end + end + def reservation_params params.require(:reservation) .permit(:checkout_handler_id, :checkin_handler_id, diff --git a/app/views/app_configs/_form.erb b/app/views/app_configs/_form.erb index 368e036a0..0e46fffd9 100644 --- a/app/views/app_configs/_form.erb +++ b/app/views/app_configs/_form.erb @@ -49,6 +49,7 @@
Reservation Settings
<%= f.input :requests_affect_availability, label: 'Allow requests to affect equipment availability?', hint: 'When enabled, reservation requests will affect equipment item availability before the request is approved.' %> + <%= f.input :disable_requests, label: 'Disable reservation requests?', hint: 'When enabled, users will not be able to create special reservation requests.' %> <%= f.input :notify_admin_on_create, label: 'Notify admin on creation?',hint: 'When enabled, admins will get an e-mail whenever a reservation is created.' %> <%= f.input :request_text, label: 'Reservation request text', input_html: {rows: 10}, hint: 'This message will be displayed to users who are about to file a reservation request for invalid reservations. You can use markdown in this field' %> diff --git a/app/views/cart_js/reservation_form.js.erb b/app/views/cart_js/reservation_form.js.erb index 28a099607..460097f94 100644 --- a/app/views/cart_js/reservation_form.js.erb +++ b/app/views/cart_js/reservation_form.js.erb @@ -1,10 +1,13 @@ resume_cart(); <% if @errors.empty? or (can? :override, :reservation_errors) %> - $('#confirm-res-form').html('<%= escape_javascript(render :partial => '/reservations/new_reservation') %>'); + $('#confirm-res-form').html('<%= escape_javascript(render partial: '/reservations/new_reservation') %>'); $('.page-header').html('

Confirm Reservation

'); +<% elsif AppConfig.check(:disable_requests) %> + $('#confirm-res-form').html('<%= escape_javascript(render partial: '/reservations/requests_disabled') %>'); + $('.page-header').html('

Reservation Denied

'); <% else %> - $('#confirm-res-form').html('<%= escape_javascript(render :partial => '/reservations/new_request' ) %>'); + $('#confirm-res-form').html('<%= escape_javascript(render partial: '/reservations/new_request' ) %>'); $('.page-header').html('

Confirm Reservation Request

'); <% end %> diff --git a/app/views/reservations/_new_request.html.erb b/app/views/reservations/_new_request.html.erb index 77df698b4..4f985ad8d 100644 --- a/app/views/reservations/_new_request.html.erb +++ b/app/views/reservations/_new_request.html.erb @@ -2,7 +2,7 @@
<% unless @errors.empty? %>

-

Would you like to resolve the following error(s)?

+

Would you like to resolve the following error(s)?

<% @errors.each do |msg| %> @@ -13,13 +13,13 @@
<% end %>

-

Equipment requested for +

Equipment requested for <%= link_to User.find(cart.reserver_id).name, User.find(cart.reserver_id), - target: '_blank' %> + target: '_blank' %> from <%= cart.start_date.to_s(:long) %> to <%= cart.due_date.to_s(:long) %>: -

+

<%= render partial: 'reservations/edit_reservation_form' %>
diff --git a/app/views/reservations/_new_reservation.html.erb b/app/views/reservations/_new_reservation.html.erb index 121bf907d..cee212275 100644 --- a/app/views/reservations/_new_reservation.html.erb +++ b/app/views/reservations/_new_reservation.html.erb @@ -3,10 +3,10 @@
<% unless @errors.empty? %>

-

- - Please be aware of the following errors: -

+

+ + Please be aware of the following errors: +

<% @errors.each do |msg| %> @@ -17,13 +17,13 @@
<% end %>

-

Equipment Reserved for +

Equipment Reserved for <%= link_to User.find(cart.reserver_id).name, User.find(cart.reserver_id), - target: '_blank' %> + target: '_blank' %> from <%= cart.start_date.to_s(:long) %> to <%= cart.due_date.to_s(:long) %>: -

+

<%= render partial: 'reservations/edit_reservation_form' %>
diff --git a/app/views/reservations/_requests_disabled.html.erb b/app/views/reservations/_requests_disabled.html.erb new file mode 100644 index 000000000..ef6262cb0 --- /dev/null +++ b/app/views/reservations/_requests_disabled.html.erb @@ -0,0 +1,49 @@ +<% title "Reservation Denied" %> +
+ <% unless @errors.empty? %> +

+

Please resolve the following error(s):

+

+
+ <% @errors.each do |msg| %> +
    +
  • <%= msg %>
    • +
+ <% end %> +
+ <% end %> +

+

Equipment requested for + <%= link_to User.find(cart.reserver_id).name, User.find(cart.reserver_id), + target: '_blank' %> + from + <%= cart.start_date.to_s(:long) %> to + <%= cart.due_date.to_s(:long) %>: +

+

+ <%= render partial: 'reservations/edit_reservation_form' %> + <%= simple_form_for @reservation do |f| %> +
+ <%= f.button :submit, "Submit", id: 'finalize_reservation_btn' %> +
+ <% end %> +
+ diff --git a/app/views/reservations/new.html.erb b/app/views/reservations/new.html.erb index 9e13ccaf5..14cf68bfe 100644 --- a/app/views/reservations/new.html.erb +++ b/app/views/reservations/new.html.erb @@ -1,7 +1,3 @@
- <% if @errors.empty? or (can? :override, :reservation_errors) %> - <%= render :partial => "new_reservation" %> - <% else %> - <%= render :partial => "new_request" %> - <% end %> -
\ No newline at end of file + <%= render partial: 'new_reservation' %> +
diff --git a/app/views/reservations/new_request.html.erb b/app/views/reservations/new_request.html.erb new file mode 100644 index 000000000..b8414f0c9 --- /dev/null +++ b/app/views/reservations/new_request.html.erb @@ -0,0 +1,3 @@ +
+ <%= render partial: 'new_request' %> +
diff --git a/app/views/reservations/requests_disabled.html.erb b/app/views/reservations/requests_disabled.html.erb new file mode 100644 index 000000000..6f21be797 --- /dev/null +++ b/app/views/reservations/requests_disabled.html.erb @@ -0,0 +1,3 @@ +
+ <%= render partial: 'requests_disabled' %> +
diff --git a/db/migrate/20160809194337_add_disable_requests_to_app_configs.rb b/db/migrate/20160809194337_add_disable_requests_to_app_configs.rb new file mode 100644 index 000000000..e82dd5569 --- /dev/null +++ b/db/migrate/20160809194337_add_disable_requests_to_app_configs.rb @@ -0,0 +1,5 @@ +class AddDisableRequestsToAppConfigs < ActiveRecord::Migration + def change + add_column :app_configs, :disable_requests, :boolean, default: false + end +end diff --git a/db/schema.rb b/db/schema.rb index babcf8bcf..1536c8223 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160610135455) do +ActiveRecord::Schema.define(version: 20160809194337) do create_table "announcements", force: :cascade do |t| t.text "message", limit: 65535 @@ -58,6 +58,7 @@ t.boolean "disable_user_emails", default: false t.boolean "autodeactivate_on_archive", default: false t.boolean "requests_affect_availability", default: false + t.boolean "disable_requests", default: false end create_table "blackouts", force: :cascade do |t| diff --git a/lib/reservation_creator.rb b/lib/reservation_creator.rb new file mode 100644 index 000000000..10c91f929 --- /dev/null +++ b/lib/reservation_creator.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true +class ReservationCreator + # Service Object to create reservations in the reservations controller + def initialize(cart:, current_user:, override: false, notes: '') + @current_user = current_user + @cart = cart + @cart_errors = cart.validate_all + @override = override + @notes = notes + end + + def create! + return { result: nil, error: error } if error + reservation_transaction + end + + def request? + !override && !cart_errors.blank? + end + + private + + attr_reader :cart, :current_user, :override, :notes, :cart_errors + + def error + return 'requests disabled' if request? && AppConfig.check(:disable_requests) + return 'needs notes' if needs_notes? + end + + def needs_notes? + (request? || override?) && notes.blank? + end + + def override? + override && !cart_errors.blank? + end + + def reservation_transaction + result = {} + Reservation.transaction do + begin + create_method = request? ? :request_all : :reserve_all + cart_result = cart.send(create_method, current_user, notes) + result = { result: cart_result, error: nil } + rescue ActiveRecord::RecordNotSaved, ActiveRecord::RecordInvalid => e + result = { result: nil, error: e.message } + raise ActiveRecord::Rollback + end + end + result + end +end diff --git a/spec/controllers/contact_controller_spec.rb b/spec/controllers/contact_controller_spec.rb index 999e44721..d5b3e7360 100644 --- a/spec/controllers/contact_controller_spec.rb +++ b/spec/controllers/contact_controller_spec.rb @@ -7,7 +7,7 @@ sign_in FactoryGirl.create(:user) # goes after the above to skip certain user validations - @ac = mock_app_config(contact_email: 'contact@email.com', + @ac = mock_app_config(contact_link_location: 'contact@email.com', admin_email: 'admin@email.com', site_title: 'Reservations Specs') end diff --git a/spec/controllers/reservations_controller_spec.rb b/spec/controllers/reservations_controller_spec.rb index 320038827..fc9236498 100644 --- a/spec/controllers/reservations_controller_spec.rb +++ b/spec/controllers/reservations_controller_spec.rb @@ -8,7 +8,6 @@ override_at_checkout: false, res_exp_time: false, admin_email: 'admin@email.com' }.freeze - before(:each) { mock_app_config(AC_DEFAULTS) } shared_examples 'inaccessible by banned user' do @@ -198,209 +197,104 @@ end describe '#create (POST /reservations/create)' do - # SMELLS: so, so many of them - # not going to refactor this yet - before(:all) do - @user = FactoryGirl.create(:user) - @checkout_person = FactoryGirl.create(:checkout_person) - end - after(:all) do - User.destroy_all - end - it_behaves_like 'inaccessible by banned user' do - before do - post :create, - reservation: FactoryGirl.attributes_for(:valid_reservation) - end - end - - context 'when accessed by non-banned user' do - before(:each) { sign_in @user } - - context 'with validation-failing items in Cart' do - before(:each) do - @invalid_cart = - FactoryGirl.build(:invalid_cart, reserver_id: @user.id) - @req = proc do - post :create, - { reservation: { notes: 'because I can' } }, - cart: @invalid_cart - end - @req_no_notes = proc do - post :create, - { reservation: { notes: '' } }, - cart: @invalid_cart - end - end - - context 'no justification provided' do - before do - sign_in @checkout_person - @req_no_notes.call - end - - it { is_expected.to render_template(:new) } - - it 'should set @notes_required to true' do - expect(assigns(:notes_required)).to be_truthy - end - end - - context 'and user can override errors' do - before(:each) do - mock_app_config(AC_DEFAULTS.merge(override_on_create: true)) - sign_in @checkout_person - end - - it 'affects the database' do - expect { @req.call }.to change { Reservation.count } - end - - it 'sets the reservation notes' do - @req.call - expect(Reservation.last.notes.empty?).not_to be_truthy - end - - it 'should redirect' do - @req.call - expect(response).to\ - redirect_to(manage_reservations_for_user_path(@user.id)) - end - - it 'sets the flash' do - @req.call - expect(flash[:notice]).not_to be_nil - end + let!(:cart) { instance_spy('Cart') } + before do + allow_any_instance_of(described_class).to \ + receive(:cart).and_return(cart) + allow_any_instance_of(described_class).to receive(:fix_cart_date) + allow_any_instance_of(ApplicationController).to \ + receive(:fix_cart_date) + allow_any_instance_of(ApplicationController).to \ + receive(:make_cart_compatible) + end + context 'successful create' do + context 'priviliged user, not a request' do + let!(:user) { UserMock.new(:checkout_person) } + let!(:reserver) { UserMock.new(:user) } + before do + mock_user_sign_in(user) + allow(cart).to receive(:reserver_id).and_return(reserver.id) + creator = instance_spy('ReservationCreator', + create!: { result: 'msg', error: nil }, + request?: false) + allow(ReservationCreator).to receive(:new).and_return(creator) + post :create, reservation: { id: 1 } end - - context 'and user cannot override errors' do - # request would be filed - before(:each) do - mock_app_config(AC_DEFAULTS.merge(override_on_create: false)) - sign_in @checkout_person - end - it 'affects database' do - expect { @req.call }.to change { Reservation.count } - end - - it 'sets the reservation notes' do - @req.call - expect(Reservation.last.notes.empty?).not_to be_truthy - end - - it 'redirects to catalog_path' do - @req.call - expect(response).to redirect_to(catalog_path) - end - - it 'should not set the flash' do - @req.call - expect(flash[:error]).to be_nil - end + it { is_expected.to set_flash[:notice] } + it do + is_expected.to \ + redirect_to(manage_reservations_for_user_path(reserver.id)) end end - - context 'with validation-passing items in Cart' do - before(:each) do - @valid_cart = FactoryGirl.build(:cart_with_items) - @req = proc do - post :create, - { reservation: { start_date: Time.zone.today, - due_date: (Time.zone.today + 1.day), - reserver_id: @user.id } }, - cart: @valid_cart - end - end - - it 'saves items into database' do - expect { @req.call }.to change { Reservation.count } - end - - it 'sets the reservation notes' do - @req.call - expect(Reservation.last.notes.empty?).not_to be_truthy - end - - it 'sets the status to reserved' do - @req.call - expect(Reservation.last.reserved?) - end - - it 'empties the Cart' do - @req.call - expect(response.request.env['rack.session'][:cart].items.count) - .to eq(0) - # Cart.should_receive(:new) - end - - it 'sets flash[:notice]' do - @req.call - expect(flash[:notice]).not_to be_nil - end - - it 'is a redirect' do - @req.call - expect(response).to be_redirect - end - - context 'with notify_admin_on_create set' do - before(:each) do - ActionMailer::Base.deliveries.clear - mock_app_config(AC_DEFAULTS.merge(notify_admin_on_create: true)) - end - - it 'cc-s the admin on the confirmation email' do - @req.call - delivered = ActionMailer::Base.deliveries.last - expect(delivered).not_to be_nil - expect(delivered.subject).to \ - eq('[Reservations] Reservation created') - end - end - - context 'without notify_admin_on_create set' do - before(:each) do - ActionMailer::Base.deliveries.clear - mock_app_config(AC_DEFAULTS.merge(notify_admin_on_create: false)) - end - - it 'cc-s the admin on the confirmation email' do - @req.call - delivered = ActionMailer::Base.deliveries.last - expect(delivered).to be_nil - end + context 'nonpriviliged user' do + let!(:user) { UserMock.new(:user) } + before do + mock_user_sign_in(user) + creator = instance_spy('ReservationCreator', + create!: { result: 'msg', error: nil }, + request?: false) + allow(ReservationCreator).to receive(:new).and_return(creator) + post :create, reservation: { id: 1 } end + it { is_expected.to set_flash[:notice] } + it { is_expected.to redirect_to(catalog_path) } end - - context 'with banned reserver' do - before(:each) do - sign_in @checkout_person - @valid_cart = FactoryGirl.build(:cart_with_items) - @banned = FactoryGirl.create(:banned) - @valid_cart.reserver_id = @banned.id - @req = proc do - post :create, - { reservation: { start_date: Time.zone.today, - due_date: (Time.zone.today + 1.day), - reserver_id: @banned.id, - notes: 'because I can' } }, - cart: @valid_cart - end + context 'request' do + let!(:user) { UserMock.new(:checkout_person) } + before do + mock_user_sign_in(user) + creator = instance_spy('ReservationCreator', + create!: { result: 'msg', error: nil }, + request?: true) + allow(ReservationCreator).to receive(:new).and_return(creator) + post :create, reservation: { id: 1 } end + it { is_expected.to set_flash[:notice] } + it { is_expected.to redirect_to(catalog_path) } + end + end - it 'does not save' do - expect { @req.call }.not_to change { Reservation.count } + context 'unsuccessful create' do + context 'needs notes' do + before do + mock_app_config(AC_DEFAULTS.merge(request_text: '')) + mock_user_sign_in(UserMock.new(:checkout_person)) + creator = instance_spy('ReservationCreator', + create!: { result: nil, error: 'needs notes' }) + allow(ReservationCreator).to receive(:new).and_return(creator) + post :create, reservation: { id: 1 } end - - it 'is a redirect' do - @req.call - expect(response).to be_redirect + it { is_expected.to set_flash[:error] } + it { is_expected.to render_template(:new_request) } + end + context 'requests disabled' do + before do + mock_user_sign_in(UserMock.new(:checkout_person)) + creator = instance_spy('ReservationCreator', + create!: { result: nil, + error: 'requests disabled' }) + allow(ReservationCreator).to receive(:new).and_return(creator) + post :create, reservation: { id: 1 } end - - it 'sets flash[:error]' do - @req.call - expect(flash[:error]).not_to be_nil + it { is_expected.to set_flash[:error] } + it { is_expected.to render_template(:requests_disabled) } + end + context 'other error' do + before do + mock_user_sign_in(UserMock.new(:checkout_person)) + creator = instance_spy('ReservationCreator', + create!: { result: nil, error: 'err' }) + allow(ReservationCreator).to receive(:new).and_return(creator) + post :create, reservation: { id: 1 } end + it { is_expected.to set_flash[:error] } + it { is_expected.to redirect_to(catalog_path) } + end + end + it_behaves_like 'inaccessible by banned user' do + before do + post :create, + reservation: FactoryGirl.attributes_for(:valid_reservation) end end end diff --git a/spec/factories/app_configs.rb b/spec/factories/app_configs.rb index 20a4af58b..8e5124428 100644 --- a/spec/factories/app_configs.rb +++ b/spec/factories/app_configs.rb @@ -27,5 +27,6 @@ notify_admin_on_create false checkout_persons_can_edit false request_text 'tell me whyyy?' + disable_requests false end end diff --git a/spec/lib/reservation_creator_spec.rb b/spec/lib/reservation_creator_spec.rb new file mode 100644 index 000000000..996d70d0d --- /dev/null +++ b/spec/lib/reservation_creator_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe ReservationCreator do + let!(:current_user) { UserMock.new } + describe 'create!' do + shared_examples 'successful create' do |cart:, **attrs| + it do + creator = ReservationCreator.new(cart: instance_spy('Cart', **cart), + current_user: current_user, + **attrs) + results = creator.create! + expect(results[:result]).to be_truthy + end + end + shared_examples 'needs notes' do |cart:, **attrs| + it do + creator = ReservationCreator.new(cart: instance_spy('Cart', **cart), + current_user: current_user, + **attrs) + results = creator.create! + expect(results[:result]).to be_nil + expect(results[:error]).to eq('needs notes') + end + end + shared_examples 'unable to create' do |cart:, error: nil, **attrs| + it do + creator = ReservationCreator.new(cart: instance_spy('Cart', **cart), + current_user: current_user, + **attrs) + results = creator.create! + expect(results[:result]).to be_nil + expect(results[:error]).to eq(error) + end + end + context 'without errors' do + it_behaves_like 'successful create', cart: { validate_all: '' } + end + context 'with errors' do + context 'with notes' do + it_behaves_like 'successful create', notes: 'note', + cart: { validate_all: 'error' } + end + context 'without notes' do + it_behaves_like 'needs notes', cart: { validate_all: 'error' } + end + context 'requests disabled' do + before { mock_app_config(disable_requests: true) } + it_behaves_like 'unable to create', cart: { validate_all: 'error' }, + error: 'requests disabled', + notes: 'note' + end + end + end +end diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index d1974bff7..c2c8180f6 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -28,7 +28,11 @@ describe UserMailer, type: :mailer do before(:each) do @ac = mock_app_config(admin_email: 'admin@email.com', - disable_user_emails: false) + disable_user_emails: false, + upcoming_checkout_email_body: nil, + upcoming_checkin_email_body: nil, + deleted_missed_reservation_email_body: nil, + overdue_checkin_email_body: nil) ActionMailer::Base.delivery_method = :test ActionMailer::Base.perform_deliveries = true ActionMailer::Base.deliveries = [] diff --git a/spec/support/app_config_helpers.rb b/spec/support/app_config_helpers.rb index d9017ff9a..54259b8de 100644 --- a/spec/support/app_config_helpers.rb +++ b/spec/support/app_config_helpers.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module AppConfigHelpers def mock_app_config(**attrs) - ac = spy('AppConfig', require_phone: false, **attrs) + ac = instance_spy('AppConfig', require_phone: false, **attrs) allow(AppConfig).to receive(:first).and_return(ac) ac end From 35082cee448563386ce8676ec3a1bec586f068b7 Mon Sep 17 00:00:00 2001 From: Sydney Young Date: Fri, 2 Dec 2016 11:12:32 -0500 Subject: [PATCH 2/2] get rid of override? method --- lib/reservation_creator.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/reservation_creator.rb b/lib/reservation_creator.rb index 10c91f929..6a17b4a28 100644 --- a/lib/reservation_creator.rb +++ b/lib/reservation_creator.rb @@ -28,11 +28,7 @@ def error end def needs_notes? - (request? || override?) && notes.blank? - end - - def override? - override && !cart_errors.blank? + !cart_errors.blank? && notes.blank? end def reservation_transaction