| Summary: | Patron may not be debarred if backdated return | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Jonathan Druart <jonathan.druart> |
| Component: | Circulation | Assignee: | Jonathan Druart <jonathan.druart> |
| Status: | CLOSED FIXED | QA Contact: | Nick Clemens (kidclamp) <nick> |
| Severity: | major | ||
| Priority: | P5 - low | CC: | 1joynelson, gmcharlt, kyle.m.hall, martin.renvoize, nick, victor |
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | --- |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: |
20.05.00, 19.11.06, 19.05.12
|
|
| Circulation function: | |||
| Bug Depends on: | |||
| Bug Blocks: | 25551 | ||
| Attachments: |
Bug 25531: Add tests
Bug 25531: Debar patron if needed when checkin is backdated Bug 25531: Add tests Bug 25531: Add tests Bug 25531: Debar patron if needed when checkin is backdated Bug 25531: Add tests Bug 25531: Debar patron if needed when checkin is backdated Bug 24881: [19.11.x] Try to fix random failure from Circulation.t |
||
Created attachment 105030 [details] [review] Bug 25531: Add tests Created attachment 105031 [details] [review] Bug 25531: Debar patron if needed when checkin is backdated If think this case does not apply to real-life, but the logic needs to be fixed. If an item is due now, and AddReturn is called now with a return date in the future, the issue is overdue and the patron must be debarred. However it is not as we compare with now and not the return date Shouldn't the test fail when the implementation isn't applied? My plan was - apply 1st patch (test) - run => fail - apply second patch (fix) - run => ok Created attachment 105064 [details] [review] Bug 25531: Add tests (In reply to Victor Grousset/tuxayo from comment #3) > Shouldn't the test fail when the implementation isn't applied? > My plan was > - apply 1st patch (test) > - run => fail > - apply second patch (fix) > - run => ok Hum yes, it should, and it did for me twice in a row yesterday and assumed it was *always*. Now the adjusted test should fail on each run. Original failure appeared again, Master_U18 #791 Created attachment 105096 [details] [review] Bug 25531: Add tests Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 105097 [details] [review] Bug 25531: Debar patron if needed when checkin is backdated If think this case does not apply to real-life, but the logic needs to be fixed. If an item is due now, and AddReturn is called now with a return date in the future, the issue is overdue and the patron must be debarred. However it is not as we compare with now and not the return date Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> It works :D Created attachment 105102 [details] [review] Bug 25531: Add tests Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 105103 [details] [review] Bug 25531: Debar patron if needed when checkin is backdated If think this case does not apply to real-life, but the logic needs to be fixed. If an item is due now, and AddReturn is called now with a return date in the future, the issue is overdue and the patron must be debarred. However it is not as we compare with now and not the return date Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> I agree the test is a little 'unreal', but the logic is sound and the fix seems sound. Passing QA Nice work everyone! Pushed to master for 20.05 tests do not pass on 19.11.x
FAIL valid
Global symbol "$now" requires explicit package name (did you forget to declare "my $now"?)
Global symbol "$now" requires explicit package name (did you forget to declare "my $now"?)
Global symbol "$now" requires explicit package name (did you forget to declare "my $now"?)
t/db_dependent/Circulation.t had compilation errors.
Not backported to 19.11.x
Created attachment 105168 [details] [review] Bug 24881: [19.11.x] Try to fix random failure from Circulation.t This is a follow-up of bug 22917. (In reply to Joy Nelson from comment #14) > tests do not pass on 19.11.x > FAIL valid > Global symbol "$now" requires explicit package name (did you > forget to declare "my $now"?) > Global symbol "$now" requires explicit package name (did you > forget to declare "my $now"?) > Global symbol "$now" requires explicit package name (did you > forget to declare "my $now"?) > t/db_dependent/Circulation.t had compilation errors. > > > Not backported to 19.11.x Hi Joy, I attached a patch for 19.11.x on bug 24881. The tests should pass after you applied it. pushed to 19.11.x for 19.11.06 Backported to 19.05.x branch for 19.05.12 |
This has been caught by a test failing randomly: 15:41:14 koha_1 | # Failed test 'AddReturn must have debarred the patron' 15:41:14 koha_1 | # at t/db_dependent/Circulation.t line 88. 15:41:14 koha_1 | # got: '' 15:41:14 koha_1 | # expected: '1' 15:41:14 koha_1 | # AddReturn returned message $VAR1 = { 15:41:14 koha_1 | # 'WasReturned' => 1 15:41:14 koha_1 | # }; 15:41:14 koha_1 | 15:41:14 koha_1 | # Failed test 'Test at line 2282' 15:41:14 koha_1 | # at t/db_dependent/Circulation.t line 92. 15:41:14 koha_1 | # got: '0' 15:41:14 koha_1 | # expected: '1' 15:41:14 koha_1 | 15:41:14 koha_1 | # Failed test 'Test at line 2282' 15:41:14 koha_1 | # at t/db_dependent/Circulation.t line 95. 15:41:14 koha_1 | # got: undef 15:41:14 koha_1 | # expected: '2020-06-01' 15:41:14 koha_1 | # Looks like you failed 3 tests of 21. 15:41:14 koha_1 | 15:41:14 koha_1 | # Failed test 'AddReturn + suspension_chargeperiod' 15:41:14 koha_1 | # at t/db_dependent/Circulation.t line 2291. 15:41:28 koha_1 | # Looks like you failed 1 test of 48. 15:41:28 koha_1 | [13:41:28] t/db_dependent/Circulation.t To know if an issue is overdue, we compared it with today. But we should compare it with the return date is provided