Bug 11801 - In transit hold items incorrectly labeled as "Waiting to be pulled" on request.pl
Summary: In transit hold items incorrectly labeled as "Waiting to be pulled" on reques...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: Kyle M Hall
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-02-20 15:06 UTC by Kyle M Hall
Modified: 2015-06-04 23:30 UTC (History)
11 users (show)

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


Attachments
Pre-patch (21.99 KB, image/png)
2014-02-20 15:10 UTC, Kyle M Hall
Details
Post-patch (22.65 KB, image/png)
2014-02-20 15:10 UTC, Kyle M Hall
Details
Bug 11801 - In transit hold items incorrectly labels as "Waiting to be pulled" on request.pl (3.69 KB, patch)
2014-02-20 15:11 UTC, Kyle M Hall
Details | Diff | Splinter Review
DBIC version - Bug 11801 - In transit hold items incorrectly labels as "Waiting to be pulled" on request.pl (3.74 KB, patch)
2014-03-11 11:21 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 11801: In transit hold items incorrectly labels as "Waiting to be pulled" on request.pl (1.88 KB, patch)
2014-03-20 12:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11801: In transit hold items incorrectly labels as "Waiting to be pulled" on request.pl (1.93 KB, patch)
2014-06-04 18:39 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 11801: In transit hold items incorrectly labels as "Waiting to be pulled" on request.pl (2.00 KB, patch)
2014-06-05 07:44 UTC, Martin Renvoize
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 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 2014-02-20 15:10:04 UTC
Created attachment 25507 [details]
Pre-patch
Comment 2 Kyle M Hall 2014-02-20 15:10:20 UTC
Created attachment 25508 [details]
Post-patch
Comment 3 Kyle M Hall 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 2014-06-04 18:39:38 UTC Comment hidden (obsolete)
Comment 11 Martin Renvoize 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 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 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.