Bug 28950

Summary:	serialsUpdate cron does not mark an issue late until the next issue is expected
Product:	Koha	Reporter:	Nick Clemens (kidclamp) <nick>
Component:	Serials	Assignee:	Nick Clemens (kidclamp) <nick>
Status:	CLOSED FIXED	QA Contact:	Martin Renvoize (ashimema) <martin.renvoize>
Severity:	normal
Priority:	P5 - low	CC:	andrew, caroline.cyr-la-rose, joonas.kylmala, martin.renvoize, patrick.robitaille, sbrown
Version:	Main
Hardware:	All
OS:	All
See Also:	https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27499
Change sponsored?:	---	Patch complexity:	Trivial patch
Documentation contact:		Documentation submission:
Text to go in the release notes:		Version(s) released in:	22.11.00
Circulation function:
Attachments:	Bug 28950: Mark serial late when past grace period Bug 28950: Mark serial late when past grace period Bug 28950: Mark serial late when past grace period Bug 28950: Mark serial late when past grace period

Description Nick Clemens (kidclamp) 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 (kidclamp) 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 (kidclamp) 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 (kidclamp) 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 (ashimema) 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 (ashimema) 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 (tcohen) 2022-10-11 14:21:11 UTC

Pushed to master for 22.11.

Nice work everyone, thanks!