Summary: | Streamline hold error messages in staff interface | ||
---|---|---|---|
Product: | Koha | Reporter: | Kyle M Hall (khall) <kyle> |
Component: | Hold requests | Assignee: | Kyle M Hall (khall) <kyle> |
Status: | RESOLVED DUPLICATE | QA Contact: | Testopia <testopia> |
Severity: | minor | ||
Priority: | P5 - low | CC: | gmcharlt, nblake, nick |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | --- | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: | Circulation function: | ||
Attachments: |
Bug 10912 - Streamline hold error messages in staff interface
Bug 10912 - Streamline hold error messages in staff interface [SIGNED OFF] Bug 10912 - Streamline hold error messages in staff interface Bug 10912 - Streamline hold error messages in staff interface Bug 10912 - Streamline hold error messages in staff interface |
Description
Kyle M Hall (khall)
2013-09-18 20:16:38 UTC
Created attachment 21215 [details] [review] Bug 10912 - Streamline hold error messages in staff interface Right now, if a patron cannot place a hold on any items on a record, the error in the staff intranet looks like this: Cannot place hold Too Many Holds: Kyle Hall has too many holds. No copies are available to be placed on hold This can confuse some librarians, because the reason the patron cannot place the hold is not because the patron has too many holds, but only because there are no copies available. Test Plan: 1) Attempt to place a hold on a record where none of the items are holdable. 2) Note the error messages 3) Apply this patch 4) Repeat step 1 5) Note only the correct error message displays I can't reproduce this error unless I manually recreate the old maxreserves system preference and populate it with a small number. Without it I can trigger the "no copies available" but not the "too many" error. If the patron does have too many holds, and the item really does have no copies available, why shouldn't both error messages display? Hello, We were the source of this bug fix and in our estimation is solves the problem have been experiencing. This patch seems good to us. Nora Blake MassCat Could not apply patch on sandbox: The sandbox you've requested is not ready. Some problems occurred applying patches from bug 10912: <h1>Something went wrong !</h1>Applying: Bug 10912 - Streamline hold error messages in staff interface Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt Failed to merge in the changes. Patch failed at 0001 Bug 10912 - Streamline hold error messages in staff interface 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. Bug 10912 - Streamline hold error messages in staff interface 21215 - Bug 10912 - Streamline hold error messages in staff interface Apply? [(y)es, (n)o, (i)nteractive] Patch left in /tmp/Bug-10912---Streamline-hold-error-messages-in-staf-Psre_j.patch . Created attachment 29332 [details] [review] Bug 10912 - Streamline hold error messages in staff interface Right now, if a patron cannot place a hold on any items on a record, the error in the staff intranet looks like this: Cannot place hold Too Many Holds: Kyle Hall has too many holds. No copies are available to be placed on hold This can confuse some librarians, because the reason the patron cannot place the hold is not because the patron has too many holds, but only because there are no copies available. Test Plan: 1) Attempt to place a hold on a record where none of the items are holdable. 2) Note the error messages 3) Apply this patch 4) Repeat step 1 5) Note only the correct error message displays Created attachment 29342 [details] [review] [SIGNED OFF] Bug 10912 - Streamline hold error messages in staff interface Right now, if a patron cannot place a hold on any items on a record, the error in the staff intranet looks like this: Cannot place hold Too Many Holds: Kyle Hall has too many holds. No copies are available to be placed on hold This can confuse some librarians, because the reason the patron cannot place the hold is not because the patron has too many holds, but only because there are no copies available. Test Plan: 1) Attempt to place a hold on a record where none of the items are holdable. 2) Note the error messages 3) Apply this patch 4) Repeat step 1 5) Note only the correct error message displays Signed-off-by: Nick Clemens <nick@quecheelibrary.org> I am not sure about this patch - if there are multiple reasons why a hold is not possible, should we not display all of them instead of giving one reason preference of another? It seems like this patch will make the error message less accurate (elsif instead of if)? + [% ELSIF ( maxreserves ) %] + <li><strong>Too many holds: </strong> <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber %]">[% borrowerfirstname %] [% borrowersurname %] </a> has too many holds.</li> + [% ELSIF ( alreadyreserved ) %] + <li><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber %]">[% borrowerfirstname %] [% borrowersurname %]</a> <strong>already has a hold</strong> on this item </li> + [% ELSIF ( alreadypossession ) %] I suspect the error we are seeing is from this check in request.pl: unless ( CanBookBeReserved($borrowerinfo->{borrowernumber}, $biblionumber) ) { $maxreserves = 1; } It isn't checking against maxreserves or the number of reserves a patron has, just setting the maxreserves error flag when none of the items can be held. The issue is that the extra message is incorrect! However, I think maybe my first patch was too aggressive. I'm going to file a new patch that only affects the "Too many holds" and "No items are available" sections. (In reply to Katrin Fischer from comment #7) > I am not sure about this patch - if there are multiple reasons why a hold is > not possible, should we not display all of them instead of giving one reason > preference of another? It seems like this patch will make the error message > less accurate (elsif instead of if)? > > + [% ELSIF ( maxreserves ) %] > + <li><strong>Too many holds: </strong> <a > href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber > %]">[% borrowerfirstname %] [% borrowersurname %] </a> has too many > holds.</li> > + [% ELSIF ( alreadyreserved ) %] > + <li><a > href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber > %]">[% borrowerfirstname %] [% borrowersurname %]</a> <strong>already has a > hold</strong> on this item </li> > + [% ELSIF ( alreadypossession ) %] Created attachment 29435 [details] [review] Bug 10912 - Streamline hold error messages in staff interface Right now, if a patron cannot place a hold on any items on a record, the error in the staff intranet looks like this: Cannot place hold Too Many Holds: Kyle Hall has too many holds. No copies are available to be placed on hold This can confuse some librarians, because the reason the patron cannot place the hold is not because the patron has too many holds, but only because there are no copies available. Test Plan: 1) Apply this patch 2) Set the number of holds allowed to 1 3) Disable AllowOnShelfHolds 4) Place a hold for a patron 5) Attempt to place a second hold for the patron 6) You should see the "Too many holds" message 7) Set the number of holds allowed to 2 8) Repeat 5, you should be allowed to place the hold, but don't 9) Attempt to place a hold on a record with only checked in items 10) You should get a "No items are available" message Created attachment 29436 [details] [review] Bug 10912 - Streamline hold error messages in staff interface Right now, if a patron cannot place a hold on any items on a record, the error in the staff intranet looks like this: Cannot place hold Too Many Holds: Kyle Hall has too many holds. No copies are available to be placed on hold This can confuse some librarians, because the reason the patron cannot place the hold is not because the patron has too many holds, but only because there are no copies available. Test Plan: 1) Apply this patch 2) Set the number of holds allowed to 1 3) Disable AllowOnShelfHolds 4) Place a hold for a patron 5) Attempt to place a second hold for the patron 6) You should see the "Too many holds" message 7) Set the number of holds allowed to 2 8) Repeat 5, you should be allowed to place the hold, but don't 9) Attempt to place a hold on a record with only checked in items 10) You should get a "No items are available" message Hi Kyle, I want to test this but I cannot produce the original error in master. If I try to place a hold on an item with no defined rules I can get the "Too many/no available" message, adding a rule that states number of allowed holds gets rid of the 'Too Many' With this patch I do see only one message, but the 'too many' seems valid if I don't have any rules defined. Can you let me know if you can reproduce this? Thanks, Nick Tangent: I thought this was related to us seeing "Too many holds" in VOKAL, however we only get that one message even though rules are defined for all categories but playing in master, I cannot get any warning boxes if I turn on 'AllowHoldsOverride' and our system has that on so I don't know if something changed there too. Been puzzling over recreating the error we see but I am guessing it was already fixed in master I too would like more information about how to reproduce the problem which this patch tries to fix. In fact in master I'm unable to trigger a "too many holds" error message by trying to place more holds than are allowed in the rule defined in Circulation and fine rules. I also don't understand what the "Disable AllowOnShelfHolds" step is for in the test plan. (In reply to Owen Leonard from comment #13) > I too would like more information about how to reproduce the problem which > this patch tries to fix. In fact in master I'm unable to trigger a "too many > holds" error message by trying to place more holds than are allowed in the > rule defined in Circulation and fine rules. > > I also don't understand what the "Disable AllowOnShelfHolds" step is for in > the test plan. Hi Owen and Nick, In our shared instance of Koha, we routinely need to block holds on the collections located at our school libraries while they are closed for summer break. When this happens, we end up with records that have items on them that are listed as available, but are in fact not able to receive hold requests because we have blocked their collections using the hold request controls available through the Circulation and Fines Rules administrative tool. When a patron or staff member at one of our public or special libraries attempts to place a hold on a record where all the items have items that are blocked from hold requests because of these school closures they get a "Too many holds" message which is not accurate and then they start to wonder why the system is telling them that Patron X has too many holds when they do not. The second part of the message, "No copies available to placed on hold" is accurate, but they never see that part because they are thrown by the "Too many Holds" part. I hope this helps explain it. Nora Blake MassCat I tried to come up with a testing plan which matched Nora's description of their situation: I set up a Circulation and Fine rule for a Itemtype A at Branch A which set the hold limit to zero. Logged in to Branch A I attempted to place a hold on a title with items of Itemtype A and from Branch A for a patron from Branch A. In master the message I get is: Cannot place hold Too many holds: Fernando De la Cruz has too many holds. No items are available to be placed on hold After applying the patch I get this error: Cannot place hold Too many holds: Fernando De la Cruz has too many holds. It looks to me like the wrong one of the two error messages was removed. Of course I may be testing this incorrectly. I still don't understand what the "Disable AllowOnShelfHolds" step is for in the test plan. *** This bug has been marked as a duplicate of bug 11250 *** |