The subroutine getroutinglist returns a "count" variable along with the array. Not only is this unnecessary but it causes the routine to execute an unnecessary loop and results in some ugly, unidiomatic code in the calling cgi. Refactor count away.
Created attachment 5161 [details] [review] Proposed Patch
Your patch use an unsupported form of template style, that may cause problem in translation processing. In 'serials/routing.tt', you wrote: <option[%- IF r == member.ranking -%] selected="selected"[% END %] value="[% r %]">[% r %]</option> The "IF" directive is *in* tag 'option': this kind of construction is unsupported. (but not problem when directive are in attribute, like 'r' with the 'value' attribute). Your should use instead of: [% IF r == member.ranking %] <option selected="selected" value="[% r %]">[% r %]</option> [% ELSE %] <option value="[% r %]">[% r %]</option> [% END %] For more detail about this problem, please see bug 6458.
Created attachment 5217 [details] [review] Modified Patch Modified patch to avoid the bug reported in TTparser.pm
Updating Version : This ENH will be for Koha 3.8
Bug versionned for master. entries will be made against rel_3_8 once the patch has been applied (see thread about that on koha-devel yesterday)
Created attachment 6990 [details] [review] Bug 6790: Remove unnecessary variable from getroutinglist return getroutinglist returns a count variable to indicate how many elements are in the array. This is almost always a serious code smell. (We are programming in a list manipulating language) The routine was executing am unnecessary loop just to maintain that var. Removed the variable from the routine and perldoc refactored calls of the routine removed the c-style loops for more idiomatic and maintainable for loops renamed some opaquely named variables removed a call to the routine where nothing was done with the data moved some html out of the calling script and into the template Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Removes for loops in favour of foreach. Does some minor cleanup to quotes and spacing to clean various coding style violations. Looks solid, marking as Passed QA
QA comment: t/db_dependent/lib/KohaTest/Serials.pm does not work after this patch, but it was not working before the patch, so it's OK. patch pushed
There have been no further reports of problems so I am marking this bug resolved.