Bug 31579 - Improve reserve/request.pl to avoid mistakes with pick up locations
Summary: Improve reserve/request.pl to avoid mistakes with pick up locations
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: geraud.frappier
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-09-19 16:16 UTC by Blou
Modified: 2022-09-27 19:47 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 31579: Improve reserve/request.pl to avoid mistakes with pick up locations (5.97 KB, patch)
2022-09-23 13:42 UTC, geraud.frappier
Details | Diff | Splinter Review
Bug 31579: Improve reserve/request.pl to avoid mistakes with pick up locations. (3.01 KB, patch)
2022-09-23 14:18 UTC, geraud.frappier
Details | Diff | Splinter Review
Bug 31579: Improve reserve/request.pl to avoid mistakes with pick up locations (6.04 KB, patch)
2022-09-26 16:17 UTC, geraud.frappier
Details | Diff | Splinter Review
Bug 31579: Improve reserve/request.pl to avoid mistakes with pick up locations (5.99 KB, patch)
2022-09-26 19:40 UTC, geraud.frappier
Details | Diff | Splinter Review
Bug 31579: Improve reserve/request.pl to avoid mistakes with pick up locations (2.07 KB, patch)
2022-09-27 14:55 UTC, geraud.frappier
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Blou 2022-09-19 16:16:02 UTC
In the place hold page reserve/request.pl, there are two sections.

One is the "Hold details" section which allow for record hold.  The other is "Place a hold on a specific item" which speaks for itself.

The pick-up location by default is the signed-in user (the employee).

But it is different from the "Allowed pick-up location" for the item which defaults to the homebranch.
The wording should be "Pick-up location", btw, the "Allowed" states there's a list, not that the user should select it.

A common tread is that the user comes in, select the pick up location in the record section, select an item *and doesn't modify the last column*, which overrule the pickup location in the notice.

We propose a patch where a modification of the first drop down in the record modifies all the items dropdown to the same value by default.
Comment 1 Katrin Fischer 2022-09-19 18:58:39 UTC
Hi Blou,

this bug description looked familiar and I am quite sure that we discussed similar recently. The problem is, that the list of allowed pick-up locations of an item is not the same as the allowed pick-up locations on record level. Actually, the list on record level might allow for much more than a single item does, so we could not change all but maybe only some items.

I think I found the bug I was thinking about:
Bug 30579 - When placing item level hold, some options that are not used are not disabled 

Maybe you'd like to weigh in there?
Comment 2 Blou 2022-09-19 20:27:46 UTC
Would never have guessed from the subject line :)

Ok, that is a redesign as a solution.  I'll have it tested here, see if it works for us (I suppose so) then comment.

This bug here will still go ahead, just in case, and as a training ground for our new rookie :)
Comment 3 geraud.frappier 2022-09-23 13:42:19 UTC
Created attachment 140907 [details] [review]
Bug 31579: Improve reserve/request.pl to avoid mistakes with pick up locations

Reported-by: Géraud Frappier <geraud.frappier@inlibro.com>

    In the place hold page reserve/request.pl, there are two sections.
    One is the "Hold details" section which allow for record hold.  The other is "Place a hold on a specific item" which speaks for itself.
    The pick-up location by default is the signed-in user (the employee).
    But it is different from the "Allowed pick-up location" for the item which defaults to the homebranch.
    The wording should be "Pick-up location", btw, the "Allowed" states there's a list, not that the user should select it.

    This patch will resolve this problem.

    To test:
    1) Go in System preferences and find the "UseBranchTransfertLimits".
    2) Change the value of "Don't enforce" to "Enforce".
    3) Come back on the menu and search a catalog.
    4) Click on "Place a hold" and enter the name of a patron.
    5) In the sections Hold details change the value of "pickup at" by choosing an other library.
    6) Observe that the web page is refreshed, but in the second section the list of "allowed pickup locations" didn't changed.
    7) Apply the patch.
    8) Redo 3-4-5-6.
    9) See that the values of the list of "Allowed pickup locations" has changed to the library you have chosen.
Comment 4 geraud.frappier 2022-09-23 14:18:35 UTC
Created attachment 140917 [details] [review]
Bug 31579: Improve reserve/request.pl to avoid mistakes with pick up locations.

    Reported-by: Géraud Frappier <geraud.frappier@inlibro.com>

    In the place hold page reserve/request.pl, there are two sections.
    One is the "Hold details" section which allow for record hold.  The other is "Place a hold on a specific item" which speaks for itself.
    The pick-up location by default is the signed-in user (the employee).
    But it is different from the "Allowed pick-up location" for the item which defaults to the homebranch.
    The wording should be "Pick-up location", btw, the "Allowed" states there's a list, not that the user should select it.

    This patch will resolve this problem.

    To test:
    1) Go in System preferences and find the "UseBranchTransfertLimits".
    2) Change the value of "Don't enforce" to "Enforce".
    3) Come back on the menu and search a catalog.
    4) Click on "Place a hold" and enter the name of a patron.
    5) In the sections Hold details change the value of "pickup at" by choosing an other library.
    6) Observe that the web page is refreshed, but in the second section the list of "allowed pickup locations" didn't changed.
    7) Apply the patch.
    8) Redo 3-4-5-6.
    9) See that the values of the list of "Allowed pickup locations" has changed to the library you have chosen.
