Trying to test this today we found that some of the reporting features of the cronjob seem to be problematic: To recreate: 1 - Create some rotas 2 - Add some items 3 - run cronjob with -x 4 - in sql UPDATE branchtransfers SET datearrived=DATE_SUB(CURDATE(), INTERVAL 1 DAY) WHERE comments like '%rota%' 5 - run the cornjob without -x: perl misc/cronjobs/stockrotation.pl -r full 6 - get an error about undefined emails 7 - run it for branch emails perl misc/cronjobs/stockrotation.pl -r email 8 - get an error about odd numebr of elements in hash
Created attachment 86923 [details] [review] Bug 22566: Fix some problems
(In reply to Nick Clemens from comment #1) > Created attachment 86923 [details] [review] [review] > Bug 22566: Fix some problems After this patch you shoudl eb able to get some output with -r full, however, -r email says no branches have actionable items - the GetPreparedLetter call seems wrong, we aren't passing substitutions I think, but I am not sure exactly what we must pass here
It seems that SR_SLIP has never worked. The template expects branch.items, where does this method could come from?
Created attachment 90421 [details] [review] Bug 22566: Fix some problems Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 90422 [details] [review] Bug 22566: Fix some more issues 1) Fix warning for undefined value in sprintf when no items have the indemand flag set. 2) Corrections to report_full for bad ternary in string construction 3) Perldoc corrections Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 90423 [details] [review] Bug 22566: Fix malformed notice for stockrotation The default notice for stockrotation was malformed. This patch replaces it with a working one. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This is upsetting.. I think it's a case of the originally working implementation getting destroyed during the QA process and not properly tested after the massive number of QA followups. I'd appreciate some thorough testing of the cronjob reporting/dry-run functionality with these patches applied. Test plan as above.. with patches applied the 'full' and 'email' reports should both output meaningful data.
Created attachment 90433 [details] [review] Bug 22566: Fix some problems Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nadine Pierre <nadine.pierre@inLibro.com>
Created attachment 90434 [details] [review] Bug 22566: Fix some more issues 1) Fix warning for undefined value in sprintf when no items have the indemand flag set. 2) Corrections to report_full for bad ternary in string construction 3) Perldoc corrections Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nadine Pierre <nadine.pierre@inLibro.com>
Created attachment 90435 [details] [review] Bug 22566: Fix malformed notice for stockrotation The default notice for stockrotation was malformed. This patch replaces it with a working one. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nadine Pierre <nadine.pierre@inLibro.com>
Created attachment 90742 [details] [review] Bug 22566: Fix some problems Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 90743 [details] [review] Bug 22566: Fix some more issues 1) Fix warning for undefined value in sprintf when no items have the indemand flag set. 2) Corrections to report_full for bad ternary in string construction 3) Perldoc corrections Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 90744 [details] [review] Bug 22566: Fix malformed notice for stockrotation The default notice for stockrotation was malformed. This patch replaces it with a working one. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 90745 [details] [review] Bug 22566: Accept 'all' in the branch params
Created attachment 90746 [details] [review] Bug 22566: Rename 'report_email' to 'report_by_branch' To clarify the intention of this routine, lets rename it
Created attachment 90747 [details] [review] Bug 22566: Clarify intent of reports and add warnings
Created attachment 90748 [details] [review] Bug 22566: Fix some problems Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 90749 [details] [review] Bug 22566: Fix some more issues 1) Fix warning for undefined value in sprintf when no items have the indemand flag set. 2) Corrections to report_full for bad ternary in string construction 3) Perldoc corrections Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 90750 [details] [review] Bug 22566: Accept 'all' in the branch params
Created attachment 90751 [details] [review] Bug 22566: Rename 'report_email' to 'report_by_branch' To clarify the intention of this routine, lets rename it
Created attachment 90752 [details] [review] Bug 22566: Clarify intent of reports and add warnings
Created attachment 90753 [details] [review] Bug 22566: Add 'branch' key to GetPreparedLetter The branch key was missing from the subsitution call to GetPreparedLetter.
Created attachment 90989 [details] [review] Bug 22566: Fix some problems Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 90990 [details] [review] Bug 22566: Fix some more issues 1) Fix warning for undefined value in sprintf when no items have the indemand flag set. 2) Corrections to report_full for bad ternary in string construction 3) Perldoc corrections Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 90991 [details] [review] Bug 22566: Accept 'all' in the branch params Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 90992 [details] [review] Bug 22566: Rename 'report_email' to 'report_by_branch' To clarify the intention of this routine, lets rename it Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 90993 [details] [review] Bug 22566: Clarify intent of reports and add warnings Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 90994 [details] [review] Bug 22566: Add 'branch' key to GetPreparedLetter The branch key was missing from the subsitution call to GetPreparedLetter. Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 90995 [details] [review] Bug 22566: (QA follow-up) Fix pod complaint
I think there is still more work to be done here, but we need guidelines from users to know what this report must show/explain These patches fix errors and allow us to report out on stock rotation, passing QA
Nice work! Pushed to master for 19.11.00
Pushed to 19.05.x for 19.05.03
backported for 18.11.x for 18.11.09