Skip to content
This repository was archived by the owner on Jul 24, 2020. It is now read-only.

[1624] Add option to disable requests #1625

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion app/controllers/app_configs_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
135 changes: 75 additions & 60 deletions app/controllers/reservations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [email protected]? && (cannot? :override, :reservation_errors)

# check for missing notes and validation errors
if [email protected]? && 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.<br/> #{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

Expand Down Expand Up @@ -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 '\
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a little out of scope but it seems like we should just set this as a default value for request_text rather than having this live in the controller

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea... that's a great point. I'll open up an issue.

'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.<br/> #{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,
Expand Down
1 change: 1 addition & 0 deletions app/views/app_configs/_form.erb
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
<fieldset>
<legend>Reservation Settings</legend><br />
<%= 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' %>
Expand Down
7 changes: 5 additions & 2 deletions app/views/cart_js/reservation_form.js.erb
Original file line number Diff line number Diff line change
@@ -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('<h1>Confirm Reservation</h1>');
<% elsif AppConfig.check(:disable_requests) %>
$('#confirm-res-form').html('<%= escape_javascript(render partial: '/reservations/requests_disabled') %>');
$('.page-header').html('<h1>Reservation Denied</h1>');
<% 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('<h1>Confirm Reservation Request</h1>');
<% end %>

Expand Down
8 changes: 4 additions & 4 deletions app/views/reservations/_new_request.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<div id="content" class="col-md-9">
<% unless @errors.empty? %>
<p>
<h3><i class="fa fa-exclamation-triangle warning-icon"></i> Would you like to resolve the following error(s)?</h3>
<h3><i class="fa fa-exclamation-triangle warning-icon"></i> Would you like to resolve the following error(s)?</h3>
</p>
<div class ="form-errors">
<% @errors.each do |msg| %>
Expand All @@ -13,13 +13,13 @@
</div>
<% end %>
<p>
<h4>Equipment requested for
<h4>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) %>:
</h4>
</h4>
</p>
<%= render partial: 'reservations/edit_reservation_form' %>
<div class="well">
Expand Down
14 changes: 7 additions & 7 deletions app/views/reservations/_new_reservation.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@
<div id="content" class="col-md-9">
<% unless @errors.empty? %>
<p>
<h3>
<i class="fa fa-exclamation-triangle warning-icon"></i>
Please be aware of the following errors:
</h3>
<h3>
<i class="fa fa-exclamation-triangle warning-icon"></i>
Please be aware of the following errors:
</h3>
</p>
<div class ="form-errors">
<% @errors.each do |msg| %>
Expand All @@ -17,13 +17,13 @@
</div>
<% end %>
<p>
<h4>Equipment Reserved for
<h4>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) %>:
</h4>
</h4>
</p>
<%= render partial: 'reservations/edit_reservation_form' %>
</div>
Expand Down
49 changes: 49 additions & 0 deletions app/views/reservations/_requests_disabled.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<% title "Reservation Denied" %>
<div id="content" class="col-md-9">
<% unless @errors.empty? %>
<p>
<h3><i class="fa fa-exclamation-triangle warning-icon"></i> Please resolve the following error(s):</h3>
</p>
<div class ="form-errors">
<% @errors.each do |msg| %>
<ul>
<li><%= msg %></li>
</ul>
<% end %>
</div>
<% end %>
<p>
<h4>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) %>:
</h4>
</p>
<%= render partial: 'reservations/edit_reservation_form' %>
<%= simple_form_for @reservation do |f| %>
<div class="form-group">
<%= f.button :submit, "Submit", id: 'finalize_reservation_btn' %>
</div>
<% end %>
</div>
<div id="sidebar" class="col-md-3">
<div id="sidebarbottom">
<div id="cart" class="well">
<div id="cartSpinner"></div>
<header class="cart-header">
<% if cannot? :manage, Reservation %>
<h2>My Cart</h2>
<% else %>
<h2>Cart</h2>
<% end %>
</header>
<%= form_tag url_for(action: 'change_reservation_dates', controller: "catalog"), remote: true, class: 'form-vertical', id: 'dates_form', method: :put do %>
<%# allow user to set start/end dates %>
<%= render partial: 'reservations/cart_dates' %>
<% end %>
<br>
</div>
</div>
</div>
8 changes: 2 additions & 6 deletions app/views/reservations/new.html.erb
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
<div id='confirm-res-form'>
<% if @errors.empty? or (can? :override, :reservation_errors) %>
<%= render :partial => "new_reservation" %>
<% else %>
<%= render :partial => "new_request" %>
<% end %>
</div>
<%= render partial: 'new_reservation' %>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why we need partials for this if we now have independent views for each of the different cases that are rendered independently - can we just move the partial content into the full views?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's necessary for how the reservation_form JS works

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea fair enough. I feel like there might be a way to improve this but it's really not critical.

</div>
3 changes: 3 additions & 0 deletions app/views/reservations/new_request.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<div id='confirm-res-form'>
<%= render partial: 'new_request' %>
</div>
3 changes: 3 additions & 0 deletions app/views/reservations/requests_disabled.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<div id='confirm-res-form'>
<%= render partial: 'requests_disabled' %>
</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddDisableRequestsToAppConfigs < ActiveRecord::Migration
def change
add_column :app_configs, :disable_requests, :boolean, default: false
end
end
3 changes: 2 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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|
Expand Down
48 changes: 48 additions & 0 deletions lib/reservation_creator.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# frozen_string_literal: true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OMG, Service Objects!

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
Loading