[Koha-bugs] [Bug 3050] Add an option to upload scanned invoices.

bugzilla-daemon at bugs.koha-community.org bugzilla-daemon at bugs.koha-community.org
Fri Mar 21 11:47:17 CET 2014


http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=3050

--- Comment #5 from Jonathan Druart <jonathan.druart at 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.


More information about the Koha-bugs mailing list