[Bug 23382] New: Hourly rental fees are not charged
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Bug ID: 23382 Summary: Hourly rental fees are not charged Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com To recreate: 1 - Define an hourly rental fee for an item type, and set it's circ rule to hourly units 2 - Check out an item of that type 3 - Nothing is charged 4 - Enable RentalFeesCheckoutConfirmation syspref 5 - Checkout another item of that type 6 - Note you are notified of the correct charge 7 - Note that confirmation does not charge -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- I believe the issue is two calls to accumulate_rentalcharge() that do not pass current date and due date - so there is no time difference to calculate -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |20912 CC| |martin.renvoize@ptfs-europe | |.com Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20912 [Bug 20912] Rental fees based on time period -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |martin.renvoize@ptfs-europe |ity.org |.com -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 --- Comment #2 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 92221 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92221&action=edit Bug 23382: Improve test coverage -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 92222 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92222&action=edit Bug 23382: Add tests for hourly calculations -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #92221|0 |1 is obsolete| | --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 92236 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92236&action=edit Bug 23382: Improve test coverage -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #92222|0 |1 is obsolete| | --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 92237 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92237&action=edit Bug 23382: Add tests for hourly calculations -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 92238 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92238&action=edit Bug 23382: Add hourly tests for Circulation -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Hmm.. Well, I've greatly increased the test coverage.. but I've not yet managed to replicate the issue :( -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 --- Comment #8 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- oooh.. RentalFeesCheckoutConfirmation plays into it.. I missed that.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- OK, it's not that either... :( I have however found some annoying flaws in our DateTime maths which are highlighted by the hourly tests.. I have no idea how to resolve them.. In short.. according to DateTime: `DateTime->now()->delta_ms(DateTime->now()->add( hours => 4 ) = DateTime::Duration->new( hours => 3, minutes => 59, seconds => 15 );` -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #92236|0 |1 is obsolete| | --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 92273 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92273&action=edit Bug 23382: Improve test coverage -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #92237|0 |1 is obsolete| | --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 92274 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92274&action=edit Bug 23382: Add tests for hourly calculations -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #92238|0 |1 is obsolete| | --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 92275 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92275&action=edit Bug 23382: Add hourly tests for Circulation -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 92276 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92276&action=edit Bug 23382: Fix logic in C4::Circulation::CanBookBeIssued It looks like over progressive rebases of bug 20912 a clause was lost within CanBookBeIssued such that a fatal error may be triggered if an item with no corresponding itemtype was passed into the routine. Additionally the we were passing a Koha::Library object to CalcDateDue rather than a branchcode which resulted in a different duedate being used in 'CanBookBeIssued' when compared to 'AddIssue'. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Pretty sure the last patch resolves the issue reported here.. but I've not added tests to cover that fix yet so that'll have to come in a followup. (CanBookBeIssued seems to be entirely untested with regards to these charges) The increased test coverage does still seem to highlight an issue with DateTime maths but I'm not entirely sure if it's a test suit issue or if it'll result in weird fees for end-users as yet. Either way.. some feedback as to whether this resolves the original issue reported would be great so setting to NSO -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com Status|Needs Signoff |Failed QA --- Comment #15 from Kyle M Hall <kyle@bywatersolutions.com> --- root@2620803d2f65:koha(bug23382-qa)$ prove t/db_dependent/Circulation.t t/db_dependent/Circulation.t .. 43/45 # Failed test 'Hourly rental charge calculated correctly' # at t/db_dependent/Circulation.t line 3357. # got: '7.500000' # expected: '10.000000' # Looks like you failed 1 test of 16. # Failed test 'Incremented fee tests' # at t/db_dependent/Circulation.t line 3368. # Failed test 'Item needs rentalcharge confirmation to be issued' # at t/db_dependent/Circulation.t line 3419. # Structures begin differing at: # $got->{RENTALCHARGE} = '1.00' # $expected->{RENTALCHARGE} = '1' # Looks like you failed 1 test of 2. t/db_dependent/Circulation.t .. 45/45 # Failed test 'CanBookBeIssued & RentalFeesCheckoutConfirmation' # at t/db_dependent/Circulation.t line 3425. # Looks like you failed 2 tests of 45. t/db_dependent/Circulation.t .. Dubious, test returned 2 (wstat 512, 0x200) Failed 2/45 subtests Test Summary Report ------------------- t/db_dependent/Circulation.t (Wstat: 512 Tests: 45 Failed: 2) Failed tests: 44-45 Non-zero exit status: 2 Files=1, Tests=45, 23 wallclock secs ( 0.03 usr 0.03 sys + 16.31 cusr 3.22 csys = 19.59 CPU) Result: FAIL -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andreas.jonsson@kreablo.se --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- *** Bug 23586 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Hourly rental fees are not |Issuing rules failing in |charged |19.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 --- Comment #17 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Changed title to reflect the severity and scope of bug when merged with bug 23586. "Hourly rental fees are not charged" => "Issuing rules failing after bug 20912" -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Issuing rules failing in |Issuing rules failing after |19.05 |bug 20912 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|major |critical -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #92273|0 |1 is obsolete| | --- Comment #18 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 93407 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=93407&action=edit Bug 23382: Improve test coverage -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #92274|0 |1 is obsolete| | --- Comment #19 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 93408 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=93408&action=edit Bug 23382: Add tests for hourly calculations -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #92275|0 |1 is obsolete| | --- Comment #20 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 93409 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=93409&action=edit Bug 23382: Add hourly tests for Circulation -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #92276|0 |1 is obsolete| | --- Comment #21 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 93410 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=93410&action=edit Bug 23382: Fix logic in C4::Circulation::CanBookBeIssued It looks like over progressive rebases of bug 20912 a clause was lost within CanBookBeIssued such that a fatal error may be triggered if an item with no corresponding itemtype was passed into the routine. Additionally the we were passing a Koha::Library object to CalcDateDue rather than a branchcode which resulted in a different duedate being used in 'CanBookBeIssued' when compared to 'AddIssue'. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 --- Comment #22 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 93411 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=93411&action=edit Bug 23382: (follow-up) Clarify variable names This patch updates the variable name '$branch' to '$branchcode' in a number of places to make it clearer what the variable contains. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 --- Comment #23 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 93412 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=93412&action=edit Bug 23382: (follow-up) Ensure tests pass on Wednesdays -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #24 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Tests now pass. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |ASSIGNED --- Comment #25 from Nick Clemens <nick@bywatersolutions.com> --- Discussed with Martin, checkout calculations are coming up short -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=14293 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #26 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Nick Clemens from comment #25)
Discussed with Martin, checkout calculations are coming up short
This sounds like a scary bug - any news here? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 --- Comment #27 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- It is indeed scary.. I'm working on it today but I think I've just had a breakthrough.. just trying to write a bunch more tests to prove my theory -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #93407|0 |1 is obsolete| | --- Comment #28 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 94281 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94281&action=edit Bug 23382: Improve test coverage -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #93408|0 |1 is obsolete| | --- Comment #29 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 94282 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94282&action=edit Bug 23382: Add tests for hourly calculations -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #93409|0 |1 is obsolete| | --- Comment #30 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 94283 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94283&action=edit Bug 23382: Add hourly tests for Circulation -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #93410|0 |1 is obsolete| | --- Comment #31 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 94284 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94284&action=edit Bug 23382: Fix logic in C4::Circulation::CanBookBeIssued It looks like over progressive rebases of bug 20912 a clause was lost within CanBookBeIssued such that a fatal error may be triggered if an item with no corresponding itemtype was passed into the routine. Additionally the we were passing a Koha::Library object to CalcDateDue rather than a branchcode which resulted in a different duedate being used in 'CanBookBeIssued' when compared to 'AddIssue'. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #93411|0 |1 is obsolete| | --- Comment #32 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 94285 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94285&action=edit Bug 23382: (follow-up) Clarify variable names This patch updates the variable name '$branch' to '$branchcode' in a number of places to make it clearer what the variable contains. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #93412|0 |1 is obsolete| | --- Comment #33 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 94286 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94286&action=edit Bug 23382: (follow-up) Ensure tests pass on Wednesdays -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 --- Comment #34 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 94287 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94287&action=edit Bug 23382: (follow-up) Further improve test coverage -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 --- Comment #35 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 94288 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94288&action=edit Bug 23382: Truncate dates to minutes for comparison. It's somewhat of a mess in C4::Circulation as to when dates are truncated and when they are not and as such Koha::Charges::Fees could not reliably assume that the dates passed in were consistent with each other. As such, we take the approach of always truncating to the greatest minute smaller than the passed in dates so we are comparing like for like. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 --- Comment #36 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 94289 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94289&action=edit Bug 23382: hours_between should match the logic of days_between The loops for subtraction holiday dates in hours_between and days_between differed and as such their handling of start and end boundaries for days also differed. This patch makes them handle the boundary days consistently. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |arthur.suzuki@biblibre.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_19_11_candidate -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #37 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Martin Renvoize from comment #31)
Created attachment 94284 [details] [review] Bug 23382: Fix logic in C4::Circulation::CanBookBeIssued
It looks like over progressive rebases of bug 20912 a clause was lost within CanBookBeIssued such that a fatal error may be triggered if an item with no corresponding itemtype was passed into the routine.
Did not we agree we should not test the assumption? We now have a script to catch such inconsistencies, moreover we are suppose to add a FK to unsure it will never happen again. (In reply to Martin Renvoize from comment #35)
Created attachment 94288 [details] [review] Bug 23382: Truncate dates to minutes for comparison.
It's somewhat of a mess in C4::Circulation as to when dates are truncated and when they are not and as such Koha::Charges::Fees could not reliably assume that the dates passed in were consistent with each other. As such, we take the approach of always truncating to the greatest minute smaller than the passed in dates so we are comparing like for like.
How can we test that patch? Do we have test to cover it? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 --- Comment #38 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Comment on attachment 94284 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94284 Bug 23382: Fix logic in C4::Circulation::CanBookBeIssued Review of attachment 94284: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=23382&attachment=94284) ----------------------------------------------------------------- ::: C4/Circulation.pm @@ +707,4 @@
unless ( $duedate ) { my $issuedate = $now->clone();
+ $duedate = CalcDateDue( $issuedate, $effective_itemtype, $circ_library->branchcode, $patron_unblessed );
Updating $branch to $circ_library->branchcode here was the change corrected the main issue in this bug report. @@ +1006,4 @@
my ($rentalCharge) = GetIssuingCharges( $item_object->itemnumber, $patron->borrowernumber ); + + my $itemtype_object = Koha::ItemTypes->find( $item_object->effective_itemtype ); + if ($itemtype_object) {
I only added this check to be consistent with other area's where we are testing for such cases. As you suggest, I'd love to see these checks disappear, but likely as a self-contained bug that catches all cases. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 --- Comment #39 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- As for testing the truncate dates patch.. I would suggest reading the tests added within this patchset to confirm you agree that they make sense (from a datetime math perspective) then running them before this particular patch (and note they fail) then running them again once this patch is applied (noting they now pass). Hope that helps. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94281|0 |1 is obsolete| | --- Comment #40 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 94723 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94723&action=edit Bug 23382: Improve test coverage -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94282|0 |1 is obsolete| | --- Comment #41 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 94724 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94724&action=edit Bug 23382: Add tests for hourly calculations -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94283|0 |1 is obsolete| | --- Comment #42 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 94725 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94725&action=edit Bug 23382: Add hourly tests for Circulation -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94284|0 |1 is obsolete| | --- Comment #43 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 94726 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94726&action=edit Bug 23382: Fix logic in C4::Circulation::CanBookBeIssued It looks like over progressive rebases of bug 20912 a clause was lost within CanBookBeIssued such that a fatal error may be triggered if an item with no corresponding itemtype was passed into the routine. Additionally the we were passing a Koha::Library object to CalcDateDue rather than a branchcode which resulted in a different duedate being used in 'CanBookBeIssued' when compared to 'AddIssue'. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94285|0 |1 is obsolete| | --- Comment #44 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 94727 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94727&action=edit Bug 23382: (follow-up) Clarify variable names This patch updates the variable name '$branch' to '$branchcode' in a number of places to make it clearer what the variable contains. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94286|0 |1 is obsolete| | --- Comment #45 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 94728 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94728&action=edit Bug 23382: (follow-up) Ensure tests pass on Wednesdays -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94287|0 |1 is obsolete| | --- Comment #46 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 94729 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94729&action=edit Bug 23382: (follow-up) Further improve test coverage -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94288|0 |1 is obsolete| | --- Comment #47 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 94730 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94730&action=edit Bug 23382: Truncate dates to minutes for comparison. It's somewhat of a mess in C4::Circulation as to when dates are truncated and when they are not and as such Koha::Charges::Fees could not reliably assume that the dates passed in were consistent with each other. As such, we take the approach of always truncating to the greatest minute smaller than the passed in dates so we are comparing like for like. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94289|0 |1 is obsolete| | --- Comment #48 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 94731 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94731&action=edit Bug 23382: hours_between should match the logic of days_between The loops for subtraction holiday dates in hours_between and days_between differed and as such their handling of start and end boundaries for days also differed. This patch makes them handle the boundary days consistently. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 --- Comment #49 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 94743 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94743&action=edit Bug 23382: (follow-up) Ensure closed day is inside charge window We're testing with an hourly loan of 96 hours to catch a closed day. However, our closed day was hard coded to be a Wednesday so if you ran the tests on a Friday the 96 hour loan would not catch a Wednesday. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 --- Comment #50 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 94755 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94755&action=edit Bug 23382: Use floating time zone to prevent tests to fail because of DST Today Friday Oct 25th, DST next Sunday => Date math are wrong, 96 hours diff becomes 95. To avoid that we use the floating timezone -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94723|0 |1 is obsolete| | --- Comment #51 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 94756 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94756&action=edit Bug 23382: Improve test coverage Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94724|0 |1 is obsolete| | --- Comment #52 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 94757 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94757&action=edit Bug 23382: Add tests for hourly calculations Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94725|0 |1 is obsolete| | --- Comment #53 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 94758 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94758&action=edit Bug 23382: Add hourly tests for Circulation Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94726|0 |1 is obsolete| | --- Comment #54 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 94759 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94759&action=edit Bug 23382: Fix logic in C4::Circulation::CanBookBeIssued It looks like over progressive rebases of bug 20912 a clause was lost within CanBookBeIssued such that a fatal error may be triggered if an item with no corresponding itemtype was passed into the routine. Additionally the we were passing a Koha::Library object to CalcDateDue rather than a branchcode which resulted in a different duedate being used in 'CanBookBeIssued' when compared to 'AddIssue'. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94727|0 |1 is obsolete| | --- Comment #55 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 94760 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94760&action=edit Bug 23382: (follow-up) Clarify variable names This patch updates the variable name '$branch' to '$branchcode' in a number of places to make it clearer what the variable contains. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94728|0 |1 is obsolete| | --- Comment #56 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 94761 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94761&action=edit Bug 23382: (follow-up) Ensure tests pass on Wednesdays Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94729|0 |1 is obsolete| | --- Comment #57 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 94762 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94762&action=edit Bug 23382: (follow-up) Further improve test coverage Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94730|0 |1 is obsolete| | --- Comment #58 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 94763 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94763&action=edit Bug 23382: Truncate dates to minutes for comparison. It's somewhat of a mess in C4::Circulation as to when dates are truncated and when they are not and as such Koha::Charges::Fees could not reliably assume that the dates passed in were consistent with each other. As such, we take the approach of always truncating to the greatest minute smaller than the passed in dates so we are comparing like for like. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94731|0 |1 is obsolete| | --- Comment #59 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 94764 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94764&action=edit Bug 23382: hours_between should match the logic of days_between The loops for subtraction holiday dates in hours_between and days_between differed and as such their handling of start and end boundaries for days also differed. This patch makes them handle the boundary days consistently. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94743|0 |1 is obsolete| | --- Comment #60 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 94765 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94765&action=edit Bug 23382: (follow-up) Ensure closed day is inside charge window We're testing with an hourly loan of 96 hours to catch a closed day. However, our closed day was hard coded to be a Wednesday so if you ran the tests on a Friday the 96 hour loan would not catch a Wednesday. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94755|0 |1 is obsolete| | --- Comment #61 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 94766 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94766&action=edit Bug 23382: Use floating time zone to prevent tests to fail because of DST Today Friday Oct 25th, DST next Sunday => Date math are wrong, 96 hours diff becomes 95. To avoid that we use the floating timezone Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 --- Comment #62 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Thanks dude :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94756|0 |1 is obsolete| | --- Comment #63 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 94774 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94774&action=edit Bug 23382: Improve test coverage Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94757|0 |1 is obsolete| | --- Comment #64 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 94775 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94775&action=edit Bug 23382: Add tests for hourly calculations Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94758|0 |1 is obsolete| | --- Comment #65 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 94776 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94776&action=edit Bug 23382: Add hourly tests for Circulation Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94759|0 |1 is obsolete| | --- Comment #66 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 94777 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94777&action=edit Bug 23382: Fix logic in C4::Circulation::CanBookBeIssued It looks like over progressive rebases of bug 20912 a clause was lost within CanBookBeIssued such that a fatal error may be triggered if an item with no corresponding itemtype was passed into the routine. Additionally the we were passing a Koha::Library object to CalcDateDue rather than a branchcode which resulted in a different duedate being used in 'CanBookBeIssued' when compared to 'AddIssue'. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94760|0 |1 is obsolete| | --- Comment #67 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 94778 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94778&action=edit Bug 23382: (follow-up) Clarify variable names This patch updates the variable name '$branch' to '$branchcode' in a number of places to make it clearer what the variable contains. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94761|0 |1 is obsolete| | --- Comment #68 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 94779 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94779&action=edit Bug 23382: (follow-up) Ensure tests pass on Wednesdays Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94762|0 |1 is obsolete| | --- Comment #69 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 94780 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94780&action=edit Bug 23382: (follow-up) Further improve test coverage Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94763|0 |1 is obsolete| | --- Comment #70 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 94781 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94781&action=edit Bug 23382: Truncate dates to minutes for comparison. It's somewhat of a mess in C4::Circulation as to when dates are truncated and when they are not and as such Koha::Charges::Fees could not reliably assume that the dates passed in were consistent with each other. As such, we take the approach of always truncating to the greatest minute smaller than the passed in dates so we are comparing like for like. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94764|0 |1 is obsolete| | --- Comment #71 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 94782 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94782&action=edit Bug 23382: hours_between should match the logic of days_between The loops for subtraction holiday dates in hours_between and days_between differed and as such their handling of start and end boundaries for days also differed. This patch makes them handle the boundary days consistently. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94765|0 |1 is obsolete| | --- Comment #72 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 94783 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94783&action=edit Bug 23382: (follow-up) Ensure closed day is inside charge window We're testing with an hourly loan of 96 hours to catch a closed day. However, our closed day was hard coded to be a Wednesday so if you ran the tests on a Friday the 96 hour loan would not catch a Wednesday. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94766|0 |1 is obsolete| | --- Comment #73 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 94784 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94784&action=edit Bug 23382: Use floating time zone to prevent tests to fail because of DST Today Friday Oct 25th, DST next Sunday => Date math are wrong, 96 hours diff becomes 95. To avoid that we use the floating timezone Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |19.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 --- Comment #74 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work! Pushed to master for 19.11.00 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_19_11_candidate | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=23974 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=24437 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|19.11.00 |19.11.00, 19.05.07 released in| | Status|Pushed to master |Pushed to stable CC| |lucas@bywatersolutions.com --- Comment #75 from Lucas Gass <lucas@bywatersolutions.com> --- backported to 19.05.x for 19.05.07 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kelly@bywatersolutions.com --- Comment #76 from Nick Clemens <nick@bywatersolutions.com> --- *** Bug 24437 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 --- Comment #77 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 107456 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=107456&action=edit Bug 23382: (RM follow-up) Revert "hours_between should match..." This reverts commit a693c7243c23f888e2fad38a4fff9f37ff4a9301 which caused regressions. The original loop compared start date to end date and iterated all the way to start date equals end date. The alternate implimentation inadvertantly looped from start date, skipped the first day then iterated up to one day beyond end date. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 --- Comment #78 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 107457 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=107457&action=edit Bug 23382: (RM follow-up) Ensure tests pass on Wednesdays Koha::Calendar assumes we never issue an item on a closed day.. this is a fair assumption but the hardcoded closed day in our tests means it does not hold true in this case. This patch sets the closed day dynamically to prevent such a case. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23382 --- Comment #79 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- The last 2 patches were pushed to master (19.11) but not attached here and so missed for 19.05.x They fix bug 25665. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org