Bug 7398 - Show number of holds on a title in the OPAC details
Summary: Show number of holds on a title in the OPAC details
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: 3.10
Hardware: All All
: P5 - low enhancement (vote)
Target Milestone: ---
Assignee: Srdjan Jankovic
QA Contact: Ian Walls
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-02 21:30 UTC by Ian Walls
Modified: 2014-11-20 15:34 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
patch (6.40 KB, patch)
2012-03-12 02:35 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Screenshot showing total holds count versus item hold count (16.84 KB, image/png)
2012-03-12 13:11 UTC, Owen Leonard
Details
screenshot of number of holds when only holds on next available (89.67 KB, image/png)
2012-03-21 15:14 UTC, jmbroust
Details
screenshot 2 of number of holds when only holds on next available (57.96 KB, image/png)
2012-03-21 15:47 UTC, jmbroust
Details
follow-up patch (7.12 KB, patch)
2012-03-23 06:03 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
patch (12.85 KB, patch)
2012-03-26 01:52 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
patch (17.74 KB, patch)
2012-03-30 00:07 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
bug_7398: Replaced OPACDisplayRequestPriority syspref with OPACShowHoldQueueDetails (17.88 KB, patch)
2012-03-31 21:10 UTC, Chris Cormack
Details | Diff | Splinter Review
screenshot (25.14 KB, image/png)
2012-04-08 09:12 UTC, Katrin Fischer
Details
patch (16.85 KB, patch)
2012-04-19 04:37 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
bug_7398: Replaced OPACDisplayRequestPriority syspref with OPACShowHoldQueueDetails (16.93 KB, patch)
2012-05-07 16:40 UTC, Kyle M Hall
Details | Diff | Splinter Review
patch (16.88 KB, patch)
2012-05-23 01:10 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
release notes updated (1.33 MB, application/pdf)
2012-12-14 16:23 UTC, Paul Poulain
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ian Walls 2012-01-02 21:30:43 UTC
Like on the staff client, it should be possible to show the number of holds currently on a title.  This will allow patrons to know how long a wait is going to be without having to log into the OPAC.

Work to be done:

* Add a new system preference, OPACShowHoldsCount, that will control whether the total number of holds shown on the OPAC detail page or not.  Summon this syspref on the opac-detail page, and pass to the template

* Add call to GetReservesfrombiblionumber.  Take the holdcount return variable, and pass it to the template.

* Edit opac-detail.tt to check for OPACShowHoldsCount, and if it's true, display the number of holds in a div similar to the one used staff-side

* Edit the opac.css to properly and aesthetically display this new div
Comment 1 Srdjan Jankovic 2012-03-12 02:35:56 UTC Comment hidden (obsolete)
Comment 2 Owen Leonard 2012-03-12 13:10:16 UTC
I wonder if the holds total should be shown at the top of the holdings list instead of the bottom? It would be hard to spot on a title with lots of items.

I also wonder if the holds column in the holdings table should say "Holds on this item." That's a long label, but I'm concerned that it is confusing to patrons to see one number for total holds and another number in the table. For instance, 4 holds total, but only one line in the table says Holds: 1 because only one is an item-level hold?
Comment 3 Owen Leonard 2012-03-12 13:11:24 UTC
Created attachment 8181 [details]
Screenshot showing total holds count versus item hold count
Comment 4 jmbroust 2012-03-21 15:14:01 UTC Comment hidden (obsolete)
Comment 5 jmbroust 2012-03-21 15:18:58 UTC
works fine, nice feature :-) ; maybe number of holds could be more seeable when you only have holds on next available (because the table is empty in this case, see 2nd screenshot) ; and maybe it could be titled "total of holds" instead of "holds"
Comment 6 jmbroust 2012-03-21 15:47:01 UTC
Created attachment 8441 [details]
screenshot 2 of number of holds when only holds on next available
Comment 7 Paul Poulain 2012-03-22 10:44:17 UTC
QA comment
* the code is OK
* the syspref is inconsistent I think: if it's set to OFF, the patron can see how many holds are done when he place the hold (opac-reserve.pl?biblionumber=5), in the priority column. So, from 2 choose 1:
 - remove the syspref
 - make the display consistent

