Angular Staff Catalog: Place Another Hold & Multi-Holds

Bug #1889128 reported by Jennifer Pringle
20
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
High
Unassigned

Bug Description

EG 3.5beta

The new place hold screen in the angular catalogue is great, but there's currently no way to easily place another hold from that screen.

To place an additional hold you need to go back to the Record Details screen (by clicking on the title link in the "Placing TITLE holds on record(s)" section) and then place a new hold. You can also place an additional hold by refreshing the tab, but that's not necessary intuitive.

A button/link for "Place another hold" that refreshes the screen, or some other way to place another hold without having to leave the Place Hold screen would be great and similar in workflow to the current catalogue.

Revision history for this message
Bill Erickson (berick) wrote :

Thanks, Jennifer.

If a patron has been selected as the hold recipient for the first hold, should that data persist when placing subsequent holds or should that data be cleared?

Changed in evergreen:
status: New → Confirmed
Revision history for this message
Jennifer Pringle (jpringle-u) wrote :

Hi Bill, I'm not quite clear on your question. Do you mean should the data persist so subsequent holds can be placed for the same patron or so that staff can see who they have placed holds for in that session?

My assumption is that the patron data in the Place Hold screen should be cleared after each hold is placed so the next hold can be placed for a different patron.

Potentially related is the feature in the current catalogue that enables staff and patrons to specify how many holds should be placed on the title for the patron when placing a hold. (This may need to be a separate bug report.)

Revision history for this message
Bill Erickson (berick) wrote :

Jennifer, I was curious about placing holds for the same patron. Clearing the patron after each hold placement makes sense, thanks.

+1 to adding multi-hold placement as well. I'm find lumping it into this LP.

Changed in evergreen:
assignee: nobody → Bill Erickson (berick)
Bill Erickson (berick)
summary: - Angular Staff Catalog: Place Another Hold
+ Angular Staff Catalog: Place Another Hold & Multi-Holds
Revision history for this message
Bill Erickson (berick) wrote :
tags: added: pullrequest
Changed in evergreen:
milestone: none → 3.5.1
assignee: Bill Erickson (berick) → nobody
Revision history for this message
Bill Erickson (berick) wrote :

Note when testing multi-holds support, it's necessary to apply a value for the "circ.holds.max_duplicate_holds" org unit setting.

Changed in evergreen:
milestone: 3.5.1 → 3.5.2
Revision history for this message
Jane Sandberg (sandbergja) wrote :

This doesn't seem to be working for me in two ways:

1) When I place a hold for patron A and then enter a new barcode for patron B, the "Place Holds" button is still disabled, and the hold grid at the bottom of the screen still refers to patron A's hold.

2) The behavior when there is no circ.holds.max_duplicate_holds isn't great (there is an empty dropdown for Number of Copies). Could we just not display this field if there is no value for circ.holds.max_duplicate_hold? Not every library uses that feature, so I suspect a lot of libraries don't have a value entered there. Even better would be to only display the dropdown for patrons with the CREATE_DUPLICATE_HOLDS permission, when circ.holds.max_duplicate_hold exists and is true.

Bill Erickson (berick)
Changed in evergreen:
assignee: nobody → Bill Erickson (berick)
Revision history for this message
Bill Erickson (berick) wrote :

Thanks, Jane.

Rebased branch with an additional commit fixing the noted issues.

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1889128-staffcat-multi-holds-v2

Includes the CREATE_DUPLICATE_HOLDS check and a new warning displayed when a recipient barcode is entered which does not match any patrons.

Changed in evergreen:
assignee: Bill Erickson (berick) → nobody
Revision history for this message
Terran McCanna (tmccanna) wrote :

All the changes so far look good to me, except for two things:

1) When I place a hold for a patron and then switch the radio button from 'Place hold for patron' to 'Place hold for me', the form does not refresh and the info at the bottom does not update. I have to take the additional step of clicking Reset on the form.

2) On my monitor, the title/status line at the bottom is 'below the fold' so I have to scroll to see it. That was confusing for me and I can imagine it causing problems for staff who don't realize they need to scroll. Can the white space on that page be tightened up to pull that line up a bit?

Bill Erickson (berick)
Changed in evergreen:
assignee: nobody → Bill Erickson (berick)
Revision history for this message
Bill Erickson (berick) wrote :

