Merge pull request #1660 from uOttawa-Makerspace/feat/assign-keys-no-…

…form

Allow assigning keys to users with no request form completed

app/controllers/admin/keys_controller.rb

@@ -14,12 +14,7 @@ class Admin::KeysController < AdminAreaController
   before_action :set_key_request, only: %i[approve_key_request deny_key_request]
 
   def index
-    @keys =
-      Key.all.sort_by do |key|
-        value = key.number
-
-        value =~ /\A\d+\z/ ? value.to_i * -1 : Float::INFINITY
-      end
+    @keys = Key.includes(:space, :user, :supervisor).order(updated_at: :desc)
 
     @spaces = Space.order(name: :asc)
   end
@@ -79,8 +74,26 @@ def destroy
     redirect_to admin_keys_path
   end
 
+  def assign
+    @admin_options =
+      User.where(role: "admin").order("LOWER(name) ASC").pluck(:name, :id)
+    @staff_options =
+      User
+        .staff
+        .order("LOWER(name) ASC")
+        .map do |user|
+          label = ""
+          if user.key_request.blank?
+            label = " (No request form)"
+          elsif user.key_request.status_waiting_for_approval?
+            label = " (Request form awaiting approval)"
+          end
+          ["#{user.name} #{label}", user.id]
+        end
+  end
+
   def assign_key
-    user = KeyRequest.find(params[:key][:key_request_id]).user
+    user = User.find(params[:key][:user_id])
 
     key_transaction =
       KeyTransaction.new(
@@ -99,8 +112,7 @@ def assign_key
   end
 
   def revoke_key
-    if @key.status_held? &&
-         @key.update(user_id: nil, key_request_id: nil, status: :inventory) &&
+    if @key.status_held? && @key.update(user_id: nil, status: :inventory) &&
          @key.get_latest_key_transaction.update(
            return_date: Date.today,
            # Set deposit return date to today if deposit is zero
@@ -151,8 +163,8 @@ def key_params
     params.require(:key).permit(
       :number,
       :space_id,
+      :user_id,
       :supervisor_id,
-      :key_request_id,
       :status,
       :key_type,
       :custom_keycode,

app/models/key.rb

@@ -2,7 +2,7 @@ class Key < ApplicationRecord
   belongs_to :user, class_name: "User", optional: true
   belongs_to :supervisor, class_name: "User", optional: true
   belongs_to :space, optional: true
-  belongs_to :key_request, optional: true
+  # belongs_to :key_request, optional: true
   has_many :key_transactions, dependent: :destroy
 
   enum :status,
@@ -25,11 +25,11 @@ class Key < ApplicationRecord
               message: "A space is required"
             },
             if: :key_type_regular?
-  validates :key_request,
-            presence: {
-              message: "A key request is required"
-            },
-            if: :status_held?
+  # validates :key_request,
+  #           presence: {
+  #             message: "A key request is required"
+  #           },
+  #           if: :status_held?
 
   validates :number,
             presence: {
@@ -57,6 +57,10 @@ class Key < ApplicationRecord
             },
             unless: :key_type_regular?
 
+  def key_request
+    user&.key_request
+  end
+
   def get_latest_key_transaction
     KeyTransaction.where(key_id: id).order(created_at: :desc).first
   end

app/views/admin/keys/_keys_table.html.erb

@@ -45,6 +45,18 @@
               0/<%= KeyCertification::NUMBER_OF_STAFF_FILES %> (staff)<br>
               0/<%= KeyCertification::NUMBER_OF_SUPERVISOR_FILES %> (supervisor)
             <% end %>
+            <% if key.user.present? %>
+              <% if key.key_request.present? %>
+                <% if key.key_request.status_approved? %>
+                  1/1 (form)
+                <% else %>
+                  <span class="fw-bold text-danger">0/1 (form)</span>
+                  <%= link_to 'pending', key_request_path(key.key_request) %>
+                <% end %>
+              <% else %>
+                0/1 (form)
+              <% end %>
+            <% end %>
           </td>
           <td>
             <%= link_to 'Show', admin_key_path(key.id), class: 'w-100 py-1 mb-1 btn btn-secondary' %>
@@ -57,4 +69,4 @@
       <% end %>
     </tbody>
   </table>
-</div>
+</div>

app/views/admin/keys/assign.html.erb

@@ -1,16 +1,19 @@
 <section>
-  <h2 class="mb-3 fw-bold py-3 text-center">Assign key</h2>
+  <div class="mb-3 py-3 text-center">
+  <h2 class="fw-bold">Assign key #<%= @key.number %></h2>
+  <p class="fs-4"><%= @key.get_keycode %> - <%= @key.key_type.humanize %></p>
+  </div>
 
-  <%= form_for @key, url: { controller: 'admin/keys', action: 'assign_key' } do |f| %>
+  <%= form_with model: @key, url: { controller: 'admin/keys', action: 'assign_key' } do |f| %>
     <div class="row">
       <div class="col-xl-6 offset-xl-3 col-lg-8 offset-lg-2 col-md-10 offset-md-1 col-12">
         <div class="mb-3">
           <%= f.label :supervisor_id, 'Supervisor', class: 'form-label' %>
-          <%= f.select :supervisor_id, options_for_select(User.where(role: 'admin').order('LOWER(name) ASC').pluck(:name, :id), selected: @user.id), { prompt: 'Choose Supervisor...' }, { class: 'form-control', id: 'supervisor-select' } %>
+          <%= f.select :supervisor_id, options_for_select(@admin_options, selected: @user.id), { prompt: 'Choose Supervisor...' }, { class: 'form-control', id: 'supervisor-select' } %>
         </div>
         <div class="mb-3">
-          <%= f.label :key_request_id, 'User', class: 'form-label' %>
-          <%= f.select :key_request_id, options_for_select(KeyRequest.where(status: :approved).joins(:user).order('LOWER(users.name) ASC').pluck('users.name', 'key_requests.id')), { prompt: 'Choose User...' }, { class: 'form-control', id: 'key-request-select' } %>
+          <%= f.label :user_id, 'User', class: 'form-label' %>
+          <%= f.select :user_id, options_for_select(@staff_options), { prompt: 'Choose User...' }, { class: 'form-control', id: 'key-request-select' } %>
         </div>
         <div class="mb-5">
           <%= label_tag :deposit_amount, 'Deposit Amount', class: 'form-label' %>
@@ -23,4 +26,4 @@
       </div>
     </div>
   <% end %>
-</section>
+</section>

db/migrate/20250117010106_remove_key_request_from_keys.rb

@@ -0,0 +1,5 @@
+class RemoveKeyRequestFromKeys < ActiveRecord::Migration[7.0]
+  def change
+    remove_reference :keys, :key_request
+  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_12_23_024505) do
+ActiveRecord::Schema[7.0].define(version: 2025_01_17_010106) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
   enable_extension "unaccent"
@@ -541,15 +541,13 @@
     t.bigint "user_id"
     t.bigint "supervisor_id"
     t.bigint "space_id"
-    t.bigint "key_request_id"
     t.string "number"
     t.string "custom_keycode"
     t.integer "status", default: 0
     t.integer "key_type", default: 0
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
     t.string "additional_info", default: ""
-    t.index ["key_request_id"], name: "index_keys_on_key_request_id"
     t.index ["space_id"], name: "index_keys_on_space_id"
     t.index ["supervisor_id"], name: "index_keys_on_supervisor_id"
     t.index ["user_id"], name: "index_keys_on_user_id"

spec/controllers/admin/key_transactions_controller_spec.rb

@@ -53,8 +53,7 @@
             :regular_key_type,
             user_id: @admin.id,
             supervisor_id: @admin.id,
-            space_id: @space.id,
-            key_request_id: @key_request.id
+            space_id: @space.id
           )
         @key_transaction =
           create(

spec/controllers/admin/keys_controller_spec.rb

@@ -182,8 +182,7 @@
             :regular_key_type,
             space_id: @space.id,
             user_id: @user.id,
-            supervisor_id: @user.id,
-            key_request_id: kr.id
+            supervisor_id: @user.id
           )
 
         patch :update,