I'm member of the "group for removal of useless sysprefs", but I think this one is usefull, so i'm for option 2.

Please provide a follow-up and i'll be happy to pass QA & push this patch !
Comment 8 Srdjan Jankovic 2012-03-23 06:03:41 UTC Comment hidden (obsolete)
Comment 9 Paul Poulain 2012-03-23 10:30:40 UTC
Just tested this feature, and found a problem: there are 2 sysprefs related to displaying the hold counter on OPAC:
OPACShowHoldsCount and OPACDisplayRequestPriority

* 1st of all, it would be better to be consistent on names
* 2nd = if you set OPACDisplayRequestPriority=1 and OPACShowHoldsCount=0 the display is broken (you get the <th> but not the <td>, so the table is wrong (I agree it's a strange idea though)
* 3rd = overall, i'm wondering if we really need 2 sysprefs, that does almost the same thing. Is there a use case where we want to display the number of requests but not your priority ? I feel it's meaningless = when you put your request, if you know there are already 5 holds, you'll be the 6th one. So shouldn't we just have one ? (and, maybe, remove OPACDisplayRequestPriority)

keeping failed QA because of #2
If you agree with #3, #1 becomes useless
If you provide a patch for #3 i'll be pleased, otherwise, i'll be OK to push this patch without it and open an entry for removal of OPACDisplayRequestPriority. ( But, of course, I prefer the patch ;-) )
Comment 10 Melia Meggs 2012-03-23 17:41:20 UTC
> * 3rd = overall, i'm wondering if we really need 2 sysprefs, that does
> almost the same thing. Is there a use case where we want to display the
> number of requests but not your priority ? I feel it's meaningless = when
> you put your request, if you know there are already 5 holds, you'll be the
> 6th one. So shouldn't we just have one ? (and, maybe, remove
> OPACDisplayRequestPriority)

I think the two sys prefs are doing slightly different things.  
OPACShowHoldsCount shows the total # of holds to everyone, logged in or not.
OPACDisplayRequestPriority shows the priority of your hold request, when you're logged in.

I think the difference is when a patron does a search in the OPAC, even not logged in he can see how many holds are already on a particular item.  If there are already 50 holds, he may decide he doesn't want to go to the effort of logging in and placing a hold.  If all the patron can see is his priority, then he wouldn't know there are 50 holds until after he has already logged in and placed his hold, which he would then need to cancel if he doesn't want to wait in line behind 50 other people.
Comment 11 Ian Walls 2012-03-23 17:55:52 UTC
Melia's right, both prefs are necessary:

OPACShowHoldsCount shows how long the queue is.  No login required.

OPACDisplayRequestPriority shows where you are in the queue, once logged in.

When OPACShowHoldsCount shows X, and you do not yet have a hold on the material, you can infer your priority at that moment will be X+1.  Once you have a hold, however, you won't be able to see how close you are to getting it filled unless OPACDisplayRequestPriority is on
Comment 12 Owen Leonard 2012-03-23 21:17:53 UTC
The last two comments seem to come from the assumption that something is being taken away, and I don't see it that way. I think the question is, is there a situation where you would want to show one but not the other? Show total holds count but not the user's priority? Show the user's priority but not the overall hold count? Permissions for both could be combined into a "OPACShowHoldQueueDetails" or something like that.
Comment 13 Srdjan Jankovic 2012-03-26 01:52:38 UTC Comment hidden (obsolete)
Comment 14 Paul Poulain 2012-03-27 09:10:45 UTC
(In reply to comment #12)
> The last two comments seem to come from the assumption that something is
> being taken away, and I don't see it that way. I think the question is, is
> there a situation where you would want to show one but not the other? Show
> total holds count but not the user's priority? Show the user's priority but
> not the overall hold count? Permissions for both could be combined into a
> "OPACShowHoldQueueDetails" or something like that.

that's exactly what I wanted to say !
Comment 15 Srdjan Jankovic 2012-03-28 00:08:59 UTC
That's fine with me. Do we have consensus re options?
Comment 16 Melia Meggs 2012-03-28 01:04:36 UTC
Srdjan, for the three options, I think what you have proposed is good:
1) none 
2) number of holds only
3) both number of holds and priority