Comment 5 geraud.frappier 2022-09-26 16:17:10 UTC
Created attachment 140997 [details] [review]
Bug 31579: Improve reserve/request.pl to avoid mistakes with pick up locations

In the place hold page reserve/request.pl, there are two sections.
    One is the "Hold details" section which allow for record hold.  The other is "Place a hold on a specific item" which speaks for itself.
    The pick-up location by default is the signed-in user (the employee).
    But it is different from the "Allowed pick-up location" for the item which defaults to the homebranch.
    The wording should be "Pick-up location", btw, the "Allowed" states there's a list, not that the user should select it.

    This patch will resolve this problem.

    To test:
    1) Go in System preferences and find the "UseBranchTransfertLimits".
    2) Change the value of "Don't enforce" to "Enforce".
    3) Come back on the menu and search a catalog.
    4) Click on "Place a hold" and enter the name of a patron.
    5) In the sections Hold details change the value of "pickup at" by choosing an other library.
    6) Observe that the web page is refreshed, but in the second section the list of "allowed pickup locations" didn't changed.
    7) Apply the patch.
    8) Redo 3-4-5-6.
    9) See that the values of the list of "Allowed pickup locations" has changed to the library you have chosen.
Comment 6 geraud.frappier 2022-09-26 19:40:52 UTC
Created attachment 141007 [details] [review]
Bug 31579: Improve reserve/request.pl to avoid mistakes with pick up locations

In the place hold page reserve/request.pl, there are two sections.
    One is the "Hold details" section which allow for record hold.  The other is "Place a hold on a specific item" which speaks for itself.
    The pick-up location by default is the signed-in user (the employee).
    But it is different from the "Allowed pick-up location" for the item which defaults to the homebranch.
    The wording should be "Pick-up location", btw, the "Allowed" states there's a list, not that the user should select it.

    This patch will resolve this problem.

    To test:
    1) Go in System preferences and find the "UseBranchTransfertLimits".
    2) Change the value of "Don't enforce" to "Enforce".
    3) Come back on the menu and search a catalog.
    4) Click on "Place a hold" and enter the name of a patron.
    5) In the sections Hold details change the value of "pickup at" by choosing an other library.
    6) Observe that the web page is refreshed, but in the second section the list of "allowed pickup locations" didn't changed.
    7) Apply the patch.
    8) Redo 3-4-5-6.
    9) See that the values of the list of "Allowed pickup locations" has changed to the library you have chosen.

Author:    Géraud Frappier  <geraud.frappier@inlibro.com>
Comment 7 Andrew Fuerste-Henry 2022-09-26 20:06:53 UTC
>     1) Go in System preferences and find the "UseBranchTransfertLimits".
>     2) Change the value of "Don't enforce" to "Enforce".

I don't understand why the behavior you've coded here only applies when UseBranchTransfertLimits is set to Enforce. These two separate dropdowns exist regardless of whether or not one is using Branch Transfer Limits. 

More importantly, this patch breaks existing behavior when I have two items on the same bib with different allowed pickup locations. To recreate:

(apply patch, set UseBranchTransfertLimits to Enforce)
1: Set Hold Pickup Library Match in circ rules to "item's hold group"
2: Create two library groups, both defined as local holds groups. Give each group a couple of libraries, making sure they don't overlap
3: Find or create a biblio record with two items. The items should be held by different libraries, one in each of your hold groups
4: try to place a hold on your bib. on reserve/request.pl, observe that you have 3 pickup location dropdown (one for the biblio, one for each item) and that all 3 show the same value.
5: confirm that changing the biblio-level dropdown forces the page to reload and changes both item dropdowns to match what you selected in the biblio dropdown
6: HOWEVER -- your items each have a distinct set of allowed pickup locations. One of your two items will be showing a branch at which it is not allowed to be picked up.
7: attempt to place the item-level hold with an illegal pickup location that koha has suggested to you. observe that Koha leaves the holds creation page without actually making a hold or giving an error message.
Comment 8 geraud.frappier 2022-09-27 14:55:10 UTC
Created attachment 141033 [details] [review]
Bug 31579: Improve reserve/request.pl to avoid mistakes with pick up locations

Remove white space and unnecessary comments.