Bug 20660 - AddReturn should use return date override for debarments
Summary: AddReturn should use return date override for debarments
Status: Pushed to Stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: David Bourgault
QA Contact: Testopia
URL:
Whiteboard:
Keywords: release-notes-needed
Depends on:
Blocks: 17015 21188
  Show dependency treegraph
 
Reported: 2018-04-25 19:55 UTC by David Bourgault
Modified: 2018-09-21 12:15 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 20660: AddReturn uses return_date override for new debarments (3.18 KB, patch)
2018-04-25 20:01 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 20660: Test AddReturn with $return_date override (2.23 KB, patch)
2018-05-01 18:08 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 20660: Return date override affects debarments in AddReturn (1.02 KB, patch)
2018-05-01 18:08 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 20660: Test AddReturn with $return_date override (2.28 KB, patch)
2018-08-02 12:20 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20660: Return date override affects debarments in AddReturn (1.08 KB, patch)
2018-08-02 12:20 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20660: Test AddReturn with $return_date override (2.34 KB, patch)
2018-08-02 14:59 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20660: Return date override affects debarments in AddReturn (1.14 KB, patch)
2018-08-02 14:59 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Bourgault 2018-04-25 19:55:22 UTC
C4::Circulation::AddReturn does not use the return_date override when calculating new debarments. This causes issues when trying to test functions with return dates in the future.
Comment 1 David Bourgault 2018-04-25 20:01:19 UTC Comment hidden (obsolete)
Comment 2 M. Tompsett 2018-04-26 04:41:12 UTC Comment hidden (obsolete)
Comment 3 David Bourgault 2018-05-01 18:08:18 UTC
Created attachment 74955 [details] [review]
Bug 20660: Test AddReturn with $return_date override
Comment 4 David Bourgault 2018-05-01 18:08:26 UTC
Created attachment 74956 [details] [review]
Bug 20660: Return date override affects debarments in AddReturn

Re-did the patch to follow best practices.

TEST PLAN:
0. Apply first patch only
1. prove t/db_dependent/Circulation.t
2. Apply second patch
3. prove t/db_dependent/Circulation.t

prove should fail the first time and pass the second time.
Comment 5 David Bourgault 2018-05-01 18:11:13 UTC
(In reply to M. Tompsett from comment #2)
> Should dropbox override the return date logic being added?

It was already present in the code:

$today = $return_date if $return_date;
$today = $dropboxdate if $dropbox;

first line is my addition, second line is from existing code. So if $dropbox is set, it will override any return_date overrides.
Comment 6 Josef Moravec 2018-08-02 12:20:08 UTC
Created attachment 77426 [details] [review]
Bug 20660: Test AddReturn with $return_date override

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 7 Josef Moravec 2018-08-02 12:20:12 UTC
Created attachment 77427 [details] [review]
Bug 20660: Return date override affects debarments in AddReturn

Re-did the patch to follow best practices.

TEST PLAN:
0. Apply first patch only
1. prove t/db_dependent/Circulation.t
2. Apply second patch
3. prove t/db_dependent/Circulation.t

prove should fail the first time and pass the second time.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 8 Katrin Fischer 2018-08-02 14:59:43 UTC
Created attachment 77448 [details] [review]
Bug 20660: Test AddReturn with $return_date override

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 9 Katrin Fischer 2018-08-02 14:59:47 UTC
Created attachment 77449 [details] [review]
Bug 20660: Return date override affects debarments in AddReturn

Re-did the patch to follow best practices.

TEST PLAN:
0. Apply first patch only
1. prove t/db_dependent/Circulation.t
2. Apply second patch
3. prove t/db_dependent/Circulation.t

prove should fail the first time and pass the second time.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 10 Nick Clemens 2018-08-09 12:23:16 UTC
Awesome work all!

Pushed to master for 18.11
Comment 11 Jonathan Druart 2018-08-09 13:36:23 UTC
It broke a test:

t/db_dependent/Circulation/issue.t .. 1/32 Argument "2014-04-01 23:42" isn't numeric in numeric eq (==) at /usr/lib/x86_64-linux-gnu/perl5/5.20/DateTime.pm line 1863.
A DateTime object can only be compared to another DateTime object (DateTime=HASH(0x8b9dd28), 2014-04-01 23:42). at /home/vagrant/kohaclone/C4/Overdues.pm line 290.
# Looks like your test exited with 255 just after 26.

https://jenkins.koha-community.org/job/Koha_Master_D9/lastCompletedBuild/testReport/
Comment 12 Jonathan Druart 2018-08-09 13:40:32 UTC
See bug 21188.
Comment 13 Jonathan Druart 2018-08-09 13:53:24 UTC
The description of this bug report and what the patch does are a bit different. There is a change in the behavior that should have been adverted in the commit message, and tested on the UI.
Comment 14 Martin Renvoize 2018-08-10 07:59:04 UTC
Pushed to 18.05.x for 18.05.03
Comment 15 Martin Renvoize 2018-08-10 08:04:02 UTC
Can we have a short summary of the behavioural change and ui effects for the release notes, please.. I'm happy it's a bugfix, but would like the release notes to be clear there's a change.
Comment 16 Fridolin SOMERS 2018-09-21 12:15:15 UTC
I prefer not to push to 17.11.x because of th change in the behavior