@@ -252,7 +251,32 @@
               params: {
                 key_id: key.id,
                 key: {
-                  key_request_id: @key_request.id,
+                  user_id: @key_request.user.id,
+                  supervisor_id: @admin.id
+                },
+                deposit_amount: 20
+              }
+
+        expect(flash[:notice]).to eq("Successfully assigned key")
+        expect(KeyTransaction.last.deposit_amount).to eq(20)
+        expect(KeyTransaction.last.user_id).to eq(@staff.id)
+        expect(KeyTransaction.last.key_id).to eq(key.id)
+        expect(Key.last.status).to eq("held")
+      end
+
+      it "should assign a key to staff even with no form completed" do
+        key =
+          create(
+            :key,
+            :inventory_status,
+            :regular_key_type,
+            space_id: @space.id
+          )
+        patch :assign_key,
+              params: {
+                key_id: key.id,
+                key: {
+                  user_id: @staff.id,
                   supervisor_id: @admin.id
                 },
                 deposit_amount: 20
@@ -273,7 +297,7 @@
               params: {
                 key_id: key.id,
                 key: {
-                  key_request_id: @key_request.id,
+                  user_id: @key_request.user.id,
                   supervisor_id: @admin.id
                 },
                 deposit_amount: 20
@@ -319,7 +343,7 @@
               params: {
                 key_id: key.id,
                 key: {
-                  key_request_id: @key_request.id,
+                  user_id: @key_request.user.id,
                   supervisor_id: @admin.id
                 },
                 deposit_amount: 20
@@ -348,7 +372,7 @@
               params: {
                 key_id: key.id,
                 key: {
-                  key_request_id: @key_request.id,
+                  user_id: @key_request.user.id,
                   supervisor_id: @admin.id
                 },
                 deposit_amount: 0

spec/models/key_spec.rb

@@ -6,7 +6,6 @@
       it { should belong_to(:user).without_validating_presence }
       it { should belong_to(:supervisor).without_validating_presence }
       it { should belong_to(:space).without_validating_presence }
-      it { should belong_to(:key_request).without_validating_presence }
     end
 
     context "has_many" do
@@ -37,8 +36,7 @@
             :held_status,
             space_id: @space.id,
             user_id: @user.id,
-            supervisor_id: @user.id,
-            key_request_id: @kr.id
+            supervisor_id: @user.id
           )
         expect(key.valid?).to be_truthy
       end