There is superfluous use of sprintf within the stockrotation cronjob and it looks like at least one use got rebased incorrectly and was not caught by testing or qa.
Created attachment 80934 [details] [review] Bug 21620: Prevent stockrotation cronjob failures Superflous use of sprintf combined with rebasing issue left the stockrotation cronjob such that a 'No action taken' email report would catastrophically fail with the error: Redundant argument in sprintf at ./stockrotation.pl line 316 This patch both resolves the error and cleans up some additional unrequired sprintf usage. Test Plan 1) Create a rotation plan 2) Add some items to the rotation plan 3) Run the cronjob using `stockrotation.pl --report=mail --send-email --execute` 4) Note the lack of errors
Created attachment 80935 [details] [review] Bug 21620: Prevent stockrotation cronjob failures Superflous use of sprintf combined with rebasing issue left the stockrotation cronjob such that a 'No action taken' email report would catastrophically fail with the error: Redundant argument in sprintf at ./stockrotation.pl line 316 This patch both resolves the error and cleans up some additional unrequired sprintf usage. Test Plan 1) Create a rotation plan 2) Add some items to the rotation plan 3) Run the cronjob using `stockrotation.pl --report=mail --send-email --execute` 4) Note the lack of errors Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
I cannot test this because of bug 11897 comment 366: % perl misc/cronjobs/stockrotation.pl --report=email --send-email --execute ERROR PROCESSING TEMPLATE: file error - parse error - input text line 5: unexpected token (ne) [% IF item.reason ne 'in-demand' %] at misc/cronjobs/stockrotation.pl line 343. Also note that the test plan is wrong, the correct value for --report is 'email', not 'mail'
Created attachment 81055 [details] [review] Bug 21620: Prevent stockrotation cronjob failures Superflous use of sprintf combined with rebasing issue left the stockrotation cronjob such that a 'No action taken' email report would catastrophically fail with the error: Redundant argument in sprintf at ./stockrotation.pl line 316 This patch both resolves the error and cleans up some additional unrequired sprintf usage. Test Plan 1) Create a rotation plan 2) Add some items to the rotation plan 3) Run the cronjob using `stockrotation.pl --report=mail --send-email --execute` 4) Note the lack of errors Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Created attachment 81056 [details] [review] Bug 21620: (follow-up) Fix for bad TT syntax in default notice.
Grr... silly template toolkit syntax error.. I think that followup should resolve it.
(In reply to Martin Renvoize from comment #5) > Created attachment 81056 [details] [review] [review] > Bug 21620: (follow-up) Fix for bad TT syntax in default notice. Nope, you should fix other languages as well. Maybe doing it on a separate bug report would be better.
Created attachment 81108 [details] [review] Bug 21620: (follow-up) Fix for bad TT syntax in default notice.
Created attachment 81110 [details] [review] Bug 21620: Prevent stockrotation cronjob failures Superflous use of sprintf combined with rebasing issue left the stockrotation cronjob such that a 'No action taken' email report would catastrophically fail with the error: Redundant argument in sprintf at ./stockrotation.pl line 316 This patch both resolves the error and cleans up some additional unrequired sprintf usage. Test Plan 1) Create a rotation plan 2) Add some items to the rotation plan 3) Run the cronjob using `stockrotation.pl --report=mail --send-email --execute` 4) Note the lack of errors Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Created attachment 81111 [details] [review] Bug 21620: Prevent stockrotation cronjob failures Superflous use of sprintf combined with rebasing issue left the stockrotation cronjob such that a 'No action taken' email report would catastrophically fail with the error: Redundant argument in sprintf at ./stockrotation.pl line 316 This patch both resolves the error and cleans up some additional unrequired sprintf usage. Test Plan 1) Create a rotation plan 2) Add some items to the rotation plan 3) Run the cronjob using `stockrotation.pl --report=email --send-email --execute` 4) Note the lack of errors Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
As requested I've moved the followup patch to a second bug and committed the missing changes to it there.
Created attachment 81872 [details] [review] Bug 21620: Prevent stockrotation cronjob failures Superflous use of sprintf combined with rebasing issue left the stockrotation cronjob such that a 'No action taken' email report would catastrophically fail with the error: Redundant argument in sprintf at ./stockrotation.pl line 316 This patch both resolves the error and cleans up some additional unrequired sprintf usage. Test Plan 1) Create a rotation plan 2) Add some items to the rotation plan 3) Run the cronjob using `stockrotation.pl --report=email --send-email --execute` 4) Note the lack of errors Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Awesome work all! Pushed to master for 18.11
Bug 21656 not in 18.05.x series.