The code for this bug and for bug #1887429 are now stepping on each other's toes. I've opted to merge the 2 into a single branch. Will mark bug #1887429 as a duplicate.

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1887429-lp1889128-hold-prefs-and-multi

New commits to this merged branch are these:

1. Add support for the 'circ.staff_placed_holds_fallback_to_ws_ou' org unit setting when staff place holds for patrons in the Angular staff catalog. Specifically, if the patron has no preferred pickup lib set, fall back to either the patron's home org or the workstation org depending on the value of the org setting

2. Collapse the Search Form on the holds page so the holds form and data have more vertical room on the page. Additionally, to help alleviate any confusion that may be caused by the collapsed search form (and because it has come up in other conversations), add a 'Return' button to the holds page so users have an obvious way back to the previous catalog page.

Fixes an issue where setting the hold recipient from a patron to the staff account failed to properly clear the holds form, leaving (e.g.) the wrong pickup lib in the selector.

---

Revision history for this message
Michele Morgan (mmorgan) wrote :

Thanks for your work on this Bill!

Unfortunately, I retested the hold pickup location on butternut.evergreencatalog.com, and it appears to be taking the patron's home library as the pickup location, regardless of the circ.staff_placed_holds_fallback_to_ws_ou org unit setting, and regardless of whether the patron has a default pickup location preference.

Revision history for this message
Bill Erickson (berick) wrote :

Thanks, Michele.

I'm unable to reproduce the failure on my local test VM.

I see on butternut the org setting is applied to BR1. Does that match your workstation?

Revision history for this message
Bill Erickson (berick) wrote :

Scratch may question about workstation org unit, I just confirmed it's not working for me either on butternut. The UI /looks/ like it has all of the changes, so I'm stumped what's different.

Revision history for this message
Andrea Neiman (aneiman) wrote :

Confirming that when I test on butternut and try to place a hold for patron Ramos (99999389066, home lib BR2, pickup location BR3, staff workstation BR1), it defaults to hold pickup location of BR2 regardless of whether the fallback OUS is set to true or false.

If I test with patron Lamb (99999355318, home lib BR2, pickup lib unset, staff workstation BR1), it again defaults to BR2 regardless of the OU setting.

Revision history for this message
Michele Morgan (mmorgan) wrote :

Adding the angularcatblocker tag from duplicate bug 1887429 because staff placed holds can end up with the wrong pickup location.

tags: added: angularcatblocker
Revision history for this message
Bill Erickson (berick) wrote :

Hoping someone can test this on a VM that's not hosting other patches. Hoping to narrow down the source of the discrepancy between my test VM and butternut.evergreencatalog.com

Changed in evergreen:
assignee: Bill Erickson (berick) → nobody
Revision history for this message
Bill Erickson (berick) wrote :
Revision history for this message
Michele Morgan (mmorgan) wrote :

Thanks for the new branch, Bill, and thanks to Terran for the test server.

First, the good:

1. I've done some testing and the pickup location is now working as it should, using the patron's pickup preference if set, and observing the 'circ.staff_placed_holds_fallback_to_ws_ou' org unit setting such that:

If the patron has no pickup preference:

Patron's home library is used if circ.staff_placed_holds_fallback_to_ws_ou is False or unset
Staff client workstation is used if circ.staff_placed_holds_fallback_to_ws_ou is True

2. The issues Terran reported in comment 8 appear resolved.

3. Issue 2) reported by Jane in comment 6 is resolved, the number of holds option does not appear unless the org unit setting 'circ.staff_placed_holds_fallback_to_ws_ou' is True.

Now for the less good:

When testing Jane's issue 1) from comment 6 (place hold for patron A, then for patron B), I observed what looks like a manifestation of bug 1778606 (which was fixed in 3.5), but the behavior is somewhat worse in the angular catalog. Here's the workflow that shows the problem:

* Perform a catalog search
* Select Place Hold
* Enter the patron's barcode (99999380162) in the 'Place hold for patron by barcode' field but DO NOT click Submit
* Click Place Hold(s)

Patron preferences don't get filled in and the Place Hold(s) button is inactive. The hold does not get placed no matter how many times you click Place Hold(s). Clicking Submit, or clicking elsewhere on the page, or in another window will activate the Place Hold(s) button, and then the hold will be placed.

