[Koha-bugs] [Bug 24153] Add a confirm flag to the cleanup_database.pl cronjob

bugzilla-daemon at bugs.koha-community.org bugzilla-daemon at bugs.koha-community.org
Tue Jun 23 12:18:22 CEST 2020


https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24153

--- Comment #11 from Jonathan Druart <jonathan.druart at bugs.koha-community.org> ---
(In reply to Marcel de Rooy from comment #10)
> +# FIXME The output for dry-run mode needs to be improved
> Yes. Agreed.

The following line is:
# But non trivial changes to C4::Members need to be done before.

So cannot be done now.

> +    $tokens->delete if $confirm;
> +    say sprintf "Removed %s expired OAuth2 tokens", $count if $verbose;
> This might be confusing. Among other examples.

What do you mean, can you detail? Do you mean the message could be improved
(use of conditional) in dry-run mode?

> * perl misc/cronjobs/cleanup_database.pl -m 10 -v
> Mail queue purge triggered for 10 days.
> Use of uninitialized value $count in concatenation (.) or string at
> misc/cronjobs/cleanup_database.pl line 293.
>  messages were deleted from the mail queue.
> 
> So uninitialized warning that we do not want.
> But this example could serve to illustrate that even the wrong count could
> have been printed. Look at:
>     if ( $confirm ) {
>         $sth->execute($mail) or die $dbh->errstr;
>         $count = $sth->rows;
>     }
>     print "$count messages were deleted from the mail queue.\nDone with
> message_queue purge.\n" if $verbose;
> => Since the count is not set, it could well be the count of a former
> statement !

I fixed the warning.

> I do not think that this is ready for production. Since we do not really
> support testing for many options, I do not see why this patch is useful.

I wrote it for the pseudonymized transactions. The idea was to rework a bit the
whole script and introduce a common pattern. That's why I implemented
filter_by_last_update and use it all over the script.
We now have a base and a pattern to follow. It's not useful yet for all the
options, but at least it is for few of them.

-- 
You are receiving this mail because:
You are watching all bug changes.


More information about the Koha-bugs mailing list