Bug 28950 - serialsUpdate cron does not mark an issue late until the next issue is expected
Summary: serialsUpdate cron does not mark an issue late until the next issue is expected
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Serials (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-09-03 16:54 UTC by Nick Clemens
Modified: 2023-06-08 22:28 UTC (History)
7 users (show)

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


Attachments
Bug 28950: Mark serial late when past grace period (3.71 KB, patch)
2022-05-19 13:55 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 28950: Mark serial late when past grace period (3.66 KB, patch)
2022-09-07 20:09 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 28950: Mark serial late when past grace period (3.80 KB, patch)
2022-09-12 14:49 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 28950: Mark serial late when past grace period (3.87 KB, patch)
2022-10-07 10:12 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 Nick Clemens 2021-09-03 16:54:07 UTC
When running the cron it does this:
1 - Get all issues with a published date + grace period before today
2 - Loop the related subscriptions
3 - Check the next published date for the serial
4 - Compare the next issues publisheddate to today
5 - Mark late/generate the next issue if the next issue should already be published

For a monthly serial with a 30 day grace period, this is usually ok

For a yearly serial this is a problem
1 - Issue expected 2021-07-01, 30 day grace period
2 - As of 2021-08-01 that serial is past the grace period
3 - Next issue is planned for 2022-07-01
4 - Serial not marked late for a year
Comment 1 Nick Clemens 2022-05-19 13:55:36 UTC
Created attachment 135205 [details] [review]
Bug 28950: Mark serial late when past grace period

This script had some oddities:
- There was a LEFT JOIN to subscription, but no error printed if no subscription
  I changed this to a JOIN as there is a constraint, so this shoudl not happen
- Publisheddate was checked after the SQL, moved into query
- We checked for the next issue, and marked late only if that issue was already expected
  This overruled grace period, which should be the mnarker for a serial being late
  The grace period should be extended if you wish to wait for next issue
- If no next published date, we reported an error on the planneddate
- Script without confirm had no reporting
Comment 2 Joonas Kylmälä 2022-09-07 18:45:15 UTC
Doesn't apply.
Comment 3 Nick Clemens 2022-09-07 20:09:36 UTC
Created attachment 140324 [details] [review]
Bug 28950: Mark serial late when past grace period

This script had some oddities:
- There was a LEFT JOIN to subscription, but no error printed if no subscription
  I changed this to a JOIN as there is a constraint, so this shoudl not happen
- Publisheddate was checked after the SQL, moved into query
- We checked for the next issue, and marked late only if that issue was already expected
  This overruled grace period, which should be the mnarker for a serial being late
  The grace period should be extended if you wish to wait for next issue
- If no next published date, we reported an error on the planneddate
- Script without confirm had no reporting
Comment 4 Joonas Kylmälä 2022-09-08 17:11:08 UTC
Nick can you confirm, it seems that now after this patch we will create a new issue to be expected if the current one is late, and then this will continue until we have ran into subscription end date or subscription issue length? I'm just wondering if libraries are prepared for this and haven't set their end dates for subscriptions to the year 2999. The old code seems to have stopped creating these new Expected/Missing entries (not 100% on this...).
Comment 5 Nick Clemens 2022-09-08 20:20:48 UTC
(In reply to Joonas Kylmälä from comment #4)
> Nick can you confirm, it seems that now after this patch we will create a
> new issue to be expected if the current one is late, and then this will
> continue until we have ran into subscription end date or subscription issue
> length? I'm just wondering if libraries are prepared for this and haven't
> set their end dates for subscriptions to the year 2999. The old code seems
> to have stopped creating these new Expected/Missing entries (not 100% on
> this...).

I don't believe this changes for how long we wil generate issues, but only when we will mark them late.

In either case we check the next published date for generating the new, but prior to this patch it also determines when we mark it late - after this patch we will call the same routine to generate the next, we will just do it after the grace period using the expected date, not using the next planned date
Comment 6 Joonas Kylmälä 2022-09-12 14:49:52 UTC
Created attachment 140462 [details] [review]
Bug 28950: Mark serial late when past grace period

This script had some oddities:
- There was a LEFT JOIN to subscription, but no error printed if no subscription
  I changed this to a JOIN as there is a constraint, so this shoudl not happen
- Publisheddate was checked after the SQL, moved into query
- We checked for the next issue, and marked late only if that issue was already expected
  This overruled grace period, which should be the mnarker for a serial being late
  The grace period should be extended if you wish to wait for next issue
- If no next published date, we reported an error on the planneddate
- Script without confirm had no reporting

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Comment 7 Joonas Kylmälä 2022-09-12 14:51:36 UTC
(In reply to Nick Clemens from comment #5)
> (In reply to Joonas Kylmälä from comment #4)
> > Nick can you confirm, it seems that now after this patch we will create a
> > new issue to be expected if the current one is late, and then this will
> > continue until we have ran into subscription end date or subscription issue
> > length? I'm just wondering if libraries are prepared for this and haven't
> > set their end dates for subscriptions to the year 2999. The old code seems
> > to have stopped creating these new Expected/Missing entries (not 100% on
> > this...).
> 
> I don't believe this changes for how long we wil generate issues, but only
> when we will mark them late.

Thanks, you are right, I was too lazy to test this, but now did so and indeed, we are generating with and without this patch the new expected issues until we reach the end of the subscription.

Signing off.
Comment 8 Martin Renvoize 2022-10-07 10:12:29 UTC
Created attachment 141487 [details] [review]
Bug 28950: Mark serial late when past grace period

This script had some oddities:
- There was a LEFT JOIN to subscription, but no error printed if no subscription
  I changed this to a JOIN as there is a constraint, so this shoudl not happen
- Publisheddate was checked after the SQL, moved into query
- We checked for the next issue, and marked late only if that issue was already expected
  This overruled grace period, which should be the mnarker for a serial being late
  The grace period should be extended if you wish to wait for next issue
- If no next published date, we reported an error on the planneddate
- Script without confirm had no reporting

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Martin Renvoize 2022-10-07 10:13:30 UTC
All working as described, no regressions found and QA script is happy.

This is the sort of thing I'd love to see move into a module and get unit tested.. but as it's all in the script at the moment, there's no precedent for tests here.

Passing QA
Comment 10 Tomás Cohen Arazi 2022-10-11 14:21:11 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!