-
Notifications
You must be signed in to change notification settings - Fork 57
[1624] Add option to disable requests #1625
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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> |
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' %> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's necessary for how the reservation_form JS works There was a problem hiding this comment. Choose a reason for hiding this commentThe 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> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
<div id='confirm-res-form'> | ||
<%= render partial: 'new_request' %> | ||
</div> |
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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
# frozen_string_literal: true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
There was a problem hiding this comment.
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 controllerThere was a problem hiding this comment.
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.