Open
Conversation
After one-day holds shipped in #2118, Tessa noticed that items which used to be no-holds were now letting members place regular 7-day holds. The real story was that staff had been flipping holds_enabled back on by hand and sometimes forgetting to set the new hold duration dropdown, and the original plan was for one-day holds to replace no-holds for high-demand seasonal items anyway. Migrates active holds_enabled=false items to holds_enabled=true with hold_duration=1, leaving retired items alone. Adds a before_save callback that nulls hold_duration whenever holds get disabled, so the two fields can't drift apart (a handful of items had already ended up in that inconsistent state). Closes #2157
jim
approved these changes
Apr 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What it does
Runs a data migration that flips the 57 active
holds_enabled=falseitems toholds_enabled=true, hold_duration=1, turning the old "no holds" set into one-day-hold items. Also adds abefore_saveonItemthat nullshold_durationwheneverholds_enabledgets unchecked, so the two fields can't drift apart.Why it is important
When one-day holds went out in #2118, Tessa flagged that items which used to be no-holds were showing up as regular 7-day holds. Digging through the audit trail, the actual story was that staff had been flipping the checkbox back on manually over the past few months, sometimes without setting the new hold duration dropdown. The original intent was always for one-day holds to replace no-holds for high-demand seasonal items (power washers, carpet cleaners, weed whackers, reel mowers), so rather than ask staff to re-touch 57 items by hand, we do it here.
The
holds_enabled=falsestate still works the way it always has, in case we want to bring it back — it's just no longer the default path for short-hold items.Closes #2157
UI Change Screenshot
No UI changes.
Implementation notes
The migration uses
update!with anaudit_commentrather thanupdate_all, so each converted item gets a readable entry in its audit history ("Converted from no-holds to 1-day hold (issue #2157)"). At 57 rows the perf cost of iterating is a rounding error.There are also 13 stragglers — items that staff previously flipped from no-holds back to enabled without setting a duration, so they currently act as 7-day holds. A couple are clearly intentional (named "Metal wheelbarrow - 7 day hold") and a few are ambiguous. I'll post the list on #2157 once this lands so Tessa can triage.