* Change the patron's barcode in the input box (99999378730), but DO NOT click submit.
* Click the Place Hold(s) button, patron preferences fill in at that point
* Click the Place Hold(s) button again and the hold is placed

I also ran into another issue when placing the hold for the second patron. This was intermittent, but happened more than once and I'm not sure what the difference in procedure was to cause it. After placing a hold for the first patron as above:

* Change the patron's barcode in the input box (99999378730), but DO NOT click submit.
* Click the Place Hold(s) button

The results grid on the bottom of the page shows "Hold Processing..." and appears stuck.
The console shows:

ERROR TypeError: Cannot read property 'part' of undefined
    at HoldComponent.placeOneHold (hold.component.ts:460)
    at HoldComponent.placeHolds (hold.component.ts:430)
    at HoldComponent_Template_button_click_90_listener (template.html:184)
    at executeListenerWithErrorHandling (core.js:14415)
    at wrapListenerIn_markDirtyAndPreventDefault (core.js:14450)
    at HTMLButtonElement.<anonymous> (platform-browser.js:582)
    at ZoneDelegate.invokeTask (zone-evergreen.js:399)
    at Object.onInvokeTask (core.js:27136)
    at ZoneDelegate.invokeTask (zone-evergreen.js:398)
    at Zone.runTask (zone-evergreen.js:167)

All the recent changes to place holds are really looking good, but the fix for bug 1778606 should be applied to the angular catalog to avoid a regression.

Revision history for this message
Bill Erickson (berick) wrote :

Thanks, Michele

Do you see not clicking Submit (or Enter key) after entering the patron barcode as a typical work flow? If so, I can confirm that won't fly with the current code and could lead to unexpected behavior.

Revision history for this message
Michele Morgan (mmorgan) wrote :

I can see the Submit/Enter step after entering the patron barcode could be easily overlooked especially as staff place holds for multiple patrons. If there's no need to review the pickup and notification preferences for a hold, then the Submit/Enter is an extra step.

The 3.5 behavior that resulted from bug 1778606 for the TPAC is quite nice and I'd hate to lose it. It eliminates the Submit/Enter step altogether. Though if carrying that behavior over to the angular catalog is too involved, it could be pushed to another bug.

Revision history for this message
Terran McCanna (tmccanna) wrote :

Does scanning the barcode automatically include the carriage return, and so eliminate the need to tab or click submit/enter? If so, this will be the same behavior as in the current staff client.

I have been testing from a machine at home without a barcode scanner and typing or copying and pasting in barcodes for testing, so I found the need to click Submit confusing as well, but I assume that it would mainly be used with a barcode scanner.

Revision history for this message
Bill Erickson (berick) wrote :

Yes, a barcode scanner would include the carriage return and remove the need to Submit or Enter.

Revision history for this message
Michele Morgan (mmorgan) wrote :

Agreed, the barcode scanner does provide the carriage return, eliminating the need for the user to click submit or press the enter key. But the existence of bug 1778606 suggests there are plenty of holds being placed by typing or pasting in barcodes.

Revision history for this message
Terran McCanna (tmccanna) wrote :

I agree that it is problematic, but I would be in favor of making it a separate bug since it is existing behavior in the current staff client.

Revision history for this message
Bill Erickson (berick) wrote :

Regarding the "Cannot read property 'part' of undefined" error, I have identified the potential for a race condition that could cause that error. I have pushed a new commit to the same working branch to avoid that race condition.

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1887429-lp1889128-hold-prefs-and-multi-v2

Revision history for this message
Michele Morgan (mmorgan) wrote :

I'm leaning in the direction of a separate bug for the Submit/Enter issue as well. Bug 1778606 is fixed in the 3.5 staff catalog, so staff users will see the improved behavior if they choose the traditional catalog.

Terran, if you could find time to load Bill's latest branch on your test server, I'd be happy to test it.

Revision history for this message
Michele Morgan (mmorgan) wrote :

Based on a discussion in IRC, waiting for a branch that clears the patron info after a hold is places as reference in comment #3. Thanks for all your work on this Bill!

Bill Erickson (berick)
Changed in evergreen:
assignee: nobody → Bill Erickson (berick)
Revision history for this message
Bill Erickson (berick) wrote :

New patch pushed to same branch:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1887429-lp1889128-hold-prefs-and-multi-v2

I found some other stuff while testing, so the patch contains 3 fixes:

