Enforce lend permissions

app/controllers/items_controller.rb

@@ -10,7 +10,11 @@
 
 class ItemsController < ApplicationController
   before_action :set_item,
-                only: %i[ show edit update destroy request_return accept_return request_lend]
+                only: %i[ show edit update destroy add_to_waitlist leave_waitlist add_to_favorites leave_favorites
+                          start_lend abort_lend request_return accept_return deny_return request_lend]
+
+  before_action :set_lendable, only: %i[ show ]
+  before_action :check_lendable, only: %i[ request_lend add_to_waitlist ]
 
   # GET /items or /items.json
   def index
@@ -19,7 +23,6 @@ def index
 
   # GET /items/1 or /items/1.json
   def show
-    @item = Item.find(params[:id])
     return unless @item.waitlist.nil?
 
     @item.waitlist = Waitlist.new
@@ -34,7 +37,6 @@ def new
 
   # GET /items/1/edit
   def edit
-    @item = Item.find(params[:id])
     @owner_id = @item.owning_user.nil? ? "group:#{@item.owning_group.id}" : "user:#{@item.owning_user.id}"
     @groups_with_current_user = Group.all.filter { |group| group.members.include? current_user }
     @lend_group_ids = @item.groups_with_lend_permission.map(&:id)
@@ -118,7 +120,6 @@ def destroy
   end
 
   def add_to_waitlist
-    @item = Item.find(params[:id])
     @user = current_user
 
     helpers.audit_add_to_waitlist(@item)
@@ -127,7 +128,6 @@ def add_to_waitlist
   end
 
   def leave_waitlist
-    @item = Item.find(params[:id])
     @user = current_user
     @item.remove_from_waitlist(@user)
     @item.save
@@ -138,14 +138,12 @@ def leave_waitlist
   end
 
   def add_to_favorites
-    @item = Item.find(params[:id])
     @user = current_user
     @user.favorites << (@item)
     redirect_to item_url(@item), notice: t("views.show_item.enter_favorites")
   end
 
   def leave_favorites
-    @item = Item.find(params[:id])
     @user = current_user
     @user.favorites.delete(@item)
     redirect_to item_url(@item), notice: t("views.show_item.leave_favorites")
@@ -166,7 +164,6 @@ def request_lend
   end
 
   def start_lend
-    @item = Item.find(params[:id])
     @job = Job.find_by(item: @item)
     @job.destroy
     @holder = current_user.id
@@ -178,7 +175,6 @@ def start_lend
   end
 
   def abort_lend
-    @item = Item.find(params[:id])
     @job = Job.find_by(item: @item)
     @job.destroy
     @item.set_status_available
@@ -188,7 +184,6 @@ def abort_lend
   end
 
   def request_return
-    @item = Item.find(params[:id])
     @item.set_status_pending_return
     @item.save
     helpers.audit_request_return(@item)
@@ -202,7 +197,6 @@ def request_return
   end
 
   def accept_return
-    @item = Item.find(params[:id])
     @user = current_user
     @request_notification = ReturnRequestNotification.find_by(item: @item)
     @request_notification.destroy
@@ -218,7 +212,6 @@ def accept_return
   end
 
   def deny_return
-    @item = Item.find(params[:id])
     @user = current_user
     @request_notification = ReturnRequestNotification.find_by(item: @item)
     @request_notification.destroy
@@ -272,6 +265,15 @@ def set_item
     @item = Item.find(params[:id])
   end
 
