Bug 17159 - Cron: whats the use of --noemail cleanup_database.pl --mail
Summary: Cron: whats the use of --noemail cleanup_database.pl --mail
Status: CLOSED WONTFIX
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-08-22 13:34 UTC by Marcel de Rooy
Modified: 2017-01-03 15:47 UTC (History)
1 user (show)

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


Attachments
Bug 17159: Do not conditionally run cleanup_database with --mail (1.67 KB, patch)
2016-08-24 14:29 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17159: Add --mail to the cron example file too (1.27 KB, patch)
2016-08-24 14:33 UTC, Marcel de Rooy
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 2016-08-22 13:34:15 UTC
We have two lines for cleanup_database in cron.
koha-foreach --enabled /usr/share/koha/bin/cronjobs/cleanup_database.pl --sessions --zebraqueue 10 --list-invites
koha-foreach --enabled --noemail /usr/share/koha/bin/cronjobs/cleanup_database.pl --mail

The second line contains an noemail parameter that seems to be unused.
What is it doing there ?
Comment 1 Marcel de Rooy 2016-08-24 14:22:05 UTC
Hm noemail is picked up by koha-list
So it says something like: if you did not enable email, call cleanup with mail option. Some kind of contradiction in itself huh?
Comment 2 Marcel de Rooy 2016-08-24 14:29:45 UTC
Created attachment 54831 [details] [review]
Bug 17159: Do not conditionally run cleanup_database with --mail

As a simplification, I propose here to just call cleanup_database in
only one line. And not test the mail enabled/disabled flag for the --mail
option. If we have no lines to delete, that's just fine and we can go
home early :)
Comment 3 Marcel de Rooy 2016-08-24 14:33:58 UTC
Created attachment 54832 [details] [review]
Bug 17159: Add --mail to the cron example file too

It was not even there yet.

Test plan:
Verify if this change looks good.
Comment 4 Chris Cormack 2016-09-05 10:58:00 UTC
Hmm I am not sure this a safe change.

The first run cleans up the sessions, and zebraqueue for all enabled instances.
The second run deletes rows from the mailqueue for instances that don't have email enabled.

We don't want to delete them from the ones that do send email, because then we dont have a record of what notices were sent to people.

This will now delete them
Comment 5 Marcel de Rooy 2016-09-05 11:07:07 UTC
(In reply to Chris Cormack from comment #4)
> Hmm I am not sure this a safe change.
> 
> The first run cleans up the sessions, and zebraqueue for all enabled
> instances.
> The second run deletes rows from the mailqueue for instances that don't have
> email enabled.
> 
> We don't want to delete them from the ones that do send email, because then
> we dont have a record of what notices were sent to people.
> 
> This will now delete them

That is the purpose of the job. After a specified number of days (say 30) we are not interested in keeping all those records. And after all, having a record that some notice was produced does not 'prove' that the client actually received it.
Comment 6 Chris Cormack 2016-09-05 11:10:59 UTC
No, this cron is set up the way it is by design. 

/cgi-bin/koha/members/notices.pl  shows you the notices sent to borrowers. If you change this cron job, I will tell all the librarians who start complaining to me to call you :-)