Request notes, only allow one pending request

Adds a model level validation on creation to make sure users can't spam requests

app/controllers/locker_rentals_controller.rb

@@ -5,6 +5,9 @@ class LockerRentalsController < ApplicationController
   before_action :signed_in
 
   def index
+    # Only admin can see index list
+    redirect_to :new_locker_rental unless current_user.admin?
+
     @locker_types = LockerType.all
     @locker_rentals =
       LockerRental.includes(:locker_type, :rented_by).order(
@@ -13,23 +16,24 @@ def index
   end
 
   def new
-    # fun fact this isn't used at all by the view
-    # because all the fields are given default values
-    # but we keep it in case something changes to avoid breakage
-    @locker_request = LockerRental.new
-    # Don't allow new request if user already has an active or pending request
+    @locker_rental = LockerRental.new
+    # Only locker types enabled by admin
+    new_instance_attributes
   end
 
   def create
     @locker_rental = LockerRental.new(locker_rental_params)
-    # if not a staff member or has debug value set
-    if !current_user.staff? || params[:locker_request][:ask]
-      # Wait for admin approval
+    # if not admin member or has debug value set
+    # then force to wait for admin approval
+    if !current_user.admin? || params.dig(:locker_rental, :ask)
       @locker_rental.state = :reviewing
+      @locker_rental.rented_by = current_user
     end
+
     if @locker_rental.save
-      redirect_back fallback_location: lockers_path
+      redirect_back fallback_location: :new_locker_rental
     else
+      new_instance_attributes
       render :new, status: :unprocessable_entity
     end
   end
@@ -45,8 +49,21 @@ def update
 
   private
 
+  def new_instance_attributes
+    @available_locker_types = LockerType.available
+    # Who users can request as
+    # because we want to localize later
+    @available_fors = {
+      staff: ("CEED staff member" if current_user.staff?),
+      student: ("GNG student" if current_user.student?),
+      general: "community member"
+    }.compact.invert
+    # Don't allow new request if user already has an active or pending request
+    @pending_locker_request = current_user.locker_rentals.under_review.first
+  end
+
   def locker_rental_params
-    if current_user.admin?
+    if current_user.admin? && !params.dig(:locker_rental, :ask)
       admin_params =
         params.require(:locker_rental).permit(
           :locker_type_id,
@@ -68,7 +85,7 @@ def locker_rental_params
       admin_params
     else
       # people pick where they want a locker
-      params.require(:locker_rental).permit(:locker_type)
+      params.require(:locker_rental).permit(:locker_type_id)
       #.merge({ state: :reviewing })
     end
   end

app/models/locker_rental.rb

@@ -15,8 +15,23 @@ class LockerRental < ApplicationRecord
          cancelled: "cancelled"
        }
 
+  # Locker type and state always present
   validates :locker_type, :state, presence: true
 
+  # When submitting a request, ensure only one is present
+  validate :only_one_request_per_user, on: :create
+
+  def only_one_request_per_user
+    # If this request is under review, and another by same user is also under review
+    if under_review? &&
+         LockerRental.where(rented_by: rented_by).exists?(
+           state: %i[reviewing await_payment]
+         )
+      errors.add(:rented_by, "already has a request under review")
+    end
+  end
+
+  # If set to active
   with_options if: :active? do |rental|
     # Don't double assign lockers of same locker type if full assigned
     # TODO upgrade Rails and add normalization to trim spaces
@@ -29,10 +44,17 @@ class LockerRental < ApplicationRecord
     rental.validates :rented_by, :owned_until, :locker_specifier, presence: true
   end
 
+  scope :under_review, -> { where(state: %i[reviewing cancelled]) }
+
   def full_locker_name
     "#{locker_type.short_form}##{locker_specifier}"
   end
 
+  def under_review?
+    state == :reviewing || state == :await_payment
+  end
+
+  # For the view
   def state_icon
     LockerRental.state_icon(state)
   end

app/models/user.rb

@@ -64,6 +64,7 @@ class User < ApplicationRecord
   has_many :staff_spaces, dependent: :destroy
   has_many :spaces, through: :staff_spaces
   has_many :printer_issues # Don't delete issues when a user is deleted
+  has_many :locker_rentals, foreign_key: "rented_by"
 
   MAX_AUTH_ATTEMPTS = 5
 

app/views/locker_rentals/new.html.erb

@@ -27,13 +27,34 @@
     </ul>
     <p>For any questions, contact us at <a href="mailto:[email protected]">[email protected]</a></p>
     <p>You may have only one pending request at a time.</p>
-    <%= form_with model: @locker_rental do |f| %>
-        <div class="row row-cols-auto align-items-center mb-3">
-            <%= f.label :locker_type, 'Request locker for', class: 'd-inline' %>
-            <%= f.select :locker_type, options_for_select(LockerType.available.pluck(:short_form, :id)), {}, { class: 'form-select w-auto d-inline' } %>
+    <% if @pending_locker_request %>
+        <hr />
+        <p>You have a pending request for a locker at <%= @pending_locker_request.locker_type.short_form %>, submitted on <%= @pending_locker_request.created_at.to_date %>.
+        You will receive a response shortly from administration.</p>
+    <% else %>
+        <%= form_with model: @locker_rental do |f| %>
+            <div class="row row-cols-auto align-items-center">
+                <%= f.label :locker_type_id, 'Request locker for', class: 'd-inline' %>
+                <%= f.select :locker_type_id, options_for_select(@available_locker_types.pluck(:short_form, :id)), {}, { class: 'form-select w-auto' } %>
+                <%= f.label :as_a, 'as a', class: 'd-inline' %>
+                <%= f.select :as_a, options_for_select(@available_fors), {}, { class: 'form-select w-auto' } %>
+            </div>
+            <%= f.label :notes, 'Notes with request:' %>
+            <%= f.text_area :notes, class: 'form-control' %>
+            <!-- This hidden field is for testing/debug purposes only, we're still checking credentials you skiddie -->
+            <%= f.hidden_field :ask %>
+            <%= f.submit 'Submit request', class: 'btn btn-primary my-3' %>
+        <% end %>
+    <% end %>
+
+    <% if @locker_rental.errors.any? %>
+        <div class="border">
+            <p class="fw-bold">Errors:</p>
+            <ul>
+                <% @locker_rental.errors.each do |error| %>
+                    <li><%= error.full_message %></li>
+                <% end %>
+            </ul>
         </div>
-        <!-- This hidden field is for testing/debug purposes only, we're still checking credentials you skiddie -->
-        <%= f.hidden_field :ask %>
-        <%= f.submit 'Submit request', class: 'btn btn-primary mb-3' %>
     <% end %>
 </div>

db/migrate/20241127234144_create_locker_types.rb

@@ -15,6 +15,7 @@ def change
       t.references :rented_by # user
       t.string :locker_specifier # concat with short_form for full locker ID
       t.string :state # not approved, ask for payment, paid
+      t.string :notes # Any notes left by requester or admin
       t.datetime :owned_until # NOTE should this be a string or a datetime?
       t.timestamps
     end

db/migrate/20241223024505_add_initial_locker_types.rb

@@ -0,0 +1,29 @@
+class AddInitialLockerTypes < ActiveRecord::Migration[7.0]
+  def up
+    LockerType.new(
+      short_form: "MLAB119",
+      description: "By Makerlab 119",
+      available_for: :staff,
+      quantity: 50,
+      cost: 20
+    ).save
+    LockerType.new(
+      short_form: "MLAB121",
+      description: "By Makerlab 121",
+      available_for: :staff,
+      quantity: 50,
+      cost: 20
+    ).save
+    LockerType.new(
+      short_form: "BRUNS",
+      description: "By the Brunsfield Centre",
+      available_for: :staff,
+      quantity: 50,
+      cost: 20
+    ).save
+  end
+
+  def down
+    LockerType.delete_all
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2024_11_27_234144) do
+ActiveRecord::Schema[7.0].define(version: 2024_12_23_024505) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
   enable_extension "unaccent"
@@ -602,6 +602,7 @@
     t.bigint "rented_by_id"
     t.string "locker_specifier"
     t.string "state"
+    t.string "notes"
     t.datetime "owned_until"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false