|
Description
David Bourgault
2018-04-25 19:55:22 UTC
Created attachment 74857 [details] [review] Bug 20660: AddReturn uses return_date override for new debarments This patches is a minor tweak (single line) to C4::Circulation to use the return_date value as $today when calculating new debarments in AddReturn. I've also added a test in t/db_dependent/Circulation.t TEST PLAN 1) comment out line 1988 in Circulation.pm ("$today = $return_date if $return_date") 2) prove t/db_dependent/Circulation.t (Fail) 3) uncomment the line 4) prove t/db_dependent/Circulation.t (Pass) (In reply to David Bourgault from comment #1) > Created attachment 74857 [details] [review] [review] > Bug 20660: AddReturn uses return_date override for new debarments > > This patches is a minor tweak (single line) to C4::Circulation to use the > return_date value as $today when calculating new debarments in AddReturn. > > I've also added a test in t/db_dependent/Circulation.t > > TEST PLAN > > 1) comment out line 1988 in Circulation.pm ("$today = $return_date if > $return_date") > 2) prove t/db_dependent/Circulation.t (Fail) > 3) uncomment the line > 4) prove t/db_dependent/Circulation.t (Pass) It is better to split the test from the change, so that when you want to prove the problem you just apply the test. Then if you wish to prove the solution also apply the second patch, and have the tests pass. Should dropbox override the return date logic being added? Created attachment 74955 [details] [review] Bug 20660: Test AddReturn with $return_date override 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. (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. Created attachment 77426 [details] [review] Bug 20660: Test AddReturn with $return_date override Signed-off-by: Josef Moravec <josef.moravec@gmail.com> 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> 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> 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> Awesome work all! Pushed to master for 18.11 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/ See bug 21188. 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. Pushed to 18.05.x for 18.05.03 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. I prefer not to push to 17.11.x because of th change in the behavior |