http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=3050 --- Comment #5 from Jonathan Druart <jonathan.druart@biblibre.com> --- Comment on attachment 25306 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=25306 [SIGNED-OFF] Bug 3050 - Add an option to upload scanned invoices Review of attachment 25306: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=3050&attachment=25306) ----------------------------------------------------------------- QA comments: 1/ A couple of errors are raised by the qa script: FAIL Koha/Misc/Files.pm FAIL pod *** WARNING: Verbatim paragraph in NAME section in file Koha/Misc/Files.pm *** ERROR: =item without previous =over in file Koha/Misc/Files.pm FAIL acqui/invoice-files.pl FAIL forbidden patterns forbidden pattern: trailing space char (line 130) 2/ The new module needs unit tests. 3/ And the following: ::: C4/Auth.pm @@ +358,4 @@

EnableBorrowerFiles => C4::Context->preference('EnableBorrowerFiles'), UseKohaPlugins => C4::Context->preference('UseKohaPlugins'), UseCourseReserves => C4::Context->preference("UseCourseReserves"), + AcqEnableFiles => C4::Context->preference('AcqEnableFiles'),

You should check the syspref value in the templates using Koha.Preference('AcqEnableFiles') ::: Koha/Misc/Files.pm @@ +19,5 @@

+# You should have received a copy of the GNU General Public License +# along with Koha; if not, see <http://www.gnu.org/licenses>. + +use Modern::Perl; +use strict; ## not needed if Modern::Perl, but perlcritic complains..

No complain from perlcritic if you define [TestingAndDebugging::RequireUseStrict] equivalent_modules = Modern::Perl in your .perlcriticrc Take a look at the perlcriticrc file in the qa-tools. @@ +70,5 @@

+ date_uploaded + FROM misc_files + WHERE table_tag = ? AND record_id = ? + ORDER BY file_name, date_uploaded + ";

Replace quotes with simple quotes around sql queries. ::: acqui/invoice-files.pl @@ +28,5 @@

+=cut + +use strict; +use warnings; +

use Modern::Perl; @@ +40,5 @@

+ +my $input = new CGI; +my ( $template, $loggedinuser, $cookie, $flags ) = get_template_and_user( + { + template_name => 'acqui/invoice-files.tmpl',

s/tmpl/tt @@ +61,5 @@

+ my $file = $mf->GetFile( id => $file_id ); + + my $fname = $file->{'file_name'}; + my $ftype = $file->{'file_type'}; + if ($input->param('view') && ($ftype =~ /^image\//i || $fname =~ /\.pdf/i)) {

Maybe m|^image/|i is more readable. @@ +103,5 @@

+ else { + my $file_content; + while (<$uploaded_file>) { + $file_content .= $_; + }

I think my $file_content = <$uploaded_file>; does the same thing. ::: koha-tmpl/intranet-tmpl/prog/en/modules/acqui/invoice-files.tt @@ +27,5 @@

+ </div> + [% END %] + + [% IF ( files ) %] + <table>

Please use Datatable for this table. @@ +64,5 @@

+ <legend>Upload New File</legend> + <ol> + <li><input type="hidden" name="op" value="upload" /> + <input type="hidden" name="invoiceid" value="[% invoiceid %]" /> + <input type="hidden" name="MAX_FILE_SIZE" value="9000000" />

This variable is not retrieve in the pl file. Is it useless? ::: koha-tmpl/intranet-tmpl/prog/en/modules/acqui/invoice.tt @@ +172,5 @@

[% END %] + [% IF ( AcqEnableFiles && files ) %] + <br> + <h2>Files attached to invoice</h2> + <table>

Please use Datatable for this table. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.