+  def set_lendable
+    @lendable = @item.users_with_lend_permission.include?(current_user)
+  end
+
+  def check_lendable
+    set_lendable
+    render file: 'public/403.html', status: :forbidden unless @lendable
+  end
+
   # Only allow a list of trusted parameters through.
   def item_params
     params.require(:item).permit(:name, :category, :location, :description, :image, :price_ct, :rental_duration_sec,

app/views/items/_adaptive_lend_button.html.erb

@@ -10,12 +10,12 @@
             <%= button_to t('views.show_item.return'), { action: "request_return" }, class: "btn-primary" %>
         <% end %>
     <% elsif item.lend_status == "available" %>
-        <%= button_to t('views.show_item.lend'), { action: "request_lend" }, class: "btn-primary" %>
+        <%= button_to t('views.show_item.lend'), { action: "request_lend" }, class: @lendable ? "btn-primary" : "btn-secondary", disabled: !@lendable %>
     <% elsif !item.waitlist.users.exists?(current_user.id) %>
         <% if item.lend_status == "pending_lend_request" && !LendRequestNotification.find_by(receiver: item.owning_user, item: item, borrower: current_user).nil? %>
             <%= button_to t('views.show_item.pending_lend_request'), { }, class: "btn-secondary", disabled: true %>
         <% else %>
-            <%= button_to t('views.show_item.enter_waitlist'), { action: "add_to_waitlist" }, class: "btn-primary" %>
+            <%= button_to t('views.show_item.enter_waitlist'), { action: "add_to_waitlist" }, class: @lendable ? "btn-primary" : "btn-secondary", disabled: !@lendable %>
         <% end %>
     <% else %>
         <%= button_to t('views.show_item.leave_waitlist'), { action: "leave_waitlist" }, class: "btn-primary" %>

spec/features/items/show.html.erb_spec.rb

@@ -1,19 +1,25 @@
 require 'rails_helper'
 
+# rubocop:disable RSpec/MultipleMemoizedHelpers
 RSpec.describe "items/show", type: :feature do
   let(:owner) { create(:user) }
   let(:user) { create(:user) }
   let(:borrower) { create(:user) }
+  let(:lend_group) do
+    lend_group = create(:group)
+    lend_group.members << user
+    lend_group
+  end
   let(:item) do
     item = create(:item, owning_user: owner)
-    item.waitlist = create(:waitlist_with_item)
-    item.waitlist.item = item
+    item.waitlist = create(:waitlist, item: item)
+    item.groups_with_lend_permission << lend_group
     item
   end
   let(:item_lent) do
     item_lent = create(:lent, owning_user: owner, holder: borrower.id)
-    item_lent.waitlist = create(:waitlist_with_item)
-    item_lent.waitlist.item = item_lent
+    item_lent.waitlist = create(:waitlist, item: item_lent)
+    item_lent.groups_with_lend_permission << lend_group
     item_lent
   end
 
@@ -97,12 +103,18 @@
     expect(page).to have_css('main img[src^="data:"]')
   end
 
-  it "has lend button when item is available and not owner of item" do
+  it "has lend button when item is available and user has lend permission and not owner of item" do
     sign_in user
     visit item_path(item)
     expect(page).to have_button("Lend")
   end
 
+  it "has disabled lend button when item is available and user has no lend permission and not owner of item" do
+    sign_in create(:user)
+    visit item_path(item)
+    expect(page).to have_button("Lend", disabled: true)
+  end
+
   it "has pending lend request button when item is lent but not approved" do
     sign_in user
     visit item_path(item)
@@ -127,7 +139,7 @@
     sign_in borrower
     visit item_path(item_lent)
     find(:button, "Return").click
-    expect(page).to have_button("Waiting for return approval", disabled: true)
+    expect(page).to have_button("Waiting for return approval", disabled: true, class: "btn-secondary")
   end
 
   it "creates audit event when requesting for returning" do
@@ -138,12 +150,18 @@
                             triggering_user: borrower).count).to be(1)
   end
 
-  it "has enter waitlist button when not on list and item not available" do
+  it "has enter waitlist button when user has lend permission and is not on list and item not available" do
     sign_in user
     visit item_path(item_lent)
     expect(page).to have_button("Enter Waitlist")
   end
 
+  it "has disabled enter waitlist button when user has no lend permission and is not on list and item not available" do
+    sign_in create(:user)
+    visit item_path(item_lent)
+    expect(page).to have_button("Enter Waitlist", disabled: true, class: "btn-secondary")
+  end
+
   it "does not have an adaptive lend button when owner of item" do
     sign_in owner
     visit item_path(item)
@@ -357,3 +375,4 @@
     expect(page).to have_text I18n.t("views.show_item.less_than_months", months_amount: 6)
   end
 end
+# rubocop:enable RSpec/MultipleMemoizedHelpers

spec/features/notifications/lend_request_notification_spec.rb

