Bug 33302 - Placing item level holds in OPAC allows to pick forbidden pick-up locations, but then places no hold
Summary: Placing item level holds in OPAC allows to pick forbidden pick-up locations, ...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on:
Blocks: 30860
  Show dependency treegraph
 
Reported: 2023-03-22 10:34 UTC by Katrin Fischer
Modified: 2023-07-24 23:57 UTC (History)
9 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:
23.05.00,22.11.06,22.05.14


Attachments
Bug 33302: Send and display errors when a hold cannot be placed on the OPAC (7.69 KB, patch)
2023-03-28 11:44 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 33302: Send and display errors when a hold cannot be placed on the OPAC (7.74 KB, patch)
2023-03-29 10:02 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 33302: Send and display errors when a hold cannot be placed on the OPAC (7.80 KB, patch)
2023-04-04 09:18 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 33302: Send and display errors when a hold cannot be placed on the OPAC (7.85 KB, patch)
2023-05-04 14:00 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33302: (QA follow-up) Rephrase errors to be more user friendly (3.90 KB, patch)
2023-05-04 14:00 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2023-03-22 10:34:53 UTC
A bit much to put in the description, but the problem is about item level holds failing to be placed silently in the OPAC.

To test:

With the sample data in ktd or sandboxes:

1) Enforce library transfer limits (UseBranchTransferLimits) on itemtypes (BranchTransferLimitsType).
2) In administration > branch transfer rules:
   disallow transfer from Midway to any other library for item type BK (Books)
3) Search for a record with at least one item in Midway in staff
4) Try to place an item level hold on a Midway item.
   I used: Intermediate Perl, biblionumber 126
   For the Midway item, only Midway will show as possible pickup-location
   This is correct.
5) Switch to the OPAC
6) Try to place an item level hold there on the Midway item, for pick-up at  
   Centerville.
7) Place a hold on the Midway item for pick-up at any non-Midway location
8) Verify the page saves without error, but the hold was not placed.

-
The basic issue here is that item specific pickup locations != record-specific pickup locations.

In the staff interface we have a separate pull down "allowed pick-up locations" for each item in the list. I think we should implement the same in the OPAC to help the users see what is allowed and what not. And deacticate the pull down on top when an item is selected.

But: At least we should give proper feedback that a hold could not be placed because of the pick-up location instead of failing silently!
Comment 1 Nick Clemens 2023-03-28 11:44:24 UTC
Created attachment 148813 [details] [review]
Bug 33302: Send and display errors when a hold cannot be placed on the OPAC

This patch changes opac-reserve.pl to return the error(s) when placing
a hold as a pipe delimited list which is then translated to a message for the
user

To test:
1 - Find a record with items available on the opac
2 - Click 'place hold' and set things up, but do not confirm
3 - In staff client, do something to make hold invalid:
    - Make item damaged
    - Make library not a pickup location
    - Place other holds for patron up to limit
    - etc.
4 - Confirm hold on OPAC
5 - You are sent to patron's account, hold is not placed
6 - There is little or no message to explain why
7 - Apply patch
8 - Repeat
9 - Now errors are clear
Comment 2 Biblibre Sandboxes 2023-03-29 10:02:17 UTC
Created attachment 148896 [details] [review]
Bug 33302: Send and display errors when a hold cannot be placed on the OPAC

This patch changes opac-reserve.pl to return the error(s) when placing
a hold as a pipe delimited list which is then translated to a message for the
user

To test:
1 - Find a record with items available on the opac
2 - Click 'place hold' and set things up, but do not confirm
3 - In staff client, do something to make hold invalid:
    - Make item damaged
    - Make library not a pickup location
    - Place other holds for patron up to limit
    - etc.
4 - Confirm hold on OPAC
5 - You are sent to patron's account, hold is not placed
6 - There is little or no message to explain why
7 - Apply patch
8 - Repeat
9 - Now errors are clear

Signed-off-by: Andrew Auld <andrew.auld@ptfs-europe.com>
Comment 3 ByWater Sandboxes 2023-04-04 09:18:59 UTC
Created attachment 149123 [details] [review]
Bug 33302: Send and display errors when a hold cannot be placed on the OPAC

This patch changes opac-reserve.pl to return the error(s) when placing
a hold as a pipe delimited list which is then translated to a message for the
user

To test:
1 - Find a record with items available on the opac
2 - Click 'place hold' and set things up, but do not confirm
3 - In staff client, do something to make hold invalid:
    - Make item damaged
    - Make library not a pickup location
    - Place other holds for patron up to limit
    - etc.
4 - Confirm hold on OPAC
5 - You are sent to patron's account, hold is not placed
6 - There is little or no message to explain why
7 - Apply patch
8 - Repeat
9 - Now errors are clear

Signed-off-by: Andrew Auld <andrew.auld@ptfs-europe.com>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Comment 4 Jonathan Druart 2023-04-04 10:10:22 UTC
Nick, this will conflict with bug 30860.
Comment 5 Katrin Fischer 2023-04-04 11:41:47 UTC
This will need backporting - should it go in first?
Comment 6 Nick Clemens 2023-04-04 12:27:41 UTC
(In reply to Katrin Fischer from comment #5)
> This will need backporting - should it go in first?

Yes, I made it block the other
Comment 7 Katrin Fischer 2023-05-04 14:00:46 UTC
Created attachment 150667 [details] [review]
Bug 33302: Send and display errors when a hold cannot be placed on the OPAC

This patch changes opac-reserve.pl to return the error(s) when placing
a hold as a pipe delimited list which is then translated to a message for the
user

To test:
1 - Find a record with items available on the opac
2 - Click 'place hold' and set things up, but do not confirm
3 - In staff client, do something to make hold invalid:
    - Make item damaged
    - Make library not a pickup location
    - Place other holds for patron up to limit
    - etc.
4 - Confirm hold on OPAC
5 - You are sent to patron's account, hold is not placed
6 - There is little or no message to explain why
7 - Apply patch
8 - Repeat
9 - Now errors are clear

Signed-off-by: Andrew Auld <andrew.auld@ptfs-europe.com>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 8 Katrin Fischer 2023-05-04 14:00:48 UTC
Created attachment 150668 [details] [review]
Bug 33302: (QA follow-up) Rephrase errors to be more user friendly

These error message will show within the patron account. The goal
of this patch is to make them a little more user friendly avoiding
'patron' and 'hold group' terminology :)

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 9 Katrin Fischer 2023-05-04 14:01:26 UTC
I tried to make the error messages a little more user friendly - please feel free to drop the patch if you don't like it!
Comment 10 Tomás Cohen Arazi 2023-05-09 14:48:08 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 11 Matt Blenkinsop 2023-05-15 12:10:09 UTC
Nice work everyone!

Pushed to stable for 22.11.x
Comment 12 Lucas Gass 2023-06-15 16:57:05 UTC
Backported to 22.05.x for upcoming 22.05.14
Comment 13 Danyon Sewell 2023-07-24 23:57:31 UTC
Not backporting for 21.11.x