* When all holds for a given recipient have been successfully placed, directly or via override, clear the recipient data so new recipient data can easily be added.

* When placing duplicate Part holds, ensure the same part is applied to each hold context so the selected part is targeted for all of the multi-holds.

* Fixes a regression in the display of multiple hold targets (e.g. from buckets) where only the first target would display.

Changed in evergreen:
assignee: Bill Erickson (berick) → nobody
Revision history for this message
Michele Morgan (mmorgan) wrote :

This is shaping up nicely! I've just found one issue that can cause a problem with patron notifications.

The user setting opac.default_phone isn't consulted. To reproduce, edit a patron record, (99999336610, for example). Set the following:

Day phone: 888-555-1111
User Setting Default Phone Number: 888-555-2222

Perform a search and click Place hold.
Enter the patron's barcode and click Submit.

The phone number that fills in the place hold form is 888-555-1111.

Remove the Day phone from the patron record.

Place another hold for the patron, no phone number fills in the form.

Bill Erickson (berick)
Changed in evergreen:
assignee: nobody → Bill Erickson (berick)
Revision history for this message
Bill Erickson (berick) wrote :

Thanks, Michele. New rebased branch pushed:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1887429-lp1889128-hold-prefs-and-multi-v3

This adds support for the 'opac.default_phone' user setting and a patch to clear the patron barcode from the barcode input when the user clicks on 'place hold for this staff account' to avoid any potential confusion.

Changed in evergreen:
assignee: Bill Erickson (berick) → nobody
Revision history for this message
Michele Morgan (mmorgan) wrote :

Thanks for all your work on this, Bill! I tested the latest branch, and the patron's phone preference is now filling in properly.

Unfortunately, there are other user settings that are not being consulted:

opac.default_sms_carrier
opac.default_sms_notify

With the org unit setting sms.enable set to TRUE, when I place a hold on behalf of a patron that has Default SMS Number and Default SMS Carrier set, the user preferences to not fill in on the place hold screen.

Bill Erickson (berick)
Changed in evergreen:
assignee: nobody → Bill Erickson (berick)
Revision history for this message
Bill Erickson (berick) wrote :

Thanks, Michele.

Fix pushed to the same branch (user/berick/lp1887429-lp1889128-hold-prefs-and-multi-v3) to add support for the opac.default_sms_carrier and opac.default_sms_notify user settings.

Revision history for this message
Michele Morgan (mmorgan) wrote :

Thanks Bill! But I found one more issue (sorry!)

When I choose to place a hold and set an activation date, the hold is successfully placed, and is suspended, but no activation date is saved in the hold.

Other than that, everything looks good!

Changed in evergreen:
milestone: 3.5.2 → 3.6.1
Revision history for this message
Bill Erickson (berick) wrote :

Thanks, Michele

Here's a new rebased branch that fixes the activation date issue and adds support for resetting/clearing the suspend and activation date values when the hold recipient changes.

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1887429-lp1889128-hold-prefs-and-multi-v4

Changed in evergreen:
assignee: Bill Erickson (berick) → nobody
Revision history for this message
Michele Morgan (mmorgan) wrote :

Thanks Bill! And thanks to Terran for the test system.

This looks great, it fixes the majority of the issues with the staff place holds screen. I'm adding my signoff below, but will open new bugs for the following two issues:

1. When placing a suspended hold and setting an activation date, the form accepts dates in the past. The current staff catalog prevents activation dates in the past.

2. When entering a patron barcode, a carriage return or click on Submit is required to fill in the patron's info and preferencess (see discussion in comments #17 - #19). The current staff catalog fills in the patron info as soon as the barcode is entered.

My signoff branch is at:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/mmorgan/lp1889128_signoff

tags: added: signedoff
Changed in evergreen:
milestone: 3.6.1 → 3.6.2
Changed in evergreen:
importance: Undecided → High
Changed in evergreen:
assignee: nobody → Jane Sandberg (sandbej)
Revision history for this message
Jane Sandberg (sandbergja) wrote :

Pushed to master and rel_3_6. Thanks, Bill, Michele, and Terran.

Changed in evergreen:
status: Confirmed → Fix Committed
Changed in evergreen:
assignee: Jane Sandberg (sandbej) → nobody
Changed in evergreen:
milestone: 3.6.2 → 3.6.3
Changed in evergreen:
milestone: 3.6.3 → 3.6.2
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.