From C4::Letters::GetPreparedLetter: my $tables = $params{tables}; my $substitute = $params{substitute}; $tables || $substitute || $repeat or carp( "ERROR: nothing to substitute - both 'tables' and 'substitute' are empty" ), return; So if the parameter tables or substitute is passed but does not contain anything, it will not warn as expected.
Created attachment 59422 [details] [review] Bug 17970: Add tests to highlight the problem
Created attachment 59423 [details] [review] Bug 17970: Fix GetPreparedLetter behavior if nothing to substitute From C4::Letters::GetPreparedLetter: my $tables = $params{tables}; my $substitute = $params{substitute}; $tables || $substitute || $repeat or carp( "ERROR: nothing to substitute - both 'tables' and 'substitute' are empty" ), return; So if the parameter tables or substitute is passed but does not contain anything, it will not warn as expected. Test plan: 1/ Apply the patch with tests 2/ Confirm that they do not pass 3/ Apply this patch 4/ Confirm that the tests now pass
Created attachment 59424 [details] [review] Bug 17970: Add tests to highlight the problem
Created attachment 59425 [details] [review] Bug 17970: Add tests to highlight the problem
Created attachment 59481 [details] [review] Bug 17970: Add tests to highlight the problem Depends on 17958 Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Created attachment 59482 [details] [review] Bug 17970: Fix GetPreparedLetter behavior if nothing to substitute From C4::Letters::GetPreparedLetter: my $tables = $params{tables}; my $substitute = $params{substitute}; $tables || $substitute || $repeat or carp( "ERROR: nothing to substitute - both 'tables' and 'substitute' are empty" ), return; So if the parameter tables or substitute is passed but does not contain anything, it will not warn as expected. Test plan: 1/ Apply the patch with tests 2/ Confirm that they do not pass 3/ Apply this patch 4/ Confirm that the tests now pass Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Created attachment 60976 [details] [review] Bug 17970: Add tests to highlight the problem Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 60977 [details] [review] Bug 17970: Fix GetPreparedLetter behavior if nothing to substitute From C4::Letters::GetPreparedLetter: my $tables = $params{tables}; my $substitute = $params{substitute}; $tables || $substitute || $repeat or carp( "ERROR: nothing to substitute - both 'tables' and 'substitute' are empty" ), return; So if the parameter tables or substitute is passed but does not contain anything, it will not warn as expected. Test plan: 1/ Apply the patch with tests 2/ Confirm that they do not pass 3/ Apply this patch 4/ Confirm that the tests now pass Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Not really sure if we need this. But no blocker. Note that you also have wantlibrarian, which triggers an action but will not be allowed on itself now. And we still replace OPACBaseURL and call _process_tt if we do not pass table/substitute/repeat.. I diagonally checked all calls of GetPreparedLetter and they all use one or more of the parameters in the condition. So no problems expected. The error message does not mention repeat (part of the condition). The POD does not tell me which parameters are mandatory.
Pushed to master for 17.05, thanks Jonathan!
Dependency is not in 16.11.x, so this should not be needed there.