Bug 11801

Summary: In transit hold items incorrectly labeled as "Waiting to be pulled" on request.pl
Product: Koha Reporter: Kyle M Hall (khall) <kyle>
Component: Hold requestsAssignee: Kyle M Hall (khall) <kyle>
Status: CLOSED FIXED QA Contact: Martin Renvoize (ashimema) <martin.renvoize>
Severity: minor    
Priority: P5 - low CC: brendan, gmcharlt, gwilliams, jonathan.druart, m.de.rooy, martin.renvoize, nmichot, paul.poulain, sandboxes, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Attachments: Pre-patch
Post-patch
Bug 11801 - In transit hold items incorrectly labels as "Waiting to be pulled" on request.pl
DBIC version - Bug 11801 - In transit hold items incorrectly labels as "Waiting to be pulled" on request.pl
Bug 11801: In transit hold items incorrectly labels as "Waiting to be pulled" on request.pl
Bug 11801: In transit hold items incorrectly labels as "Waiting to be pulled" on request.pl
[PASSED QA] Bug 11801: In transit hold items incorrectly labels as "Waiting to be pulled" on request.pl

Description Kyle M Hall (khall) 2014-02-20 15:06:15 UTC
If an item needs to be transferred to fill a hold, the hold will be listed as 'Waiting to be pulled" while it is in transit.
Comment 1 Kyle M Hall (khall) 2014-02-20 15:10:04 UTC
Created attachment 25507 [details]
Pre-patch
Comment 2 Kyle M Hall (khall) 2014-02-20 15:10:20 UTC
Created attachment 25508 [details]
Post-patch
Comment 3 Kyle M Hall (khall) 2014-02-20 15:11:08 UTC Comment hidden (obsolete)
Comment 4 michot 2014-03-11 11:18:58 UTC
test done on sandbox today :

1) Place a hold for pickup at another library
2) Check in the item, confirm the hold transfer
3) View the holds for that record ( request.pl )

Before applying patch, message on "request.pl = wainting to be pulled

After applying patch, message on "request.pl = Item being transferred to XXX

So it seems OK
Comment 5 Biblibre Sandboxes 2014-03-11 11:21:11 UTC
Patch tested with a sandbox, by Nadine Michot <nmichot@voila.fr>
Comment 6 Biblibre Sandboxes 2014-03-11 11:21:38 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2014-03-20 12:36:12 UTC
Since I disagree with the idea to call Koha::Database directly from pl scripts, I disagree with this patch :)
Moreover, in my opinion it is useless to get the transfer using DBIC, we have a C4::Circulation::GetTransfers routine.

The patch works as expected :)

Galen and/or others QAers, could you please take a position on that?

I will provide a counter patch.
Comment 8 Jonathan Druart 2014-03-20 12:36:38 UTC Comment hidden (obsolete)
Comment 9 Galen Charlton 2014-06-04 18:36:30 UTC
(In reply to Jonathan Druart from comment #7)
> Since I disagree with the idea to call Koha::Database directly from pl
> scripts, I disagree with this patch :)
> Moreover, in my opinion it is useless to get the transfer using DBIC, we
> have a C4::Circulation::GetTransfers routine.
> 
> The patch works as expected :)
> 
> Galen and/or others QAers, could you please take a position on that?

I have no problem invoking Koha::Database directly from scripts.  Not every bit of database access needs to be wrapped in middleware.  However, I will say that if I were doing it via DBIC, I would probably add an at_destination method to the Koha::Schema::Result::Reserves class.

That said, I have a stronger aversion to having multiple ways of asking the same question.  GetTransfers() gets the job done.
Comment 10 Kyle M Hall (khall) 2014-06-04 18:39:38 UTC Comment hidden (obsolete)
Comment 11 Martin Renvoize (ashimema) 2014-06-05 07:44:50 UTC
Created attachment 28670 [details] [review]
[PASSED QA] Bug 11801: In transit hold items incorrectly labels as "Waiting to be pulled" on request.pl

If an item needs to be transferred to fill a hold, the hold will be
listed as 'Waiting to be pulled" while it is in transit.

Test Plan:
1) Place a hold for pickup at another library
2) Check in the item, confirm the hold transfer
3) View the holds for that record ( request.pl )
4) Note the priority is "In transit" and the status is 'Waiting to be
   pulled".
5) Apply this patch
6) Reload the holds page
7) Note the status is now "Item being transferred to $branch"

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Martin Renvoize (ashimema) 2014-06-05 07:47:41 UTC
Works as advertised and passes qa scripts.

I'm also happier with the alternative patch using existent routines. I'm an advocate of dbic, but in this case I think as galen rightly states we should all have an aversion to having multiple ways of asking the same question.

Nice work guys
Comment 13 Tomás Cohen Arazi (tcohen) 2014-06-18 18:42:26 UTC
Patch pushed to master. I also prefer using GetTransfers.

Thanks Jonathan!
Comment 14 Galen Charlton 2014-06-18 21:14:34 UTC
Pushed to 3.16.x for inclusion in 3.16.1.