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

1608 extract checkout object #1667

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
7 changes: 7 additions & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,13 @@ def skip_authentication?
.include?(params[:action]) && !guests_disabled?)
end

def set_user
@user = User.find(params[:user_id])
return unless @user.role == 'banned'
flash[:error] = 'This user is banned and cannot check out equipment.'
params[:banned] = true
end
Copy link
Collaborator

Choose a reason for hiding this comment

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

this should be renamed -- it does more than set the user


#-------- end before_filter methods --------#

def update_cart # rubocop:disable MethodLength, AbcSize
Expand Down
165 changes: 165 additions & 0 deletions app/controllers/manage_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
# frozen_string_literal: true
# rubocop:disable ClassLength
class ManageController < ApplicationController
load_and_authorize_resource
before_action :set_user, only: [:show, :checkout]
before_action :set_reservation, only: [:send_receipt]

private
Copy link
Collaborator

Choose a reason for hiding this comment

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

you should swap the order of the public and private sections


def set_reservation
@reservation = Reservation.find(params[:receipt_id])
end

def check_for_banned_user
if @user.role == 'banned'
flash[:error] = 'Banned users cannot check out equipment.'
redirect_to(root_path) && return
end
true
end

def check_terms_of_service
unless @user.terms_of_service_accepted ||
params[:terms_of_service_accepted].present?
flash[:error] = 'You must confirm that the user accepts the Terms of '\
'Service.'
redirect_to(:back) && return
end
true
end

def handle_overdue_reservations
if @user.overdue_reservations?
if can? :override, :checkout_errors
# Admins can ignore this
flash[:notice] = 'Admin Override: Equipment has been checked out '\
'successfully, even though the reserver has overdue equipment.'
else
# Everyone else is redirected
flash[:error] = 'Could not check out the equipment, because the '\
'reserver has reservations that are overdue.'
redirect_to(:back) && return
end
end
true
end

def approve_checkout
# check for banned user
return unless check_for_banned_user

# check terms of service
return unless check_terms_of_service

# Overdue validation
return unless handle_overdue_reservations
true
end
Copy link
Collaborator

Choose a reason for hiding this comment

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

all these checks can live inside a service object -- check out these examples from vesta:
service object, controller method, and handle_action

you can probably do something very similar by adding a less-generalized handle_action to this controller


def check_nonemptiness_of(checked_out_reservations)
if checked_out_reservations.empty?
flash[:error] = 'No reservation selected.'
redirect_to(:back) && return
end
true
end

def check_validity_of(checked_in_reservations)
unless checked_in_reservations
flash[:error] = 'One of the items you tried to check in has already '\
'been checked in.'
redirect_to(:back) && return
end
true
end

def check_uniqueness_of(checked_out_reservations)
unless Reservation.unique_equipment_items?(checked_out_reservations)
flash[:error] = 'The same equipment item cannot be simultaneously '\
'checked out in multiple reservations.'
redirect_to(:back) && return
end
true
end

def prep_receipt_page(check_in_set:, check_out_set:, user: nil)
@check_in_set = check_in_set
@check_out_set = check_out_set
@user = user if user
render('receipt', layout: 'application_with_search_sidebar') && return
end

public

def show # initializer
@check_out_set = @user.due_for_checkout.includes(:equipment_model)
@check_in_set = @user.due_for_checkin.includes(:equipment_model)

render :show, layout: 'application'
end

def checkout
Copy link
Collaborator

Choose a reason for hiding this comment

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

this process should also live in a service object (example)

return unless approve_checkout

checked_out_reservations =
CheckoutHelper.preprocess_checkout(params[:reservations],
@user, current_user)

return unless check_nonemptiness_of(checked_out_reservations)
return unless check_uniqueness_of(checked_out_reservations)

## Save reservations
Reservation.transaction do
begin
checked_out_reservations.each do |r|
CheckoutHelper.checkout_reservation(r, params[:reservations])
end
rescue ActiveRecord::RecordNotSaved, ActiveRecord::RecordInvalid => e
flash[:error] = "Checking out your reservation failed: #{e.message}"
redirect_to manage_reservations_for_user_path(@user)
raise ActiveRecord::Rollback
end
end

CheckoutHelper.update_tos(@user)
CheckoutHelper.send_checkout_receipts(checked_out_reservations)
Copy link
Collaborator

Choose a reason for hiding this comment

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

these methods belong in the batch/bulk service object

prep_receipt_page(check_in_set: [], check_out_set: checked_out_reservations)
end

def checkin
Copy link
Collaborator

Choose a reason for hiding this comment

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

same as the above -- this method should live inside a service object

# see comments for checkout, this method proceeds in a similar way
checked_in_reservations =
CheckoutHelper.preproccess_checkins(params[:reservations], current_user)

return unless check_validity_of(checked_in_reservations)

return unless check_nonemptiness_of(checked_in_reservations)
## Save reservations
Reservation.transaction do
begin
checked_in_reservations.each do |r|
CheckoutHelper.checkin_reservation(r, params[:reservations])
end
rescue ActiveRecord::RecordNotSaved, ActiveRecord::RecordInvalid => e
flash[:error] = "Checking in your reservation failed: #{e.message}"
redirect_to :back
raise ActiveRecord::Rollback
end
end

prep_receipt_page(check_in_set: checked_in_reservations, check_out_set: [],
user: checked_in_reservations.first.reserver)
end

def send_receipt
if UserMailer.reservation_status_update(@reservation, 'checked out')
.deliver_now
flash[:notice] = 'Successfully delivered receipt email.'
else
flash[:error] = 'Unable to deliver receipt email. Please contact '\
'administrator for more support.'
end
redirect_to @reservation
end
end
Loading