Bug 8448 - Holds Awaiting Pickup : Cancelling a hold on a waiting item with multiple holds displays a blank screen instead of a warning prompt
Summary: Holds Awaiting Pickup : Cancelling a hold on a waiting item with multiple hol...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: 3.8
Hardware: All All
: P2 major (vote)
Assignee: David Cook
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-16 03:55 UTC by David Cook
Modified: 2013-12-05 19:59 UTC (History)
5 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 8448 - Holds Awaiting Pickup : Cancelling a hold on a waiting item with multiple holds displays a blank screen (3.46 KB, patch)
2012-07-16 04:24 UTC, David Cook
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8448 - Holds Awaiting Pickup : Cancelling a hold on a waiting item with multiple holds displays a blank screen instead of a warning prompt (3.46 KB, patch)
2012-07-20 13:50 UTC, Marc Véron
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8448 - Holds Awaiting Pickup : Cancelling a hold on a waiting item with multiple holds displays a blank screen instead of a warning prompt (3.68 KB, patch)
2012-07-23 14:07 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2012-07-16 03:55:02 UTC
When you cancel a hold in the "Hold Waiting" tab of Holds Awaiting Pickup, you should be prompted to retain or transfer the hold for the next patron in the queue. However, instead of seeing this prompt, you are just shown white screen that fails to alert you to the fact that there are more holds on that item!

There is a missing FOREACH reference in waitingreserves.tt. The template tries to directly reference the hash within the cancel_results array, but we need to access the array before we can access the hash.
Comment 1 David Cook 2012-07-16 04:24:19 UTC Comment hidden (obsolete)
Comment 2 David Cook 2012-07-18 06:25:11 UTC
To test:

Before applying the patch:

1) Place a hold on item XX for Patron 1
2) Place a hold on item XX for Patron 2
3) Check in item XX and confirm hold for Patron 1
3) Go to Holds Awaiting Pickup in the Circulation module 
4) Cancel the hold for Patron 1
5) Instead of seeing a prompt or the table, you should see nothing but white and the masthead/header at the top of the page

After applying the patch:

Follow steps 1-4 again. 

This time, you should see a prompt telling you to retain the hold (or transfer it, if Patron 2 had a different pick-up location)
Comment 3 Marc Véron 2012-07-19 07:28:17 UTC
How I tested:

- Item X is checked out.

- Place holds for patron ONE and patron TWO (Next available item, there ist just 1 item for this biblio anyway)

- Check in Itm X. Koha says that there is a hold for Patron ONE (General note, not related to this patch: I would expect that it says soething about patron TWO as well or something like 'There are N holds for this item').

- I confirm hold for patron ONE

- I switch to Holds Awaiting Pickup in the Circulation module

- I see the hold waiting for Patron ONE

- I cancel this hold.

- I get following message (as expected after applying the patch): 
-------------------
This hold (Fuss- und Wanderwege bei der Planung von ländlichen Wegnetzen im Berggebiet) was placed by : Patron TWO, Please retain this hold. 
[Daten absenden]  (a button, it seems to take the OS language)
------------------

- Now I'm not quite sure what 'Daten absenden' really means? - It seems to delete the hold for patron ONE (what actually I expect), but then, I would expect the item now to appear in the pickup list as waiting for pickup by patron TWO? 

- But now the list is empty. What happens now to the hold placed by patron TWO? Is there some action I have to take now manually?
Comment 4 David Cook 2012-07-19 07:50:44 UTC
(In reply to comment #3)
> How I tested:
> 
> - Item X is checked out.
> 
> - Place holds for patron ONE and patron TWO (Next available item, there ist
> just 1 item for this biblio anyway)
> 
> - Check in Itm X. Koha says that there is a hold for Patron ONE (General
> note, not related to this patch: I would expect that it says soething about
> patron TWO as well or something like 'There are N holds for this item').
> 
> - I confirm hold for patron ONE
> 
> - I switch to Holds Awaiting Pickup in the Circulation module
> 
> - I see the hold waiting for Patron ONE
> 
> - I cancel this hold.
> 
> - I get following message (as expected after applying the patch): 
> -------------------
> This hold (Fuss- und Wanderwege bei der Planung von ländlichen Wegnetzen im
> Berggebiet) was placed by : Patron TWO, Please retain this hold. 
> [Daten absenden]  (a button, it seems to take the OS language)
> ------------------
> 
> - Now I'm not quite sure what 'Daten absenden' really means? - It seems to
> delete the hold for patron ONE (what actually I expect), but then, I would
> expect the item now to appear in the pickup list as waiting for pickup by
> patron TWO? 
> 
> - But now the list is empty. What happens now to the hold placed by patron
> TWO? Is there some action I have to take now manually?

When I translated "Daten absenden" to English, it said "Data send", so it's the equivalent of "Submit". I think "OK" would be better than "Submit", but that's outside of the scope of this particular patch.

Yes, now that the list is empty, you have to manually check-in the item again to generate the prompt to "confirm hold" for Patron TWO. This is the default Koha behaviour.

In Bug 8449, I change the wording and styling of the messages you receive during this process, and I've indicated that you need to check-in the item to continue processing the hold. 

Sounds like it worked though :)
Comment 5 Marc Véron 2012-07-20 05:27:32 UTC
David,

Thanks for the clarification regarding the behaviour of Koha. 

I will sign off the patch as soon I have a better internet connection (still on vacation in a house with DSL via overhead phone wires, the connection breaks frequently due to the heavy wind we have at the moment).

Marc
Comment 6 Marc Véron 2012-07-20 13:50:21 UTC Comment hidden (obsolete)
Comment 7 Marc Véron 2012-07-20 13:53:58 UTC
Always got Traceback error during git bz attach -e (due to slow internet connection).

Anyway, patch behaves as expected, signed off.
Comment 8 Jonathan Druart 2012-07-23 14:07:50 UTC
Created attachment 11080 [details] [review]
[SIGNED-OFF] Bug 8448 - Holds Awaiting Pickup : Cancelling a hold on a waiting item with multiple holds displays a blank screen instead of a warning prompt

Since Bug 6039, cancel_result is an arrayref.
It is logical to do a FOREACH on it.

Marking as Passed QA.

This new patch replaces tab characters with 4 spaces, please update your editor configuration.
Comment 9 Paul Poulain 2012-07-24 14:40:21 UTC
QA comment: valid_tt.t still OK after this patch
Comment 10 Chris Cormack 2012-07-25 03:02:31 UTC
Nice catch David, pushed to 3.8.x will be in 3.8.4