Bug 24543 - Wrong test in api/v1/checkouts.t
Summary: Wrong test in api/v1/checkouts.t
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 13895
Blocks:
  Show dependency treegraph
 
Reported: 2020-01-30 10:50 UTC by Jonathan Druart
Modified: 2021-06-14 21:28 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.03, 19.05.08


Attachments
Bug 24543: Fix wrong test in api/v1/checkouts.t (2.41 KB, patch)
2020-01-30 10:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24543: Fix wrong test in api/v1/checkouts.t (2.56 KB, patch)
2020-01-30 13:52 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 24543: Fix wrong test in api/v1/checkouts.t (2.56 KB, patch)
2020-01-30 13:53 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 24543: Fix wrong test in api/v1/checkouts.t (2.62 KB, patch)
2020-01-30 14:53 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2020-01-30 10:50:53 UTC
In t/db_dependent/api/v1/checkouts.t we define a circulation rule with renewalperiod=7. But then the expected due date is today+14 days.
However, at the beginning of the script, the due_date of the issue is today+14 days.

That highlight that the renewal period is not taken into account.

The circulation rule is created with renewalperiod and renewalsallowed, however GetLoanLength check the existence of issuelength to return the rule. GetLoanLength finally return the default rule, with renewalperiod=0
Comment 1 Jonathan Druart 2020-01-30 10:53:17 UTC
Created attachment 98155 [details] [review]
Bug 24543: Fix wrong test in api/v1/checkouts.t

In t/db_dependent/api/v1/checkouts.t we define a circulation rule with renewalperiod=7.
But then the expected due date is today+14 days.
However, at the beginning of the script, the due_date of the issue is today+14 days.

That highlight that the renewal period is not taken into account.

The circulation rule is created with renewalperiod and renewalsallowed,
however GetLoanLength check the existence of issuelength to return the rule.
GetLoanLength finally return the default rule, with renewalperiod=0

Note that this has been found working on bug 18936, code will be cleaned on that patchset.

Test plan:
 % prove t/db_dependent/api/v1/checkouts.t
must return green before and after this patch
Comment 2 Joonas Kylmälä 2020-01-30 13:52:33 UTC
Created attachment 98158 [details] [review]
Bug 24543: Fix wrong test in api/v1/checkouts.t

In t/db_dependent/api/v1/checkouts.t we define a circulation rule with renewalperiod=7.
But then the expected due date is today+14 days.
However, at the beginning of the script, the due_date of the issue is today+14 days.

That highlight that the renewal period is not taken into account.

The circulation rule is created with renewalperiod and renewalsallowed,
however GetLoanLength check the existence of issuelength to return the rule.
GetLoanLength finally return the default rule, with renewalperiod=0

Note that this has been found working on bug 18936, code will be cleaned on that patchset.

Test plan:
 % prove t/db_dependent/api/v1/checkouts.t
must return green before and after this patch

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 3 Joonas Kylmälä 2020-01-30 13:53:33 UTC
Created attachment 98159 [details] [review]
Bug 24543: Fix wrong test in api/v1/checkouts.t

In t/db_dependent/api/v1/checkouts.t we define a circulation rule with renewalperiod=7.
But then the expected due date is today+14 days.
However, at the beginning of the script, the due_date of the issue is today+14 days.

That highlight that the renewal period is not taken into account.

The circulation rule is created with renewalperiod and renewalsallowed,
however GetLoanLength check the existence of issuelength to return the rule.
GetLoanLength finally return the default rule, with renewalperiod=0

Note that this has been found working on bug 18936, code will be cleaned on that patchset.

Test plan:
 % prove t/db_dependent/api/v1/checkouts.t
must return green before and after this patch

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 4 Martin Renvoize 2020-01-30 14:53:43 UTC
Created attachment 98163 [details] [review]
Bug 24543: Fix wrong test in api/v1/checkouts.t

In t/db_dependent/api/v1/checkouts.t we define a circulation rule with renewalperiod=7.
But then the expected due date is today+14 days.
However, at the beginning of the script, the due_date of the issue is today+14 days.

That highlight that the renewal period is not taken into account.

The circulation rule is created with renewalperiod and renewalsallowed,
however GetLoanLength check the existence of issuelength to return the rule.
GetLoanLength finally return the default rule, with renewalperiod=0

Note that this has been found working on bug 18936, code will be cleaned on that patchset.

Test plan:
 % prove t/db_dependent/api/v1/checkouts.t
must return green before and after this patch

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 5 Martin Renvoize 2020-01-30 14:53:57 UTC
Good catch, passing QA
Comment 6 Martin Renvoize 2020-01-30 15:46:08 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 7 Joy Nelson 2020-02-10 23:29:40 UTC
Pushed to 19.11.x branch for 19.11.03
Comment 8 Lucas Gass 2020-02-14 20:42:05 UTC
backported to 19.05.x for 19.05.08