Bug 15533 - Allow patrons and librarians to select itemtype when placing hold
Summary: Allow patrons and librarians to select itemtype when placing hold
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Keywords:
: 17619 (view as bug list)
Depends on: 15532
Blocks: 16213
  Show dependency treegraph
 
Reported: 2016-01-09 11:10 UTC by Kyle M Hall
Modified: 2017-09-15 15:08 UTC (History)
9 users (show)

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


Attachments
Bug 15533 - Allow patrons and librarians to select itemtype when placing hold (36.52 KB, patch)
2016-01-09 11:14 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15533 - Allow patrons and librarians to select itemtype when placing hold (36.54 KB, patch)
2016-02-21 12:18 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15533 - Allow patrons and librarians to select itemtype when placing hold (36.61 KB, patch)
2016-03-16 13:59 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15533 - Allow patrons and librarians to select itemtype when placing hold (36.78 KB, patch)
2016-03-23 21:44 UTC, Andreas Hedström Mace
Details | Diff | Splinter Review
Bug 15533 - Allow patrons and librarians to select itemtype when placing hold (36.84 KB, patch)
2016-04-07 09:53 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 15533: Fix 'ambiguous' SQL query in CanItemBeReserved (1.17 KB, patch)
2016-04-12 13:23 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15533 - Allow patrons and librarians to select itemtype when placing hold (31.86 KB, patch)
2016-04-28 14:56 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15533: Fix 'ambiguous' SQL query in CanItemBeReserved (1.17 KB, patch)
2016-04-28 14:57 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15533 - Allow patrons and librarians to select itemtype when placing hold (32.83 KB, patch)
2016-04-28 15:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15533: Fix 'ambiguous' SQL query in CanItemBeReserved (1.17 KB, patch)
2016-04-28 15:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15533 - Allow patrons and librarians to select itemtype when placing hold (36.87 KB, patch)
2016-04-28 15:35 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15533: Fix 'ambiguous' SQL query in CanItemBeReserved (1.17 KB, patch)
2016-04-28 15:35 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15533 [QA Followup] - Add a system preference (7.46 KB, patch)
2016-04-28 15:35 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15533 [QA Followup] - Move tags for translatability (1.08 KB, patch)
2016-04-28 15:35 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15533 [QA Followup] - Itemtype limit missing from tables (2.70 KB, patch)
2016-04-28 18:20 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15533 [QA Followup] - All itemtypes for all items showing in OPAC multi-hold (4.16 KB, patch)
2016-04-28 18:55 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15533 [QA Followup] - All itemtypes for all items showing in OPAC multi-hold (5.04 KB, patch)
2016-04-28 18:57 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 15533 - Allow patrons and librarians to select itemtype when placing hold (36.94 KB, patch)
2016-04-28 19:02 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 15533: Fix 'ambiguous' SQL query in CanItemBeReserved (1.24 KB, patch)
2016-04-28 19:03 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 15533 [QA Followup] - Add a system preference (7.53 KB, patch)
2016-04-28 19:03 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 15533 [QA Followup] - Move tags for translatability (1.15 KB, patch)
2016-04-28 19:03 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 15533 [QA Followup] - Itemtype limit missing from tables (2.77 KB, patch)
2016-04-28 19:03 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 15533: QA follow-up - Formatting and improving translatability (2.65 KB, patch)
2016-04-28 19:03 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 15533 [QA Followup] - All itemtypes for all items showing in OPAC multi-hold (5.11 KB, patch)
2016-04-28 19:03 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 Kyle M Hall 2016-01-09 11:10:29 UTC
Some libraries would like the ability to select the itemtype to request when placing holds. For example, if a record has 3 copies of BookA and 3 copies of BookA in large print, this feature would allow a person to place a hold on the record, but still be able to target only the Large Print edition so that the first Large Print copy that becomes available is targeted, rather than forcing the patron to select a particular copy to hold.
Comment 1 Kyle M Hall 2016-01-09 11:14:26 UTC Comment hidden (obsolete)
Comment 2 Frédéric Demians 2016-01-15 17:32:32 UTC
I can't apply this patch on master. I get:

fatal: sha1 information is lacking or useless (C4/HoldsQueue.pm).
error: could not build fake ancestor
Comment 3 Héctor Eduardo Castro Avalos 2016-01-16 00:25:47 UTC
(In reply to Frédéric Demians from comment #2)
> I can't apply this patch on master. I get:
> 
> fatal: sha1 information is lacking or useless (C4/HoldsQueue.pm).
> error: could not build fake ancestor

Applying: Bug 15533 - Allow patrons and librarians to select itemtype when placing hold
fatal: sha1 information is lacking or useless (C4/HoldsQueue.pm).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 15533 - Allow patrons and librarians to select itemtype when placing hold
The copy of the patch that failed is found in:
   /var/koha/koha/.git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-15533---Allow-patrons-and-librarians-to-select-UnO2iN.patch
Comment 4 Kyle M Hall 2016-01-16 12:39:52 UTC
I'm having no trouble applying the patch. Did you apply it's dependency ( Bug 15532 ) first?
Comment 5 Héctor Eduardo Castro Avalos 2016-01-17 01:55:38 UTC
(In reply to Kyle M Hall from comment #4)
> I'm having no trouble applying the patch. Did you apply it's dependency (
> Bug 15532 ) first?

You're right, patch apply without trouble
Comment 6 Kyle M Hall 2016-02-21 12:18:25 UTC Comment hidden (obsolete)
Comment 7 Kyle M Hall 2016-03-16 13:59:48 UTC Comment hidden (obsolete)
Comment 8 Andreas Hedström Mace 2016-03-23 21:44:33 UTC Comment hidden (obsolete)
Comment 9 Benjamin Rokseth 2016-04-07 09:53:36 UTC Comment hidden (obsolete)
Comment 10 Julian Maurice 2016-04-12 13:23:02 UTC Comment hidden (obsolete)
Comment 11 Kyle M Hall 2016-04-28 14:56:56 UTC Comment hidden (obsolete)
Comment 12 Kyle M Hall 2016-04-28 14:57:03 UTC Comment hidden (obsolete)
Comment 13 Kyle M Hall 2016-04-28 15:00:18 UTC Comment hidden (obsolete)
Comment 14 Kyle M Hall 2016-04-28 15:00:32 UTC Comment hidden (obsolete)
Comment 15 Katrin Fischer 2016-04-28 15:29:02 UTC
Ok, starting with code review and found a few things:

1) Please make this feature optional! I think not all libraries will want to use it, while it's very useful for others. I'd suggest to add a preference here, so it can't be circumvented like CSS would allow to.

2) Some smallish translation issues, easy to fix:
<i>Next available <b>[% ItemTypes.GetDescription( reserveloo.itemtype ) %]</b> item</i> 
Pending for next available <i>[% ItemTypes.GetDescription( RESERVE.itemtype ) %]</i> item

Tests and QA script pass.

I will continue testing a bit more!
Comment 16 Kyle M Hall 2016-04-28 15:35:20 UTC Comment hidden (obsolete)
Comment 17 Kyle M Hall 2016-04-28 15:35:27 UTC Comment hidden (obsolete)
Comment 18 Kyle M Hall 2016-04-28 15:35:30 UTC Comment hidden (obsolete)
Comment 19 Kyle M Hall 2016-04-28 15:35:33 UTC Comment hidden (obsolete)
Comment 20 Katrin Fischer 2016-04-28 17:00:38 UTC
Found a problem:

When placing multiple holds at once from the result list, the itemtype list is not correct - It seems to show the itemtypes from the first record. This allows you to place 'unfillable' holds - as there are no items of this itemtype on the other records necessarily.
Comment 21 Katrin Fischer 2016-04-28 17:19:25 UTC
Another thing: The requested itemtype is perfectly visible in the patron account in the OPAC, but not on the holds summary page of the record nor in the patron account in staff (Checkouts and detail tabs)
Comment 22 Katrin Fischer 2016-04-28 17:24:57 UTC
On a positive note: holds are not triggered for the wrong itemtype, but are triggered for the correct one. Setting holds with the pref off still works as expected.

Kyle, can you please take a look at the last 2 comments?
Comment 23 Kyle M Hall 2016-04-28 18:20:41 UTC Comment hidden (obsolete)
Comment 24 Kyle M Hall 2016-04-28 18:55:26 UTC Comment hidden (obsolete)
Comment 25 Kyle M Hall 2016-04-28 18:57:15 UTC Comment hidden (obsolete)
Comment 26 Katrin Fischer 2016-04-28 19:02:59 UTC
Created attachment 50943 [details] [review]
[PASSED QA] Bug 15533 - Allow patrons and librarians to select itemtype when placing hold

Some libraries would like the ability to select the itemtype to request
when placing holds. For example, if a record has 3 copies of BookA and 3
copies of BookA in large print, this feature would allow a person to
place a hold on the record, but still be able to target only the Large
Print edition so that the first Large Print copy that becomes available
is targeted, rather than forcing the patron to select a particular copy
to hold.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Create a record with items of two or more itemtypes
4) Place a record level hold on the record while choosing one particular
   itemtype
5) Check in an item from the record that is not of that itemtype
6) Notee it is not trapped for the hold
7) Check in an item from the record that does match the selected itemtype
8) Note the item is trapped for the hold

Signed-off-by: Andreas Hedström Mace <andreas.hedstrom.mace@sub.su.se>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 27 Katrin Fischer 2016-04-28 19:03:03 UTC
Created attachment 50944 [details] [review]
[PASSED QA] Bug 15533: Fix 'ambiguous' SQL query in CanItemBeReserved

Adding reserves.itemtype made a query to fail in CanItemBeReserved.
This patch prefixes the column names with the corresponding table names
to prevent that.

You can trigger the error by running
  prove t/db_dependent/Holds.t

You should see something like this:
  Column 'itemtype' in where clause is ambiguous

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 28 Katrin Fischer 2016-04-28 19:03:06 UTC
Created attachment 50945 [details] [review]
[PASSED QA] Bug 15533 [QA Followup] - Add a system preference

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 29 Katrin Fischer 2016-04-28 19:03:10 UTC
Created attachment 50946 [details] [review]
[PASSED QA] Bug 15533 [QA Followup] - Move tags for translatability

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 30 Katrin Fischer 2016-04-28 19:03:13 UTC
Created attachment 50947 [details] [review]
[PASSED QA] Bug 15533 [QA Followup] - Itemtype limit missing from tables

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 31 Katrin Fischer 2016-04-28 19:03:17 UTC
Created attachment 50948 [details] [review]
Bug 15533: QA follow-up - Formatting and improving translatability

- Makes new message in the holds tables in the patron account
  translatable
- Unbolds the note in the holds table for the record
Comment 32 Katrin Fischer 2016-04-28 19:03:20 UTC
Created attachment 50949 [details] [review]
[PASSED QA] Bug 15533 [QA Followup] - All itemtypes for all items showing in OPAC multi-hold

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 33 Kyle M Hall 2016-04-29 10:34:13 UTC
Pushed to Master - should be in the May 2016 - Release. Thanks Katrin!
Comment 34 Josef Moravec 2016-11-13 09:51:49 UTC
*** Bug 17619 has been marked as a duplicate of this bug. ***
Comment 35 Christopher Brannon 2016-11-14 15:29:06 UTC
What version is this pushed to?
Comment 36 Katrin Fischer 2016-11-15 06:13:01 UTC
(In reply to Kyle M Hall from comment #33)
> Pushed to Master - should be in the May 2016 - Release. Thanks Katrin!

16.05