Summary: | C4::Serials::getroutinglist returns unnecessary variable | ||
---|---|---|---|
Product: | Koha | Reporter: | Colin Campbell <colin.campbell> |
Component: | Serials | Assignee: | Colin Campbell <colin.campbell> |
Status: | CLOSED FIXED | QA Contact: | Bugs List <koha-bugs> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | chris, koha.sekjal, paul.poulain |
Version: | 3.8 | ||
Hardware: | All | ||
OS: | All | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | --- | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: | Circulation function: | ||
Attachments: |
Proposed Patch
Modified Patch Bug 6790: Remove unnecessary variable from getroutinglist return |
Description
Colin Campbell
2011-08-26 15:16:13 UTC
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. |