I just need to make sure that if you choose 2) that the patron will be able to see the number of holds regardless of whether he is logged in or not.  The main concern of the library that is sponsoring this is showing patrons the # of holds on an item when the patron is NOT logged in.

I understand that with 3) obviously the patron can only see his priority if logged in.
Comment 17 Srdjan Jankovic 2012-03-28 02:11:09 UTC
I will check, but I assume 3) must be correct, cause no login - no user, therefore no priority.
Comment 18 Srdjan Jankovic 2012-03-30 00:07:28 UTC Comment hidden (obsolete)
Comment 19 Nicole C. Engard 2012-03-30 17:40:28 UTC
patch
Apply? [yn] y

Applying: bug_7398: Replaced OPACDisplayRequestPriority syspref with OPACShowHoldQueueDetails
/home/nengard/kohaclone/.git/rebase-apply/patch:230: trailing whitespace.
            $_->{itemnumber} 
error: patch failed: installer/data/mysql/updatedatabase.pl:5109
error: installer/data/mysql/updatedatabase.pl: patch does not apply
Patch failed at 0001 bug_7398: Replaced OPACDisplayRequestPriority syspref with OPACShowHoldQueueDetails
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".
Patch left in /tmp/patch-q5mYrL.patch
nengard@kohavm:~/kohaclone$
Comment 20 Chris Cormack 2012-03-31 21:10:53 UTC Comment hidden (obsolete)
Comment 21 Chris Cormack 2012-03-31 21:11:51 UTC
Just an updatedatabase conflict, fixed.
Comment 22 Katrin Fischer 2012-04-08 09:12:03 UTC
Created attachment 9008 [details]
screenshot

I tested this and I see some problems:

1) There is no setting to keep current behaviour "only show priority" is not an available setting. We have no information, number of holds and number of holds and priority. I can imagine some libraries want the information about priority only to show to their patrons, not to everyone. So keeping current behaviour seems important to me.

2) I think the display is confusing if you have item level and title level holds. I am attaching a screenshot. Perhaps it would help to make the descriptions a bit more clear. The column could be named something like "specific item holds"? (Item level holds, holds on this item?) Currently it only says holds. The number at the bottom of the table could pehraps be "Total holds"?
Comment 23 Srdjan Jankovic 2012-04-17 03:50:37 UTC
> 1) There is no setting to keep current behaviour "only show priority" is not
> an available setting.

My thinking was that there's no reason to have "only show priority", ie if we want o show priority we may as well show the queue size. But I may be wrong. I will ad that one if you wish.
 
> 2) The column could be named something like
> "specific item holds"? (Item level holds, holds on this item?) Currently it
> only says holds. The number at the bottom of the table could pehraps be
> "Total holds"?

We can do that.

Please respond to 1) so I can do (or not) both things at once.
Comment 24 Katrin Fischer 2012-04-17 05:36:05 UTC
Hi Srdjan, 
yes, please also make the current behaviour a valid option. In my opionino it should also be the default when updating existing installations.
Comment 25 Srdjan Jankovic 2012-04-19 04:37:20 UTC Comment hidden (obsolete)
Comment 26 Kyle M Hall 2012-05-07 16:40:15 UTC Comment hidden (obsolete)
Comment 27 Paul Poulain 2012-05-22 09:57:03 UTC
QA comment:

