Bug 22593 - Cronjobs/Scripts dealing with accountlines need updating for bug 22008
Summary: Cronjobs/Scripts dealing with accountlines need updating for bug 22008
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: master
Hardware: All All
: P5 - low blocker (vote)
Assignee: Martin Renvoize
QA Contact: Josef Moravec
URL:
Keywords:
Depends on: 22008
Blocks: 22518 22577
  Show dependency treegraph
 
Reported: 2019-03-27 12:34 UTC by Martin Renvoize
Modified: 2020-01-06 20:17 UTC (History)
9 users (show)

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


Attachments
Bug 22593: Fix bad ternaries after bug 22008 (5.20 KB, patch)
2019-03-27 13:59 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22593: Fix bad ternaries after bug 22008 (5.27 KB, patch)
2019-03-27 15:04 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 22593: Fix bad ternaries after bug 22008 (5.31 KB, patch)
2019-03-28 08:12 UTC, Josef Moravec
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2019-03-27 12:34:30 UTC
Currently we use 0 to indicate an action performed by command line scripts (userenv is undef, default to 0)

After bug 22008 we must have a real borrowernumber.

If we simply default to 'NULL' we will no longer be able to differentiate fines from command line from those of deleted users
Comment 1 Martin Renvoize 2019-03-27 13:59:11 UTC
Created attachment 87069 [details] [review]
Bug 22593: Fix bad ternaries after bug 22008

A series of ternaries were introduced when we moved to add_debit which
defaulted to 'user 0' should a userenv not be set.  This was incorrect
as userenv may well not be set (during cronscript runs for example) and
the new constraint would not allow such a default. We switch to 'undef'
here to satisfy the constraint.

Test plan
1) Ensure you have data in your system that would be caught by the
longoverdues cronjob.
2) Ensure you're sysprefs are setup to charge for lost items
3) Run the script and varify it runs to completion without errors

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 2 Nick Clemens 2019-03-27 14:40:54 UTC
(In reply to Martin Renvoize from comment #0)
> Currently we use 0 to indicate an action performed by command line scripts
> (userenv is undef, default to 0)
> 
> After bug 22008 we must have a real borrowernumber.
> 
> If we simply default to 'NULL' we will no longer be able to differentiate
> fines from command line from those of deleted users

I believe that we can differentiate deleted users from cron users by the branchcode of the accountline - branch should remain for deleted user - I think if we deleted a branch we would want to delete the accountlines instead of NULL them also, but that is for elsewhere
Comment 3 Kyle M Hall 2019-03-27 15:04:00 UTC
Created attachment 87075 [details] [review]
Bug 22593: Fix bad ternaries after bug 22008

A series of ternaries were introduced when we moved to add_debit which
defaulted to 'user 0' should a userenv not be set.  This was incorrect
as userenv may well not be set (during cronscript runs for example) and
the new constraint would not allow such a default. We switch to 'undef'
here to satisfy the constraint.

Test plan
1) Ensure you have data in your system that would be caught by the
longoverdues cronjob.
2) Ensure you're sysprefs are setup to charge for lost items
3) Run the script and varify it runs to completion without errors

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 4 Josef Moravec 2019-03-28 08:12:03 UTC
Created attachment 87130 [details] [review]
Bug 22593: Fix bad ternaries after bug 22008

A series of ternaries were introduced when we moved to add_debit which
defaulted to 'user 0' should a userenv not be set.  This was incorrect
as userenv may well not be set (during cronscript runs for example) and
the new constraint would not allow such a default. We switch to 'undef'
here to satisfy the constraint.

Test plan
1) Ensure you have data in your system that would be caught by the
longoverdues cronjob.
2) Ensure you're sysprefs are setup to charge for lost items
3) Run the script and varify it runs to completion without errors

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 5 Nick Clemens 2019-03-28 16:00:55 UTC
Awesome work all!

Pushed to master for 19.05
Comment 6 Martin Renvoize 2019-04-01 11:14:47 UTC
Bug with enhancement not in 18.11.x series.