@@ -1,11 +1,17 @@
 require "rails_helper"
 
+# rubocop:disable RSpec/MultipleMemoizedHelpers
 describe "Lend Request Notifications", type: :feature do
   let(:password) { 'password' }
   let(:user) { create(:user, password: password) }
   let(:owner) { create(:max) }
   let(:borrower) { create(:peter) }
   let(:item) { create(:item, owning_user: user) }
+  let(:lend_group) do
+    lend_group = create(:group)
+    lend_group.members << borrower
+    lend_group
+  end
 
   before do
     sign_in user
@@ -59,6 +65,7 @@
 
   it "user gets notified someone wants to lend his/her item" do
     item = create(:item, owning_user: owner)
+    item.groups_with_lend_permission << lend_group
     sign_in borrower
     visit item_path(item)
     click_button('Lend')
@@ -113,3 +120,4 @@
     expect(@notification.unread).to be false
   end
 end
+# rubocop:enable RSpec/MultipleMemoizedHelpers

spec/features/request/accept_lend_request_spec.rb

@@ -1,11 +1,20 @@
 require "rails_helper"
 
 describe "Requests handling", type: :feature do
+  let(:owner) { create(:max) }
+  let(:borrower) { create(:peter) }
+  let(:item) { create(:item, owning_user: owner) }
+  let(:lend_group) do
+    lend_group = create(:group)
+    lend_group
+  end
+
+  before do
+    lend_group.members << borrower
+    item.groups_with_lend_permission << lend_group
+  end
 
   it "if borrower send a lend request, the item status changes to pending_lend_request" do
-    owner = create(:max)
-    borrower = create(:peter)
-    item = create(:item, owning_user: owner)
     sign_in borrower
     visit item_path(item)
     click_button('Lend')
@@ -14,9 +23,6 @@
   end
 
   it "if owner accepts lend request, the item lend status changes to pending_pickup" do
-    owner = create(:max)
-    borrower = create(:peter)
-    item = create(:item, owning_user: owner)
     sign_in borrower
     visit item_path(item)
     click_button('Lend')
@@ -28,9 +34,6 @@
   end
 
   it "if borrower confirms pickup, the item lend status changes to lent" do
-    owner = create(:max)
-    borrower = create(:peter)
-    item = create(:item, owning_user: owner)
     sign_in borrower
     visit item_path(item)
     click_button('Lend')

spec/requests/lend_process_spec.rb

@@ -0,0 +1,54 @@
+require 'rails_helper'
+
+RSpec.describe "Lend Process", type: :request do
+  before do
+    @group = create(:group)
+    @owner = create(:user)
+    @item = create(:item, owning_user: @owner, groups_with_lend_permission: [@group])
+    @item.waitlist = create(:waitlist_with_item)
+    @borrower = create(:user)
+    @borrower.to_member_of!(@group)
+    @forbidden = create(:user)
+    @waiting = create(:user)
+    @waiting.to_member_of!(@group)
+  end
+
+  describe "POST /request_lend/item" do
+
+    it "with permission changes the item state to reflect the lend request" do
+      sign_in @borrower
+      expect do
+        post request_lend_path(@item)
+      end.to(change { @item.reload.lend_status }.from("available").to("pending_lend_request"))
+    end
+
+    it "without permission does not change the item state and returns forbidden" do
+      sign_in @forbidden
+      expect do
+        post request_lend_path(@item)
+      end.not_to(change { @item.reload.lend_status })
+      expect(response).to be_forbidden
+    end
+  end
+
+  describe "POST /add_to_waitlist/item" do
+    before do
+      @item.holder = @borrower.id
+    end
+
+    it "with permission adds a user to the waitlist" do
+      sign_in @waiting
+      expect do
+        post add_to_waitlist_path(@item)
+      end.to change(@item.waitlist.users, :count).by(1)
+    end
+
+    it "without permission without permission does not add the user to the waitlist and returns forbidden" do
+      sign_in @forbidden
+      expect do
+        post add_to_waitlist_path(@item)
+      end.not_to change(@item.waitlist.users, :count)
+      expect(response).to be_forbidden
+    end
+  end
+end