trying to test this patch, I get a nasty =
Template process failed: file error - parse error - /home/paul/koha.dev/koha-community/koha-tmpl/opac-tmpl/prog/en/modules/opac-reserve.tt line 314-315: unexpected token (])
  [% END ]
                        [% IF showholds && showpriority %] at /home/paul/koha.dev/koha-community/C4/Templates.pm line 127.

( My tests were done with the syspref set to holds_priority )

Could you please fix (probably a missing %) and resubmit ?
Comment 28 Srdjan Jankovic 2012-05-23 01:10:45 UTC
Created attachment 9733 [details] [review]
patch

Sorry about that. It happens to me sometimes when I open files remotely. I try to exit file with :x, and somehow that : disappears, whichleaves only x and that deletes the character. Should be more careful.
Comment 29 Paul Poulain 2012-05-24 13:09:11 UTC
(In reply to comment #28)
> Sorry about that. It happens to me sometimes when I open files remotely. I
> try to exit file with :x, and somehow that : disappears, whichleaves only x
> and that deletes the character. Should be more careful.

<troll mode ON>
You should first use a tool that is an efficient one, like Emacs
<troll mode OFF>

PS : just kidding, most of developers use VIM at BibLibre ;-)
Comment 30 Paul Poulain 2012-05-24 13:45:33 UTC
QA comments:
 * This patch clean some code indenting or writting, gg !

 * perlcritic was complaining before the patch, still complains after the patch, not a reason to failed QA.

 * There are some additions like:
+$template->param( showholds=>$show_holds_count);
+$template->param( showpriority=>$show_priority);
that could more cleanly be written 
+$template->param( showholds=>$show_holds_count,
                   showpriority=>$show_priority
                 );

but that's a minor comment, passed QA
Comment 31 Kyle M Hall 2012-05-24 15:30:20 UTC
I always feel like the odd man out. I use joe set to wordstar style as my primary editor.

(In reply to comment #29)
> (In reply to comment #28)
> > Sorry about that. It happens to me sometimes when I open files remotely. I
> > try to exit file with :x, and somehow that : disappears, whichleaves only x
> > and that deletes the character. Should be more careful.
> 
> <troll mode ON>
> You should first use a tool that is an efficient one, like Emacs
> <troll mode OFF>
> 
> PS : just kidding, most of developers use VIM at BibLibre ;-)
Comment 32 Chris Cormack 2012-05-24 23:26:20 UTC
(In reply to comment #31)
> I always feel like the odd man out. I use joe set to wordstar style as my
> primary editor.
> 
I just write mine in my own blood, with a toothpick :)
Comment 33 Srdjan Jankovic 2012-05-24 23:28:46 UTC
How do you sterilise a toothpick?
Comment 34 Jared Camins-Esakov 2012-05-24 23:29:57 UTC
(In reply to comment #33)
> How do you sterilise a toothpick?

I was always told that a real programmer doesn't need to sterilize the toothpick. There's so much cola in their veins nothing can harm them.
Comment 35 Nicole C. Engard 2012-12-03 16:06:28 UTC
I am looking at both master and 3.10 and I don't see this new system preference ... was there a problem with this patch and upgrades?
Comment 36 Melia Meggs 2012-12-03 17:19:07 UTC
Nicole, it is OPACShowHoldQueueDetails.  You'll see 4 choices there now.
Comment 37 Nicole C. Engard 2012-12-03 17:59:12 UTC
Yes, the release notes need to be updated
Comment 38 Paul Poulain 2012-12-14 16:23:19 UTC
Created attachment 14138 [details]
release notes updated

Hi Nicole,

Here is the Koha 3.10 enhancements document fixed. I don't know/see how to update koha-community.org website. Do I have to re-upload the pdf & update the link & remove the previous file ? isn't there a versioning management?
Comment 39 Nicole C. Engard 2012-12-14 21:49:01 UTC
For the PDF you do have to re-upload and change the link - for the post itself you just click 'edit' and edit the text of the post and it does keep track of revisions.

Nicole