[Bug 9505] New: acqui/invoices.pl contains an unnecessary loop
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9505 Bug ID: 9505 Summary: acqui/invoices.pl contains an unnecessary loop Classification: Unclassified Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Acquisitions Assignee: koha-bugs@lists.koha-community.org Reporter: colin.campbell@ptfs-europe.com Script retrieves a list of invoices it loops through the list to produce a new bu identical list. While the execution time wasted is trivial it obscures the purpose of the code -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9505 Colin Campbell <colin.campbell@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Assignee|koha-bugs@lists.koha-commun |colin.campbell@ptfs-europe. |ity.org |com --- Comment #1 from Colin Campbell <colin.campbell@ptfs-europe.com> --- Created attachment 14942 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=14942&action=edit Proposed Patch Testing: Should not change functionality of searching invoices and displaying results. Refactoring is for readability and maintainability of code -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9505 Colin Campbell <colin.campbell@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #14942|0 |1 is obsolete| | --- Comment #2 from Colin Campbell <colin.campbell@ptfs-europe.com> --- Created attachment 14943 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=14943&action=edit Revised patch The commit message mentioned cleaning the name space but that change wasnt included in the patch. Amended accordingly -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9505 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |gmcharlt@gmail.com Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9505 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9505 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #14943|0 |1 is obsolete| | --- Comment #3 from Galen Charlton <gmcharlt@gmail.com> --- Created attachment 14963 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=14963&action=edit bug 9505 refactor loops in invoices.pl - Remove an unnecessary loop where output just recreated input. - Remove unnecessary temp variables that obscure code purpose. - Call the variable containing invoices, invoices rather than anonymous and ambiguous results reflect namechange in template. - Lists are passed to template as array refs; declare them as scalars as that is how we use them. - No need to introduce the whole namespace of some C4 modules for 1 routine. Test plan: Note that this patch should not change any visible behavior. [1] Open the invoice search page. [2] Verify that the list of suppliers in the drop-down on the search form is complete. [3] Verify that the list of libraries in the drop-down on the saerch form is complete. [4] Perform a search. Verify that the list of invoices is correct. Signed-off-by: Galen Charlton <gmc@esilibrary.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9505 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Patch doesn't apply CC| |jonathan.druart@biblibre.co | |m --- Comment #4 from Jonathan Druart <jonathan.druart@biblibre.com> --- Patch does not apply: CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/acqui/invoices.tt CONFLICT (content): Merge conflict in acqui/invoices.pl -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9505 Colin Campbell <colin.campbell@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #14963|0 |1 is obsolete| | --- Comment #5 from Colin Campbell <colin.campbell@ptfs-europe.com> --- Created attachment 18354 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=18354&action=edit Amended Patch Updated patch to apply on current master -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9505 Colin Campbell <colin.campbell@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff --- Comment #6 from Colin Campbell <colin.campbell@ptfs-europe.com> --- Reset status, Patch should now apply. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9505 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #7 from Galen Charlton <gmcharlt@gmail.com> --- Tested and confirmed that my signoff is still good. Setting status accordingly. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9505 --- Comment #8 from Jonathan Druart <jonathan.druart@biblibre.com> --- Comment on attachment 18354 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=18354 Amended Patch Review of attachment 18354: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=9505&attachment=18354) ----------------------------------------------------------------- ::: koha-tmpl/intranet-tmpl/prog/en/modules/acqui/invoices.tt @@ +92,4 @@
[% IF ( invoicenumber ) %] <li>Invoice no.: [% invoicenumber %]</li> [% END %] + [% IF booksellerid %]
I think this test can be removed. I don't find anywhere supplier was passed. Or perhaps should we check the supplierid variable. It is passed when we come from the acqui/supplier.pl page. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9505 --- Comment #9 from Colin Campbell <colin.campbell@ptfs-europe.com> --- (In reply to comment #8)
Comment on attachment 18354 [details] [review] Amended Patch
Review of attachment 18354 [details] [review]: -----------------------------------------------------------------
::: koha-tmpl/intranet-tmpl/prog/en/modules/acqui/invoices.tt @@ +92,4 @@
[% IF ( invoicenumber ) %] <li>Invoice no.: [% invoicenumber %]</li> [% END %] + [% IF booksellerid %]
I think this test can be removed. I don't find anywhere supplier was passed. Or perhaps should we check the supplierid variable. It is passed when we come from the acqui/supplier.pl page.
It gets passed from invoices.pl -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9505 --- Comment #10 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to comment #9)
I think this test can be removed. I don't find anywhere supplier was passed. Or perhaps should we check the supplierid variable. It is passed when we come from the acqui/supplier.pl page.
It gets passed from invoices.pl
Humm... yes :) It looks like I forgot to read the code with the context. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9505 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA QA Contact| |jonathan.druart@biblibre.co | |m --- Comment #11 from Jonathan Druart <jonathan.druart@biblibre.com> --- QA comment: All look good. No regression found. Marked as Passed QA. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9505 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #18354|0 |1 is obsolete| | --- Comment #12 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 18505 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=18505&action=edit bug 9505 refactor loops in invoices.pl - Remove an unnecessary loop where output just recreated input. - Remove unnecessary temp variables that obscure code purpose. - Call the variable containing invoices, invoices rather than anonymous and ambiguous results reflect namechange in template. - Lists are passed to template as array refs; declare them as scalars as that is how we use them. - No need to introduce the whole namespace of some C4 modules for 1 routine. Test plan: Note that this patch should not change any visible behavior. [1] Open the invoice search page. [2] Verify that the list of suppliers in the drop-down on the search form is complete. [3] Verify that the list of libraries in the drop-down on the saerch form is complete. [4] Perform a search. Verify that the list of invoices is correct. Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9505 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #13 from Galen Charlton <gmcharlt@gmail.com> --- Pushed to master. Thanks, Colin! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9505 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |10794 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org