Skip to content

Commit 389009f

Browse files
authored
Refactor item status code and add to item search page (#2220)
* Consolidates most of the status display and search code into a single file and names a concept that was always present: borrow status (which is if an item is available, checked out, etc). There are less nested conditionals in a view helper as a result. It also means that we're more consistent about how we were rendering status values for admins. * Creates two new helper methods to make working with the statuses in views clearer. We now have `item_status_label`, `member_item_status_label`, and `borrow_status_label`. * Adds tooltips to the two status labels in admin views to make it easier to understand what they mean. * Finally, adds these status labels to the new item search view.
1 parent e48bccf commit 389009f

18 files changed

Lines changed: 283 additions & 121 deletions

File tree

app/assets/stylesheets/admin.scss

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,8 @@ ul.simple-list {
155155
align-items: center;
156156
}
157157

158-
th.no-wrap {
158+
th.no-wrap,
159+
td.no-wrap {
159160
white-space: nowrap;
160161
}
161162
}

app/assets/stylesheets/application_styles.scss

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -517,7 +517,8 @@ $min-width: variables.$size-md + 1;
517517
}
518518

519519
.item-borrow-policy,
520-
.item-checkout-status,
520+
.item-status,
521+
.borrow-status,
521522
.item-days-until-due {
522523
font-size: 0.8em;
523524
position: relative;

app/assets/stylesheets/styles.scss

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -832,7 +832,8 @@ form.membership-amount {
832832
margin-top: 1em;
833833
}
834834

835-
.item-checkout-status {
835+
.item-status,
836+
.borrow-status {
836837
font-size: 0.8em;
837838
position: relative;
838839
top: -1px;

app/controllers/admin/items/searches_controller.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,9 @@ def show
3636
end
3737

3838
@q = scope.ransack(ransack_params)
39-
result = @q.result.includes(:categories, :borrow_policy).with_attached_image
39+
result = @q.result
40+
.includes(:categories, :borrow_policy, :active_holds, :checked_out_exclusive_loan)
41+
.with_attached_image
4042
@pagy, @items = pagy(result)
4143
end
4244

app/helpers/items_helper.rb

Lines changed: 22 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,6 @@ def item_status_options(disabled_statuses: [])
2323
end
2424
end
2525

26-
def item_retired_reason_name(reason)
27-
Item::RETIRED_REASON_NAMES[reason]
28-
end
29-
3026
def item_retired_reason_options(disabled_statuses: [])
3127
Item.retired_reasons.map do |key, value|
3228
description = " (#{Item::RETIRED_REASON_DESCRIPTIONS[key]})" if Item::RETIRED_REASON_DESCRIPTIONS[key]
@@ -178,31 +174,29 @@ def full_item_number(item)
178174
item.complete_number
179175
end
180176

181-
def css_class_and_status_label(item)
177+
# Admins see both statuses side by side, so each is labeled with a tooltip
178+
def item_status_label(item)
179+
status_label item.full_status_name, css_class: "item-status", tooltip: "item status"
180+
end
181+
182+
def borrow_status_label(item, tooltip: "borrow status")
183+
return unless item.in_circulation?
184+
185+
status_label item.borrow_status_name,
186+
css_class: "borrow-status #{Item::BORROW_STATUS_CSS_CLASSES[item.borrow_status]}",
187+
tooltip: tooltip
188+
end
189+
190+
# Members see a single label: how borrowable an active item is, or why an item
191+
# that has left circulation can't be borrowed at all.
192+
def member_item_status_label(item)
182193
if item.active?
183-
if item.checked_out_exclusive_loan
184-
if item.overdue?
185-
["label-error", "Overdue"]
186-
else
187-
["label-warning", "Checked Out"]
188-
end
189-
elsif item.borrow_policy.uniquely_numbered? && item.active_holds.size > 0
190-
["label-warning", "On Hold"]
191-
else
192-
["label-success", "Available"]
193-
end
194-
elsif item.maintenance?
195-
["", "In Maintenance"]
194+
borrow_status_label(item, tooltip: nil)
196195
else
197-
["", "Unavailable"]
196+
status_label item.member_status_name, css_class: "item-status"
198197
end
199198
end
200199

201-
def item_status_label(item)
202-
class_name, label = css_class_and_status_label(item)
203-
tag.span label, class: "label item-checkout-status #{class_name}"
204-
end
205-
206200
def days_until_due_label(item)
207201
return unless show_days_until_due?(item)
208202

@@ -216,13 +210,10 @@ def days_until_due_label(item)
216210
tag.span text, class: "label item-days-until-due"
217211
end
218212

219-
def item_holds_label(item)
220-
if item.active?
221-
count = item.active_holds.size
222-
if count > 0
223-
tag.span pluralize(count, "hold"), class: "label item-hold-status"
224-
end
225-
end
213+
private def status_label(name, css_class:, tooltip: nil)
214+
css_class = "tooltip tooltip-bottom #{css_class}" if tooltip
215+
216+
tag.span name, class: "label #{css_class}", data: {tooltip: tooltip}
226217
end
227218

228219
def audit_item_status(audit)
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
require "active_support/concern"
2+
3+
module ItemBorrowStatuses
4+
extend ActiveSupport::Concern
5+
6+
# Where an item is in the borrowing cycle. This is independent of the item's
7+
# status: a retired item can still be checked out, and a pending item is
8+
# available in the sense that nobody has it.
9+
BORROW_STATUS_NAMES = {
10+
"available" => "Available",
11+
"on_hold" => "On Hold",
12+
"checked_out" => "Checked Out",
13+
"overdue" => "Overdue"
14+
}
15+
16+
BORROW_STATUS_CSS_CLASSES = {
17+
"available" => "label-success",
18+
"on_hold" => "label-warning",
19+
"checked_out" => "label-warning",
20+
"overdue" => "label-error"
21+
}
22+
23+
# Items are ordered in search results by how borrowable they are: active
24+
# items first, ordered by borrow status, then everything a member can't
25+
# borrow right now.
26+
BORROW_STATUS_SEARCH_PRIORITIES = {
27+
"available" => 1,
28+
"on_hold" => 2,
29+
"checked_out" => 3,
30+
"overdue" => 4
31+
}
32+
33+
MAINTENANCE_SEARCH_PRIORITY = 5
34+
UNBORROWABLE_SEARCH_PRIORITY = 6
35+
36+
def borrow_status
37+
if checked_out_exclusive_loan
38+
overdue? ? "overdue" : "checked_out"
39+
elsif borrow_policy.uniquely_numbered? && active_holds.size > 0
40+
"on_hold"
41+
else
42+
"available"
43+
end
44+
end
45+
46+
def borrow_status_name
47+
BORROW_STATUS_NAMES[borrow_status]
48+
end
49+
50+
class_methods do
51+
# SQL equivalent of #borrow_status, for ordering search results. Expects the
52+
# relation to join loans, borrow_policies, and the active_hold_counts CTE.
53+
def borrow_status_search_priority
54+
Arel::Nodes::Case.new
55+
.when(Loan.arel_table[:id].not_eq(nil)).then(
56+
Arel::Nodes::Case.new
57+
.when(Loan.arel_table[:due_at].lt(Time.current))
58+
.then(BORROW_STATUS_SEARCH_PRIORITIES["overdue"])
59+
.else(BORROW_STATUS_SEARCH_PRIORITIES["checked_out"])
60+
)
61+
.when(
62+
Arel::Nodes::And.new([
63+
BorrowPolicy.arel_table[:uniquely_numbered],
64+
Arel::Nodes::SqlLiteral.new("active_hold_counts.item_id IS NOT NULL")
65+
])
66+
).then(BORROW_STATUS_SEARCH_PRIORITIES["on_hold"])
67+
.else(BORROW_STATUS_SEARCH_PRIORITIES["available"])
68+
end
69+
70+
def search_priority
71+
Arel::Nodes::Case.new(arel_table[:status])
72+
.when("active").then(borrow_status_search_priority)
73+
.when("maintenance").then(MAINTENANCE_SEARCH_PRIORITY)
74+
.else(UNBORROWABLE_SEARCH_PRIORITY)
75+
.as("search_priority")
76+
end
77+
end
78+
end

app/models/concerns/item_statuses.rb

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,13 @@ module ItemStatuses
1111
"retired" => "Retired"
1212
}
1313

14+
# Members are shown the borrow status of active items, so these only come up
15+
# for items that have left circulation. Members normally can't see those at
16+
# all, but an item saved for later can end up in any status.
17+
MEMBER_STATUS_NAMES = Hash.new("Unavailable").merge(
18+
"maintenance" => "In Maintenance"
19+
).freeze
20+
1421
STATUS_DESCRIPTIONS = {
1522
"pending" => "just acquired; not ready to loan",
1623
"active" => "available to loan",
@@ -33,6 +40,29 @@ module ItemStatuses
3340
"upgraded" => "replaced with a newer or better item"
3441
}
3542

43+
def status_name
44+
STATUS_NAMES[status]
45+
end
46+
47+
# The status, plus why the item was retired when we know it
48+
def full_status_name
49+
retired_reason ? "#{status_name} (#{retired_reason_name})" : status_name
50+
end
51+
52+
def member_status_name
53+
MEMBER_STATUS_NAMES[status]
54+
end
55+
56+
# Items that are part of the circulating inventory. Anything else has either
57+
# not entered circulation yet or has left it, so its borrow status is moot.
58+
def in_circulation?
59+
active? || maintenance?
60+
end
61+
62+
def retired_reason_name
63+
RETIRED_REASON_NAMES[retired_reason]
64+
end
65+
3666
included do
3767
enum :status, {
3868
pending: "pending",

app/models/item.rb

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
class Item < ApplicationRecord
22
include ItemCategorization
33
include ItemStatuses
4+
include ItemBorrowStatuses
45
include ItemNumbering
56

67
include PgSearch::Model
@@ -113,27 +114,6 @@ def next_hold
113114
.left_joins(:checked_out_exclusive_loan)
114115
.left_joins(:borrow_policy)
115116

116-
items = arel_table
117-
search_priority = Arel::Nodes::Case
118-
.new(items[:status])
119-
.when("active").then(
120-
Arel::Nodes::Case.new
121-
.when(Loan.arel_table[:id].not_eq(nil)).then(
122-
Arel::Nodes::Case.new
123-
.when(Loan.arel_table[:due_at].lt(Time.current)).then(4).else(3)
124-
)
125-
.when(
126-
Arel::Nodes::And.new([
127-
BorrowPolicy.arel_table[:uniquely_numbered],
128-
Arel::Nodes::SqlLiteral.new("active_hold_counts.item_id IS NOT NULL")
129-
])
130-
).then(2)
131-
.else(1)
132-
)
133-
.when("maintenance").then(5)
134-
.else(6)
135-
.as("search_priority")
136-
137117
item_scope.select(
138118
"#{item_scope.pg_search_rank_table_alias}.rank",
139119
"items.*",

app/views/account/for_later_list_items/index.html.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
<div class="item-content">
1717
<%= tag.div class: "item-info" do %>
1818
<strong><%= link_to item.name, item_path(item), class: "item-name", id: "item-name-#{item.id}" %></strong>
19-
<%= item_status_label(item) %>
19+
<%= member_item_status_label(item) %>
2020
<% if item.borrow_policy.requires_approval? %>
2121
<span class="label label-secondary item-borrow-policy"><%= item.borrow_policy.code %>-Tool</span>
2222
<% end %>

app/views/admin/items/_item_panel.html.erb

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,7 @@
1111
<h6>
1212
<strong><%= item.complete_number %></strong>
1313
<%= item_status_label(item) %>
14-
<span class="label item-checkout-status">
15-
<%= item.status.capitalize %>
16-
<% if item.status == "retired" %>(<%= item_retired_reason_name(item.retired_reason) %>)<% end %>
17-
</span>
14+
<%= borrow_status_label(item) %>
1815
</h6>
1916
</div>
2017

0 commit comments

Comments
 (0)