Bug 18607 - Fix date calculations for monthly frequencies in Serials
Summary: Fix date calculations for monthly frequencies in Serials
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Serials (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Keywords:
Depends on: 18356
Blocks: 18697
  Show dependency treegraph
 
Reported: 2017-05-15 13:27 UTC by Marcel de Rooy
Modified: 2018-12-03 20:04 UTC (History)
5 users (show)

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


Attachments
Bug 18607: Fix date calculations for monthly frequencies in Serials (5.71 KB, patch)
2017-05-30 12:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18607: Adjusting unit tests for monthly serial frequencies (5.41 KB, patch)
2017-05-30 12:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18607: Fix date calculations for monthly frequencies in Serials (6.08 KB, patch)
2017-05-30 12:53 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18607: Adjusting unit tests for monthly serial frequencies (5.41 KB, patch)
2017-05-30 12:53 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18607: Fix date calculations for monthly frequencies in Serials (4.36 KB, patch)
2017-05-31 14:36 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18607: Adjusting unit tests for monthly serial frequencies (5.55 KB, patch)
2017-05-31 14:36 UTC, Marcel de Rooy
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 18607: Fix date calculations for monthly frequencies in Serials (4.39 KB, patch)
2017-06-06 07:40 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 18607: Adjusting unit tests for monthly serial frequencies (5.58 KB, patch)
2017-06-06 07:40 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18607: Fix date calculations for monthly frequencies in Serials (4.43 KB, patch)
2017-06-15 13:06 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18607: Adjusting unit tests for monthly serial frequencies (5.62 KB, patch)
2017-06-15 13:08 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2017-05-15 13:27:44 UTC
Similar to bug 18356
Comment 1 Marcel de Rooy 2017-05-30 12:39:39 UTC Comment hidden (obsolete)
Comment 2 Marcel de Rooy 2017-05-30 12:39:42 UTC Comment hidden (obsolete)
Comment 3 Marcel de Rooy 2017-05-30 12:53:02 UTC Comment hidden (obsolete)
Comment 4 Marcel de Rooy 2017-05-30 12:53:05 UTC Comment hidden (obsolete)
Comment 5 Marcel de Rooy 2017-05-31 14:36:46 UTC
Created attachment 63873 [details] [review]
Bug 18607: Fix date calculations for monthly frequencies in Serials

Similarly to the solution of bug 18356, this patch fixes the date
calculation for monthly frequencies.

The calculation in GetFictiveIssueNumber now makes use of the new
_delta_units sub introduced on bug 18356.

The calculation in _get_next_date_month is also very similar to the one
in _get_next_date_year. I do not merge them here, but this could still
be considered later on. At least consistency is achieved now between
both routines. The connection with firstacquidate has been cut thru
just like for year units.

Test plan:
[1] Without this patch, look at the prediction pattern for a
    subscription with first issue on Feb 21 and 5 per month. The first
    issues will be 21, 22, 23, 24, 25. Then jumping to 21, 23, 25, etc.
[2] Apply the patch. Look at the same prediction pattern. You will now
    see 6 day intervals and a new cycle starting on the 21st.
    So Feb 21, 27, Mar 5, 11, 17 and Mar 21, 27, etc.
[3] Edit an subscription. Try a few other monthly frequencies.
[4] The next patch adjusts related unit tests.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 6 Marcel de Rooy 2017-05-31 14:36:50 UTC
Created attachment 63874 [details] [review]
Bug 18607: Adjusting unit tests for monthly serial frequencies

The changes in the first patch require some corrections as well as
additional test cases.

GetNextDate.t: Since the calculation for multiple issues per unit has
slightly changed, a few dates (day 15 or day 16) have been changed in the
unit test; when we use 2/month, the algorithm now always adds 15 days.
Added a few test descriptions in this regard too.

GetFictiveIssueNumber.t: Add the monthly subtest with two test cases. In
the first case we tests multiple units per issue, and in the second case
we test multiple issues per unit (month).

Test plan:
[1] Run t/db_dependent/Serials/GetFictiveIssueNumber.t
[2] Run t/db_dependent/Serials/GetNextDate.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Josef Moravec 2017-06-06 07:40:41 UTC
Created attachment 64023 [details] [review]
[SIGNED-OFF] Bug 18607: Fix date calculations for monthly frequencies in Serials

Similarly to the solution of bug 18356, this patch fixes the date
calculation for monthly frequencies.

The calculation in GetFictiveIssueNumber now makes use of the new
_delta_units sub introduced on bug 18356.

The calculation in _get_next_date_month is also very similar to the one
in _get_next_date_year. I do not merge them here, but this could still
be considered later on. At least consistency is achieved now between
both routines. The connection with firstacquidate has been cut thru
just like for year units.

Test plan:
[1] Without this patch, look at the prediction pattern for a
    subscription with first issue on Feb 21 and 5 per month. The first
    issues will be 21, 22, 23, 24, 25. Then jumping to 21, 23, 25, etc.
[2] Apply the patch. Look at the same prediction pattern. You will now
    see 6 day intervals and a new cycle starting on the 21st.
    So Feb 21, 27, Mar 5, 11, 17 and Mar 21, 27, etc.
[3] Edit an subscription. Try a few other monthly frequencies.
[4] The next patch adjusts related unit tests.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 8 Josef Moravec 2017-06-06 07:40:44 UTC
Created attachment 64024 [details] [review]
[SIGNED-OFF] Bug 18607: Adjusting unit tests for monthly serial frequencies

The changes in the first patch require some corrections as well as
additional test cases.

GetNextDate.t: Since the calculation for multiple issues per unit has
slightly changed, a few dates (day 15 or day 16) have been changed in the
unit test; when we use 2/month, the algorithm now always adds 15 days.
Added a few test descriptions in this regard too.

GetFictiveIssueNumber.t: Add the monthly subtest with two test cases. In
the first case we tests multiple units per issue, and in the second case
we test multiple issues per unit (month).

Test plan:
[1] Run t/db_dependent/Serials/GetFictiveIssueNumber.t
[2] Run t/db_dependent/Serials/GetNextDate.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 9 Kyle M Hall 2017-06-15 13:06:47 UTC
Created attachment 64315 [details] [review]
Bug 18607: Fix date calculations for monthly frequencies in Serials

Similarly to the solution of bug 18356, this patch fixes the date
calculation for monthly frequencies.

The calculation in GetFictiveIssueNumber now makes use of the new
_delta_units sub introduced on bug 18356.

The calculation in _get_next_date_month is also very similar to the one
in _get_next_date_year. I do not merge them here, but this could still
be considered later on. At least consistency is achieved now between
both routines. The connection with firstacquidate has been cut thru
just like for year units.

Test plan:
[1] Without this patch, look at the prediction pattern for a
    subscription with first issue on Feb 21 and 5 per month. The first
    issues will be 21, 22, 23, 24, 25. Then jumping to 21, 23, 25, etc.
[2] Apply the patch. Look at the same prediction pattern. You will now
    see 6 day intervals and a new cycle starting on the 21st.
    So Feb 21, 27, Mar 5, 11, 17 and Mar 21, 27, etc.
[3] Edit an subscription. Try a few other monthly frequencies.
[4] The next patch adjusts related unit tests.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 10 Kyle M Hall 2017-06-15 13:08:11 UTC
Created attachment 64316 [details] [review]
Bug 18607: Adjusting unit tests for monthly serial frequencies

The changes in the first patch require some corrections as well as
additional test cases.

GetNextDate.t: Since the calculation for multiple issues per unit has
slightly changed, a few dates (day 15 or day 16) have been changed in the
unit test; when we use 2/month, the algorithm now always adds 15 days.
Added a few test descriptions in this regard too.

GetFictiveIssueNumber.t: Add the monthly subtest with two test cases. In
the first case we tests multiple units per issue, and in the second case
we test multiple issues per unit (month).

Test plan:
[1] Run t/db_dependent/Serials/GetFictiveIssueNumber.t
[2] Run t/db_dependent/Serials/GetNextDate.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 11 Jonathan Druart 2017-06-19 19:38:09 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 12 Fridolin Somers 2017-06-29 07:11:20 UTC
Pushed to 17.05.x, will be in 17.05.02
Comment 13 Katrin Fischer 2017-06-29 20:35:15 UTC
These patches have been pushed to 16.11.x and will be in 16.11.10.
Comment 14 Mason James 2017-07-31 11:32:05 UTC
Pushed to 16.05.x, for 16.05.15 release