Bug 13517 - Show waiting date on reserve/request.pl
Summary: Show waiting date on reserve/request.pl
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement
Assignee: Kyle M Hall (khall)
QA Contact: Testopia
URL:
Keywords:
Depends on: 13853
Blocks: 13918
  Show dependency treegraph
 
Reported: 2015-01-05 15:54 UTC by Kyle M Hall (khall)
Modified: 2017-06-14 22:10 UTC (History)
4 users (show)

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


Attachments
Bug 13517 - Tidy request.pl (32.29 KB, patch)
2015-01-05 15:57 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 13517 - Show waiting date on reserve/request.pl (3.67 KB, patch)
2015-01-05 15:57 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 13517 - Show waiting date on reserve/request.pl (3.73 KB, patch)
2015-01-21 22:41 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13517 - Show waiting date on reserve/request.pl (3.70 KB, patch)
2015-02-11 14:18 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 13517 - Show waiting date on reserve/request.pl (3.77 KB, patch)
2015-02-12 10:21 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13517: Reintroduce the field list (1.43 KB, patch)
2015-02-12 10:21 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13517: Add a regression test (1.25 KB, patch)
2015-02-12 10:21 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13517 - Show waiting date on reserve/request.pl (12.42 KB, patch)
2015-03-18 12:53 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 13517 - Show waiting date on reserve/request.pl (12.42 KB, patch)
2015-03-18 13:21 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 13517 - Show waiting date on reserve/request.pl (11.97 KB, patch)
2015-06-09 16:33 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 13517 - Show waiting date on reserve/request.pl (11.97 KB, patch)
2015-06-09 17:14 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 13517 - Show waiting date on reserve/request.pl (12.15 KB, patch)
2015-06-09 17:35 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 13517 - Show waiting date on reserve/request.pl (12.14 KB, patch)
2015-06-26 14:52 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 13517 [QA Followup] - Add unit tests (2.41 KB, patch)
2015-06-26 14:52 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 13517 [QA Followup] - Fix unit tests (1.29 KB, patch)
2015-07-09 15:24 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 13517 [QA Followup] - Fix unit tests (1.29 KB, patch)
2015-07-09 15:25 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 13517 - Show waiting date on reserve/request.pl (12.21 KB, patch)
2015-07-09 16:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13517 [QA Followup] - Add unit tests (2.47 KB, patch)
2015-07-09 16:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13517 [QA Followup] - Fix unit tests (1.35 KB, patch)
2015-07-09 16:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13517 - Show waiting date on reserve/request.pl (11.82 KB, patch)
2015-09-16 14:47 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 13517 [QA Followup] - Add unit tests (2.48 KB, patch)
2015-09-16 14:48 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 13517 [QA Followup] - Fix unit tests (1.36 KB, patch)
2015-09-16 14:48 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 13517 - Show waiting date on reserve/request.pl (11.98 KB, patch)
2015-10-02 14:24 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 13517 [QA Followup] - Add unit tests (2.48 KB, patch)
2015-10-02 14:24 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 13517 [QA Followup] - Fix unit tests (1.36 KB, patch)
2015-10-02 14:24 UTC, Kyle M Hall (khall)
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 (khall) 2015-01-05 15:54:12 UTC
The holds table should show the waiting date for a hold in s similar manner to the way it does on catalogue/detail.pl
Comment 1 Kyle M Hall (khall) 2015-01-05 15:57:13 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall (khall) 2015-01-05 15:57:25 UTC Comment hidden (obsolete)
Comment 3 Aleisha Amohia 2015-01-21 22:41:17 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2015-01-23 15:29:31 UTC
(In reply to Kyle M Hall from comment #1)
> Created attachment 34958 [details] [review] [review]
> Bug 13517 - Tidy request.pl

This is not needed, it will introduce conflict with other patches in the queue.
Why did you tidy this file?
Comment 5 Kyle M Hall (khall) 2015-02-11 14:18:07 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2015-02-11 15:56:15 UTC
Comment on attachment 35834 [details] [review]
Bug 13517 - Show waiting date on reserve/request.pl

Review of attachment 35834 [details] [review]:
-----------------------------------------------------------------

::: C4/Reserves.pm
@@ +303,4 @@
>      # Find the desired items in the reserves
>      my @params;
>      my $query = "
> +        SELECT * FROM reserves WHERE biblionumber = ? ";

Please keep the field list.
Comment 7 Jonathan Druart 2015-02-12 10:21:09 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2015-02-12 10:21:13 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2015-02-12 10:21:19 UTC Comment hidden (obsolete)
Comment 10 Kyle M Hall (khall) 2015-03-18 12:53:36 UTC Comment hidden (obsolete)
Comment 11 Kyle M Hall (khall) 2015-03-18 13:21:39 UTC Comment hidden (obsolete)
Comment 12 Kyle M Hall (khall) 2015-06-09 16:33:46 UTC Comment hidden (obsolete)
Comment 13 Kyle M Hall (khall) 2015-06-09 17:14:18 UTC Comment hidden (obsolete)
Comment 14 Kyle M Hall (khall) 2015-06-09 17:35:41 UTC Comment hidden (obsolete)
Comment 15 Jonathan Druart 2015-06-23 09:44:38 UTC
Blocked by bug 13853.
Comment 16 Kyle M Hall (khall) 2015-06-26 14:52:48 UTC Comment hidden (obsolete)
Comment 17 Kyle M Hall (khall) 2015-06-26 14:52:59 UTC Comment hidden (obsolete)
Comment 18 Jonathan Druart 2015-07-02 13:58:47 UTC
Comment on attachment 40677 [details] [review]
Bug 13517 [QA Followup] - Add unit tests

Review of attachment 40677 [details] [review]:
-----------------------------------------------------------------

Please amend the last patch.

::: t/db_dependent/Hold.t
@@ +48,5 @@
>  is( $dt, undef, "Koha::Hold->waiting_expires_on returns undef if ReservesMaxPickUpDelay is not set");
>  
>  is( $hold->is_waiting, 1, 'The hold is waiting' );
> +is( $hold->is_found, 1, 'The hold is found');
> +isnt( $hold->is_in_transit,  'The hold is not in transit' );

This is wrong.

@@ +66,5 @@
>  $dt = $hold->waiting_expires_on();
>  is( $dt, undef, "Koha::Hold->waiting_expires_on returns undef if found is not 'W' ( Set to empty string )");
>  isnt( $hold->is_waiting, 1, 'The hold is not waiting (W)' );
> +is( $hold->is_found, 0, 'The hold is not found' );
> +isnt( $hold->is_in_transit, 'The hold is not in transit' );

Same here.
Comment 19 Kyle M Hall (khall) 2015-07-09 15:24:23 UTC Comment hidden (obsolete)
Comment 20 Kyle M Hall (khall) 2015-07-09 15:25:39 UTC Comment hidden (obsolete)
Comment 21 Jonathan Druart 2015-07-09 16:36:24 UTC
So... I was passing qa on this one (I promise!) and, during my last test, I have found a waiting reserve in my DB without itemnumber (??)

Which makes the script explodes:
  Can't call method "holdingbranch" on an undefined value at /home/koha/src/reserve/request.pl line 488.

It's an old data and a DB I used to test, so maybe this case is impossible to reach
      reserve_id: 2051
  borrowernumber: 7781
     reservedate: 2011-08-19
    biblionumber: 4934
  constrainttype: a
      branchcode: MAURES
notificationdate: NULL
    reminderdate: NULL
cancellationdate: NULL
    reservenotes: 
        priority: 0
           found: W
       timestamp: 2015-04-28 16:41:22
      itemnumber: NULL
     waitingdate: 2015-04-28
  expirationdate: NULL
  lowestPriority: 0
         suspend: 0
   suspend_until: NULL

Sorry I don't remember what I did on April 28th :)

I don't really know what to do, we could let the code as it, and see if someone report the issue.
Or add an existing test on the item.

What do you prefer?
Comment 22 Jonathan Druart 2015-07-09 16:47:55 UTC Comment hidden (obsolete)
Comment 23 Jonathan Druart 2015-07-09 16:48:02 UTC Comment hidden (obsolete)
Comment 24 Jonathan Druart 2015-07-09 16:48:06 UTC Comment hidden (obsolete)
Comment 25 Jonathan Druart 2015-07-09 16:50:45 UTC
(In reply to Jonathan Druart from comment #21)

Ok, let's consider it comes from my DB.

I think it's better to know if a bug exists somewhere else, instead of adding useless tests in this (already messy) script.

But, dear RM, don't push it just before the release :)
Comment 26 Kyle M Hall (khall) 2015-09-16 14:47:51 UTC Comment hidden (obsolete)
Comment 27 Kyle M Hall (khall) 2015-09-16 14:48:28 UTC Comment hidden (obsolete)
Comment 28 Kyle M Hall (khall) 2015-09-16 14:48:31 UTC Comment hidden (obsolete)
Comment 29 Jonathan Druart 2015-10-01 13:18:34 UTC
To test bug 13918 I need to apply this patch set before but it does not apply anymore.
Comment 30 Kyle M Hall (khall) 2015-10-02 14:24:39 UTC
Created attachment 43066 [details] [review]
Bug 13517 - Show waiting date on reserve/request.pl

The holds table should show the waiting date for a hold in s similar
manner to the way it does on catalogue/detail.pl In addition, if the
ReservesMaxPickUpDelay syspref is set, we should display the date
when the waiting hold expires.

Test Plan:
1) Apply this patch
2) Set ReservesMaxPickUpDelay to 7
3) Find a waiting hold
4) Browse to reserve/request.pl for that record
5) Notice the "Item waiting" message now has "since <waitingdate>"
6) Notice the expiration field now has the date of <waitingdate> + 7 days

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Cathi Wiggins <CWIGGINS@ci.arcadia.ca.us>

Signed-off-by: Megan Wianecki <mwianecki@mplmain.mtpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 31 Kyle M Hall (khall) 2015-10-02 14:24:46 UTC
Created attachment 43067 [details] [review]
Bug 13517 [QA Followup] - Add unit tests

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 32 Kyle M Hall (khall) 2015-10-02 14:24:48 UTC
Created attachment 43068 [details] [review]
Bug 13517 [QA Followup] - Fix unit tests

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 33 Jesse Weaver 2015-12-30 23:44:00 UTC
Pushed to master. Thanks for your contribution!