<% 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):
+
+
+ <% 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..6a17b4a28
--- /dev/null
+++ b/lib/reservation_creator.rb
@@ -0,0 +1,48 @@
+# 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?
+ !cart_errors.blank? && notes.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