Attachment 26724 Details for Bug 3050 – Bug 3050: [QA Followup] Fix various QA issues, plus some minor improvements

[patch] Bug 3050: [QA Followup] Fix various QA issues, plus some minor improvements

Bug-3050-QA-Followup-Fix-various-QA-issues-plus-so.patch (text/plain), 29.47 KB, created by Jacek Ablewicz on 2014-04-01 11:04:44 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jacek Ablewicz

Created: 2014-04-01 11:04:44 UTC

Size: 29.47 KB

patch

obsolete

>From 54b25cec69068cd8023d35cfab953b8ba662de48 Mon Sep 17 00:00:00 2001
>From: Jacek Ablewicz <abl@biblos.pk.edu.pl>
>Date: Tue, 1 Apr 2014 12:35:28 +0200
>Subject: [PATCH] Bug 3050: [QA Followup] Fix various QA issues, plus some
> minor improvements
>
>QA followup for: Bug 3050 - Add an option to upload scanned invoices.
>
>- Fixed various QA issues.
>- Added unit tests for the new module.
>- Improved error handling; extended pod documentation
>in Koha::Misc::Files
>- Attached file records were not properly handled when merging
>invoices; fixed.
>
>Test plan add-ons:
>
>1/ prove t/db_dependent/Koha_Misc_Files.t
>2/ try to merge some invoices with files attached; after merge,
>all files previously attached to individual invoices being
>merged should be attached to resulting invoice (merge destination)
>---
> C4/Auth.pm                                         |    1 -
> Koha/Misc/Files.pm                                 |  172 +++++++++++++++-----
> acqui/invoice-files.pl                             |   35 ++--
> acqui/invoice.pl                                   |   18 +-
> .../prog/en/modules/acqui/invoice-files.tt         |  134 ++++++++-------
> .../intranet-tmpl/prog/en/modules/acqui/invoice.tt |   28 +++-
> t/db_dependent/Koha_Misc_Files.t                   |   87 ++++++++++
> 7 files changed, 334 insertions(+), 141 deletions(-)
> create mode 100755 t/db_dependent/Koha_Misc_Files.t
>
>diff --git a/C4/Auth.pm b/C4/Auth.pm
>index 2708d43..44edf67 100644
>--- a/C4/Auth.pm
>+++ b/C4/Auth.pm
>@@ -358,7 +358,6 @@ sub get_template_and_user {
>             EnableBorrowerFiles         => C4::Context->preference('EnableBorrowerFiles'),
>             UseKohaPlugins              => C4::Context->preference('UseKohaPlugins'),
>             UseCourseReserves            => C4::Context->preference("UseCourseReserves"),
>-            AcqEnableFiles              => C4::Context->preference('AcqEnableFiles'),
>         );
>     }
>     else {
>diff --git a/Koha/Misc/Files.pm b/Koha/Misc/Files.pm
>index b8ed3eb..3896764 100644
>--- a/Koha/Misc/Files.pm
>+++ b/Koha/Misc/Files.pm
>@@ -20,40 +20,74 @@ package Koha::Misc::Files;
> # along with Koha; if not, see <http://www.gnu.org/licenses>.
> 
> use Modern::Perl;
>-use strict; ## not needed if Modern::Perl, but perlcritic complains..
> use vars qw($VERSION);
>-$VERSION = '0.20';
>+$VERSION = '0.23';
> 
> use C4::Context;
> use C4::Output;
> use C4::Dates;
>-use C4::Debug;
> 
> =head1 NAME
> 
>-Koha::Misc::Files - module for managing miscellaneous files
>-associated with records from arbitrary tables
>+Koha::Misc::Files - module for managing miscellaneous files associated
>+with records from arbitrary tables
>+
>+=head1 SYNOPSIS
>+
>+use Koha::Misc::Files;
>+
>+my $mf = Koha::Misc::Files->new( tabletag => $tablename,
>+    recordid => $recordnumber );
>+
>+=head1 FUNCTIONS
>+
>+=over
>+
>+=item new()
>+
>+my $mf = Koha::Misc::Files->new( tabletag => $tablename,
>+    recordid => $recordnumber );
>+
>+Creates new Koha::Misc::Files object. Such object is essentially
>+a pair: in typical usage scenario, 'tabletag' parameter will be
>+a database table name, and 'recordid' an unique record ID number
>+from this table. However, this method does accept an arbitrary
>+string as 'tabletag', and an arbitrary integer as 'recordid'.
>+
>+Particular Koha::Misc::Files object can have one or more file records
>+(actuall file contents + various file metadata) associated with it.
>+
>+In case of an error (wrong parameter format) it returns undef.
> 
> =cut
> 
> sub new {
>     my ( $class, %args ) = @_;
> 
>-    ( defined( $args{'tabletag'} ) && defined( $args{'recordid'} ) )
>+    my $recid = $args{'recordid'};
>+    my $tag   = $args{'tabletag'};
>+    ( defined($tag) && $tag ne '' && defined($recid) && $recid =~ /^\d+$/ )
>       || return ();
>+
>     my $self = bless( {}, $class );
> 
>-    $self->{'table_tag'} = $args{'tabletag'};
>-    $self->{'record_id'} = $args{'recordid'};
>+    $self->{'table_tag'} = $tag;
>+    $self->{'record_id'} = '' . ( 0 + $recid );
> 
>     return $self;
> }
> 
> =item GetFilesInfo()
> 
>-    my $mf = Koha::Misc::Files->new( tabletag => $tablename,
>-        recordid => $recordnumber);
>-    my $files_hashref = $mf->GetFilesInfo
>+my $files_descriptions = $mf->GetFilesInfo();
>+
>+This method returns a reference to an array of hashes
>+containing files metadata (file_id, file_name, file_type,
>+file_description, file_size, date_uploaded) for all file records
>+associated with given $mf object, or an empty arrayref if there are
>+no such records yet.
>+
>+In case of an error it returns undef.
> 
> =cut
> 
>@@ -61,53 +95,69 @@ sub GetFilesInfo {
>     my $self = shift;
> 
>     my $dbh   = C4::Context->dbh;
>-    my $query = "
>+    my $query = '
>         SELECT
>             file_id,
>             file_name,
>             file_type,
>             file_description,
>-            date_uploaded
>+            date_uploaded,
>+            LENGTH(file_content) AS file_size
>         FROM misc_files
>         WHERE table_tag = ? AND record_id = ?
>         ORDER BY file_name, date_uploaded
>-    ";
>+    ';
>     my $sth = $dbh->prepare($query);
>     $sth->execute( $self->{'table_tag'}, $self->{'record_id'} );
>     return $sth->fetchall_arrayref( {} );
> }
> 
> =item AddFile()
>-    my $mf = Koha::Misc::Files->new( tabletag => $tablename,
>-        recordid => $recordnumber);
>-    $mf->AddFile( name => $filename, type => $mimetype, description => $description, content => $content );
>+
>+$mf->AddFile( name => $filename, type => $mimetype,
>+    description => $description, content => $content );
>+
>+Adds a new file (we want to store for / associate with a given
>+object) to the database. Parameters 'name' and 'content' are mandatory.
>+Note: this method would (silently) fail if there is no 'name' given
>+or if the 'content' provided is empty.
>+
> =cut
> 
> sub AddFile {
>     my ( $self, %args ) = @_;
> 
>     my $name        = $args{'name'};
>-    my $type        = $args{'type'};
>+    my $type        = $args{'type'} // '';
>     my $description = $args{'description'};
>     my $content     = $args{'content'};
> 
>-    return unless ( $name && $content );
>+    return unless ( defined($name) && $name ne '' && defined($content) && $content ne '' );
> 
>     my $dbh   = C4::Context->dbh;
>-    my $query = "
>+    my $query = '
>         INSERT INTO misc_files ( table_tag, record_id, file_name, file_type, file_description, file_content )
>         VALUES ( ?,?,?,?,?,? )
>-    ";
>+    ';
>     my $sth = $dbh->prepare($query);
>     $sth->execute( $self->{'table_tag'}, $self->{'record_id'}, $name, $type,
>         $description, $content );
> }
> 
> =item GetFile()
>-    my $mf = Koha::Misc::Files->new( tabletag => $tablename,
>-        recordid => $recordnumber);
>-    ...
>-    my $file = $mf->GetFile( id => $file_id );
>+
>+my $file = $mf->GetFile( id => $file_id );
>+
>+For an individual, specific file ID this method returns a hashref
>+containing all metadata (file_id, table_tag, record_id, file_name,
>+file_type, file_description, file_content, date_uploaded), plus
>+an actuall contents of a file (in 'file_content'). In typical usage
>+scenarios, for a given $mf object, specific file IDs have to be
>+obtained first by GetFilesInfo() call.
>+
>+Returns undef in case when file ID specified as 'id' parameter was not
>+found in the database.
>+
> =cut
> 
> sub GetFile {
>@@ -116,19 +166,21 @@ sub GetFile {
>     my $file_id = $args{'id'};
> 
>     my $dbh   = C4::Context->dbh;
>-    my $query = "
>+    my $query = '
>         SELECT * FROM misc_files WHERE file_id = ? AND table_tag = ? AND record_id = ?
>-    ";
>+    ';
>     my $sth = $dbh->prepare($query);
>     $sth->execute( $file_id, $self->{'table_tag'}, $self->{'record_id'} );
>     return $sth->fetchrow_hashref();
> }
> 
> =item DelFile()
>-    my $mf = Koha::Misc::Files->new( tabletag => $tablename,
>-        recordid => $recordnumber);
>-    ...
>-    $mf->DelFile( id => $file_id );
>+
>+$mf->DelFile( id => $file_id );
>+
>+Deletes specific, individual file record (file contents and metadata)
>+from the database.
>+
> =cut
> 
> sub DelFile {
>@@ -137,38 +189,78 @@ sub DelFile {
>     my $file_id = $args{'id'};
> 
>     my $dbh   = C4::Context->dbh;
>-    my $query = "
>+    my $query = '
>         DELETE FROM misc_files WHERE file_id = ? AND table_tag = ? AND record_id = ?
>-    ";
>+    ';
>     my $sth = $dbh->prepare($query);
>     $sth->execute( $file_id, $self->{'table_tag'}, $self->{'record_id'} );
> }
> 
> =item DelAllFiles()
>-    my $mf = Koha::Misc::Files->new( tabletag => $tablename,
>-        recordid => $recordnumber);
>-    $mf->DelAllFiles;
>+
>+$mf->DelAllFiles();
>+
>+Deletes all file records associated with (stored for) a given $mf object.
>+
> =cut
> 
> sub DelAllFiles {
>     my ($self) = @_;
> 
>     my $dbh   = C4::Context->dbh;
>-    my $query = "
>+    my $query = '
>         DELETE FROM misc_files WHERE table_tag = ? AND record_id = ?
>-    ";
>+    ';
>     my $sth = $dbh->prepare($query);
>     $sth->execute( $self->{'table_tag'}, $self->{'record_id'} );
> }
> 
>+=item MergeFileRecIds()
>+
>+$mf->MergeFileRecIds(@ids_to_be_merged);
>+
>+This method re-associates all individuall file records associated with
>+some "parent" records IDs (provided in @ids_to_be_merged) with the given
>+single $mf object (which would be treated as a "parent" destination).
>+
>+This a helper method; typically it needs to be called only in cases when
>+some "parent" records are being merged in the (external) 'tablename'
>+table.
>+
>+=cut
>+
>+sub MergeFileRecIds {
>+    my ( $self, @ids_to_merge ) = @_;
>+
>+    my $dst_recid = $self->{'record_id'};
>+    @ids_to_merge = map { ( $dst_recid == $_ ) ? () : ($_); } @ids_to_merge;
>+    @ids_to_merge > 0 || return ();
>+
>+    my $dbh   = C4::Context->dbh;
>+    my $query = '
>+        UPDATE misc_files SET record_id = ?
>+        WHERE table_tag = ? AND record_id = ?
>+    ';
>+    my $sth = $dbh->prepare($query);
>+
>+    for my $src_recid (@ids_to_merge) {
>+        $sth->execute( $dst_recid, $self->{'table_tag'}, $src_recid );
>+    }
>+}
>+
> 1;
>+
> __END__
> 
> =back
> 
>+=head1 SEE ALSO
>+
>+Koha::Borrower::Files
>+
> =head1 AUTHOR
> 
>-Kyle M Hall <kyle.m.hall@gmail.com>
>-Jacek Ablewicz <ablewicz@gmail.com>
>+Kyle M Hall E<lt>kyle.m.hall@gmail.comE<gt>,
>+Jacek Ablewicz E<lt>ablewicz@gmail.comE<gt>
> 
> =cut
>diff --git a/acqui/invoice-files.pl b/acqui/invoice-files.pl
>index 73c81b6..635c427 100755
>--- a/acqui/invoice-files.pl
>+++ b/acqui/invoice-files.pl
>@@ -27,21 +27,18 @@ Manage files associated with invoice
> 
> =cut
> 
>-use strict;
>-use warnings;
>+use Modern::Perl;
> 
> use CGI;
> use C4::Auth;
> use C4::Output;
> use C4::Acquisition;
>-use C4::Debug;
>-use Koha::DateUtils;
> use Koha::Misc::Files;
> 
> my $input = new CGI;
> my ( $template, $loggedinuser, $cookie, $flags ) = get_template_and_user(
>     {
>-        template_name   => 'acqui/invoice-files.tmpl',
>+        template_name   => 'acqui/invoice-files.tt',
>         query           => $input,
>         type            => 'intranet',
>         authnotrequired => 0,
>@@ -50,11 +47,12 @@ my ( $template, $loggedinuser, $cookie, $flags ) = get_template_and_user(
>     }
> );
> 
>-my $invoiceid = $input->param('invoiceid');
>+my $invoiceid = $input->param('invoiceid') // '';
> my $op = $input->param('op') // '';
>+my %errors;
> 
>-$template->param( 'invoice_files' => 1 );
> my $mf = Koha::Misc::Files->new( tabletag => 'aqinvoices', recordid => $invoiceid );
>+defined($mf) || do { $op = 'none'; $errors{'invalid_parameter'} = 1; };
> 
> if ( $op eq 'download' ) {
>     my $file_id = $input->param('file_id');
>@@ -62,11 +60,11 @@ if ( $op eq 'download' ) {
> 
>     my $fname = $file->{'file_name'};
>     my $ftype = $file->{'file_type'};
>-    if ($input->param('view') && ($ftype =~ /^image\//i || $fname =~ /\.pdf/i)) {
>+    if ($input->param('view') && ($ftype =~ m|^image/|i || $fname =~ /\.pdf/i)) {
>         $fname =~ /\.pdf/i && do { $ftype='application/pdf'; };
>         print $input->header(
>             -type       => $ftype,
>-            -charset    => 'utf-8',
>+            -charset    => 'utf-8'
>         );
>     } else {
>         print $input->header(
>@@ -86,7 +84,6 @@ else {
>         booksellerid     => $details->{'booksellerid'},
>         datereceived     => $details->{'datereceived'},
>     );
>-    my %errors;
> 
>     if ( $op eq 'upload' ) {
>         my $uploaded_file = $input->upload('uploadfile');
>@@ -96,23 +93,16 @@ else {
>             my $mimetype = $input->uploadInfo($filename)->{'Content-Type'};
> 
>             $errors{'empty_upload'} = 1 if ( -z $uploaded_file );
>-
>-            if (%errors) {
>-                $template->param( errors => %errors );
>-            }
>-            else {
>-                my $file_content;
>-                while (<$uploaded_file>) {
>-                    $file_content .= $_;
>-                }
>-                if ($mimetype =~ /^application\/(force-download|unknown)$/i && $filename =~ /\.pdf$/) {
>+            unless (%errors) {
>+                my $file_content = do { local $/; <$uploaded_file>; };
>+                if ($mimetype =~ /^application\/(force-download|unknown)$/i && $filename =~ /\.pdf$/i) {
>                     $mimetype = 'application/pdf';
>                 }
>                 $mf->AddFile(
>                     name    => $filename,
>                     type    => $mimetype,
>                     content => $file_content,
>-                    description => $input->param('description'),
>+                    description => $input->param('description')
>                 );
>             }
>         }
>@@ -124,9 +114,8 @@ else {
>     }
> 
>     $template->param(
>-        files => $mf->GetFilesInfo(),
>+        files => (defined($mf)? $mf->GetFilesInfo(): undef),
>         errors => \%errors
>     );
>-
>     output_html_with_http_headers $input, $cookie, $template->output;
> }
>diff --git a/acqui/invoice.pl b/acqui/invoice.pl
>index 44a2f66..85a571c 100755
>--- a/acqui/invoice.pl
>+++ b/acqui/invoice.pl
>@@ -52,6 +52,12 @@ my ( $template, $loggedinuser, $cookie, $flags ) = get_template_and_user(
> my $invoiceid = $input->param('invoiceid');
> my $op        = $input->param('op');
> 
>+my $invoice_files;
>+if ( C4::Context->preference('AcqEnableFiles') ) {
>+    $invoice_files = Koha::Misc::Files->new(
>+        tabletag => 'aqinvoices', recordid => $invoiceid );
>+}
>+
> if ( $op && $op eq 'close' ) {
>     CloseInvoice($invoiceid);
>     my $referer = $input->param('referer');
>@@ -87,14 +93,13 @@ elsif ( $op && $op eq 'mod' ) {
>     } elsif ($input->param('merge')) {
>         my @sources = $input->param('merge');
>         MergeInvoices($invoiceid, \@sources);
>+        defined($invoice_files) && $invoice_files->MergeFileRecIds(@sources);
>     }
>     $template->param( modified => 1 );
> }
> elsif ( $op && $op eq 'delete' ) {
>     DelInvoice($invoiceid);
>-    C4::Context->preference('AcqEnableFiles')
>-      && $invoiceid && Koha::Misc::Files->new(
>-        tabletag => 'aqinvoices', recordid => $invoiceid )->DelAllFiles();
>+    defined($invoice_files) && $invoice_files->DelAllFiles();
>     my $referer = $input->param('referer') || 'invoices.pl';
>     if ($referer) {
>         print $input->redirect($referer);
>@@ -213,12 +218,7 @@ $template->param(
>     budgets_loop             => \@budgets_loop,
> );
> 
>-{
>-    C4::Context->preference('AcqEnableFiles') || last;
>-    my $mf = Koha::Misc::Files->new(
>-        tabletag => 'aqinvoices', recordid => $invoiceid );
>-    defined( $mf ) && $template->param( files => $mf->GetFilesInfo() );
>-}
>+defined( $invoice_files ) && $template->param( files => $invoice_files->GetFilesInfo() );
> 
> sub get_infos {
>     my $order      = shift;
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/invoice-files.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/invoice-files.tt
>index 098e175..1121536 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/invoice-files.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/invoice-files.tt
>@@ -4,6 +4,23 @@
> <title>Koha &rsaquo; Acquisitions &rsaquo; Invoice &rsaquo; Files</title>
> <link rel="stylesheet" type="text/css" href="[% themelang %]/css/datatables.css" />
> [% INCLUDE 'doc-head-close.inc' %]
>+[% INCLUDE 'datatables.inc' %]
>+<script type="text/javascript">
>+//<![CDATA[
>+    $(document).ready(function() {
>+        $("#invoice_files_details_table").dataTable($.extend(true, {}, dataTablesDefaults, {
>+            "aoColumnDefs": [
>+                { "aTargets": [ -1, -2 ], "bSortable": false, "bSearchable": false },
>+                { "aTargets": [ 3 ], "sType": "natural" }
>+            ],
>+            bInfo: false,
>+            bPaginate: false,
>+            bFilter: false,
>+            sDom: "t"
>+        }));
>+    });
>+//]]>
>+</script>
> </head>
> <body>
> [% INCLUDE 'header.inc' %]
>@@ -16,67 +33,64 @@
> <div id="bd">
>   <div id="yui-main">
>     <div class="yui-b">
>-      <h1>Files for invoice: [% invoicenumber %]</h1>
>-
>-      <p>Vendor: <a href="/cgi-bin/koha/acqui/supplier.pl?booksellerid=[% booksellerid %]">[% suppliername %]</a></p>
>-
>-                [% IF errors %]
>-                    <div class="dialog alert">
>-                        [% IF errors.empty_upload %]The file you are attempting to upload has no contents.[% END %]
>-                        [% IF errors.no_file %]You did not select a file to upload.[% END %]
>-                    </div>
>-                [% END %]
>-
>-                [% IF ( files ) %]
>-                <table>
>-                    <thead>
>-                        <tr>
>-                            <th>Name</th>
>-                            <th>Type</th>
>-                            <th>Description</th>
>-                            <th>Uploaded</th>
>-                            <th>&nbsp;</th>
>-                            <th>&nbsp;</th>
>-                        </tr>
>-                    </thead>
>-
>-                    <tbody>
>-                        [% FOREACH f IN files %]
>-                            <tr>
>-                                 <td><a href="?invoiceid=[% invoiceid %]&amp;op=download&amp;view=1&amp;file_id=[% f.file_id %]">[% f.file_name %]</a></td>
>-                                 <td>[% f.file_type %]</td>
>-                                 <td>[% f.file_description %]</td>
>-                                 <td>[% f.date_uploaded | $KohaDates %]</td>
>-                                 <td><a href="?invoiceid=[% invoiceid %]&amp;op=delete&amp;file_id=[% f.file_id %]">Delete</a></td>
>-                                 <td><a href="?invoiceid=[% invoiceid %]&amp;op=download&amp;file_id=[% f.file_id %]">Download</a></td>
>-                            </tr>
>-                        [% END %]
>-                    </tbody>
>-                </table>
>-                [% ELSE %]
>-                <div class="dialog message">
>-                    <p>This invoice has no files attached.</p>
>-                </div>
>+      <h2>Files for invoice: [% invoicenumber | html %]</h2>
>+      <p><b>Vendor: </b><a href="/cgi-bin/koha/acqui/supplier.pl?booksellerid=[% booksellerid %]">[% suppliername %]</a></p>
>+      <br />
>+      [% IF errors %]
>+        <div class="dialog alert">
>+          [% IF errors.empty_upload %]The file you are attempting to upload has no contents.[% END %]
>+          [% IF errors.no_file %]You did not select a file to upload.[% END %]
>+          [% IF errors.invalid_parameter %]Invalid or missing script parameter.[% END %]
>+        </div>
>+      [% END %]
>+      [% IF files %]
>+          <table id="invoice_files_details_table">
>+              <thead>
>+                  <tr>
>+                      <th>Name</th>
>+                      <th>Type</th>
>+                      <th>Description</th>
>+                      <th>Uploaded</th>
>+                      <th>Bytes</th>
>+                      <th>&nbsp;</th>
>+                      <th>&nbsp;</th>
>+                  </tr>
>+              </thead>
>+              <tbody>
>+                [% FOREACH f IN files %]
>+                  <tr>
>+                      <td><a href="?invoiceid=[% invoiceid %]&amp;op=download&amp;view=1&amp;file_id=[% f.file_id %]">[% f.file_name | html %]</a></td>
>+                      <td>[% f.file_type | html %]</td>
>+                      <td>[% f.file_description | html %]</td>
>+                      <td>[% f.date_uploaded | $KohaDates %]</td>
>+                      <td>[% f.file_size %]</td>
>+                      <td><a href="?invoiceid=[% invoiceid %]&amp;op=delete&amp;file_id=[% f.file_id %]">Delete</a></td>
>+                      <td><a href="?invoiceid=[% invoiceid %]&amp;op=download&amp;file_id=[% f.file_id %]">Download</a></td>
>+                  </tr>
>                 [% END %]
>-
>-                <form method="post" action="/cgi-bin/koha/acqui/invoice-files.pl" enctype="multipart/form-data">
>-                    <fieldset class="rows">
>-                        <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" />
>-
>-                        <label for="description">Description:</label>
>-                        <input name="description" id="description" type="text" /></li>
>-
>-                        <li><label for="uploadfile">File:</label><input name="uploadfile" type="file" id="uploadfile" /></li>
>-
>-                        </ol>
>-                        <fieldset class="action"><input name="upload" type="submit" id="upload" value="Upload File" /></fieldset>
>-                    </fieldset>
>-                </form>
>-
>+              </tbody>
>+          </table>
>+      [% ELSE %]
>+          <div class="dialog message">
>+              <p>This invoice has no files attached.</p>
>+          </div>
>+      [% END %]
>+      [% IF invoiceid %]
>+          <br />
>+          <form method="post" action="/cgi-bin/koha/acqui/invoice-files.pl" enctype="multipart/form-data">
>+              <fieldset class="rows">
>+                  <legend>Upload New File</legend>
>+                  <ol>
>+                      <li><input type="hidden" name="op" value="upload" />
>+                      <input type="hidden" name="invoiceid" value="[% invoiceid %]" />
>+                      <label for="description">Description:</label>
>+                      <input name="description" id="description" type="text" /></li>
>+                      <li><label for="uploadfile">File:</label><input name="uploadfile" type="file" id="uploadfile" /></li>
>+                  </ol>
>+                  <fieldset class="action"><input name="upload" type="submit" id="upload" value="Upload File" /></fieldset>
>+              </fieldset>
>+          </form>
>+      [% END %]
>     </div>
>   </div>
>   <div class="yui-b">
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/invoice.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/invoice.tt
>index 607f66b..e61d426 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/invoice.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/invoice.tt
>@@ -1,3 +1,4 @@
>+[% USE Koha %]
> [% USE KohaDates %]
> 
> [% INCLUDE 'doc-head-open.inc' %]
>@@ -15,6 +16,17 @@
>             bFilter: false,
>             sDom: "t"
>         }));
>+[% IF ( (Koha.Preference('AcqEnableFiles')) && files ) %]
>+        $("#invoice_files_table").dataTable($.extend(true, {}, dataTablesDefaults, {
>+            "aoColumnDefs": [
>+                { "aTargets": [ 3 ], "sType": "natural" }
>+            ],
>+            bInfo: false,
>+            bPaginate: false,
>+            bFilter: false,
>+            sDom: "t"
>+        }));
>+[% END %]
>     });
> //]]>
> </script>
>@@ -89,7 +101,7 @@
>       </form>
>       <p>
>           <a href="/cgi-bin/koha/acqui/parcel.pl?invoiceid=[% invoiceid %]">Go to receipt page</a>
>-          [% IF ( AcqEnableFiles ) %]| <a href="/cgi-bin/koha/acqui/invoice-files.pl?invoiceid=[% invoiceid %]">Manage invoice files</a>[% END %]
>+          [% IF Koha.Preference('AcqEnableFiles') %]| <a href="/cgi-bin/koha/acqui/invoice-files.pl?invoiceid=[% invoiceid %]">Manage invoice files</a>[% END %]
>       </p>
>       <h2>Invoice details</h2>
>       [% IF orders_loop.size %]
>@@ -170,10 +182,10 @@
>         [% ELSE %]
>             <div class="dialog message"><p>No orders yet</p></div>
>         [% END %]
>-        [% IF ( AcqEnableFiles && files ) %]
>-            <br>
>+        [% IF ( (Koha.Preference('AcqEnableFiles')) && files ) %]
>+            <br />
>             <h2>Files attached to invoice</h2>
>-            <table>
>+            <table id="invoice_files_table">
>                 <thead>
>                     <tr>
>                         <th>Name</th>
>@@ -185,10 +197,10 @@
>                 <tbody>
>                 [% FOREACH f IN files %]
>                     <tr>
>-                         <td><a href="/cgi-bin/koha/acqui/invoice-files.pl?invoiceid=[% invoiceid %]&amp;op=download&amp;view=1&amp;file_id=[% f.file_id %]">[% f.file_name %]</a></td>
>-                         <td>[% f.file_type %]</td>
>-                         <td>[% f.file_description %]</td>
>-                         <td>[% f.date_uploaded | $KohaDates %]</td>
>+                         <td><a href="/cgi-bin/koha/acqui/invoice-files.pl?invoiceid=[% invoiceid %]&amp;op=download&amp;view=1&amp;file_id=[% f.file_id %]">[% f.file_name | html %]</a></td>
>+                         <td>[% f.file_type | html %]</td>
>+                         <td>[% f.file_description | html %]</td>
>+                         <td>[% f.date_uploaded | $KohaDates %]</td>
>                     </tr>
>                 [% END %]
>                 </tbody>
>diff --git a/t/db_dependent/Koha_Misc_Files.t b/t/db_dependent/Koha_Misc_Files.t
>new file mode 100755
>index 0000000..d88da5c
>--- /dev/null
>+++ b/t/db_dependent/Koha_Misc_Files.t
>@@ -0,0 +1,87 @@
>+#!/usr/bin/perl
>+
>+# Unit tests for Koha::Misc::Files
>+# Author: Jacek Ablewicz, abl@biblos.pk.edu.pl
>+
>+use Modern::Perl;
>+use C4::Context;
>+use Test::More tests => 27;
>+
>+BEGIN {
>+    use_ok('Koha::Misc::Files');
>+}
>+
>+my $dbh = C4::Context->dbh;
>+$dbh->{AutoCommit} = 0;
>+$dbh->{RaiseError} = 1;
>+
>+## new() parameter handling check
>+is(Koha::Misc::Files->new(recordid => 12), undef, "new() param check test/1");
>+is(Koha::Misc::Files->new(recordid => 'aa123', tabletag => 'ttag_a'), undef, "new() param check test/2");
>+
>+## create some test objects with arbitrary (tabletag, recordid) pairs
>+my $mf_a_123 = Koha::Misc::Files->new(recordid => '123', tabletag => 'tst_table_a');
>+my $mf_a_124 = Koha::Misc::Files->new(recordid => '124', tabletag => 'tst_table_a');
>+my $mf_b_221 = Koha::Misc::Files->new(recordid => '221', tabletag => 'tst_table_b');
>+is(ref($mf_a_123), "Koha::Misc::Files", "new() returned object type");
>+
>+## GetFilesInfo() initial tests (dummy AddFile() / parameter handling checks)
>+is(ref($mf_a_123->GetFilesInfo()), 'ARRAY', "GetFilesInfo() return type");
>+is(scalar @{$mf_a_123->GetFilesInfo()}, 0, "GetFilesInfo() empty/non-empty result/1");
>+$mf_a_123->AddFile(name => '', type => 'text/plain', content => "aaabbcc");
>+is(scalar @{$mf_a_123->GetFilesInfo()}, 0, "GetFilesInfo() empty/non-empty result/2");
>+
>+## AddFile(); add 5 sample file records for 3 test objects
>+$mf_a_123->AddFile(name => 'File_name_1.txt', type => 'text/plain',
>+  content => "file contents\n1111\n", description => "File #1 sample description");
>+$mf_a_123->AddFile(name => 'File_name_2.txt', type => 'text/plain',
>+  content => "file contents\n2222\n", description => "File #2 sample description");
>+$mf_a_124->AddFile(name => 'File_name_3.txt', content => "file contents\n3333\n", type => 'text/whatever');
>+$mf_a_124->AddFile(name => 'File_name_4.txt', content => "file contents\n4444\n");
>+$mf_b_221->AddFile(name => 'File_name_5.txt', content => "file contents\n5555\n");
>+
>+## check GetFilesInfo() results for added files
>+my $files_a_123_infos = $mf_a_123->GetFilesInfo();
>+is(scalar @$files_a_123_infos, 2, "GetFilesInfo() result count/1");
>+is(scalar @{$mf_b_221->GetFilesInfo()}, 1, "GetFilesInfo() result count/2");
>+is(ref($files_a_123_infos->[0]), 'HASH', "GetFilesInfo() item file result type");
>+is($files_a_123_infos->[0]->{file_name}, 'File_name_1.txt', "GetFilesInfo() result check/1");
>+is($files_a_123_infos->[1]->{file_name}, 'File_name_2.txt', "GetFilesInfo() result check/2");
>+is($files_a_123_infos->[1]->{file_type}, 'text/plain', "GetFilesInfo() result check/3");
>+is($files_a_123_infos->[1]->{file_size}, 19, "GetFilesInfo() result check/4");
>+is($files_a_123_infos->[1]->{file_description}, 'File #2 sample description', "GetFilesInfo() result check/5");
>+
>+## GetFile() result checks
>+is($mf_a_123->GetFile(), undef, "GetFile() result check/1");
>+is($mf_a_123->GetFile(id => 0), undef, "GetFile() result check/2");
>+
>+my $a123_file_1 = $mf_a_123->GetFile(id => $files_a_123_infos->[0]->{file_id});
>+is(ref($a123_file_1), 'HASH', "GetFile() result check/3");
>+is($a123_file_1->{file_id}, $files_a_123_infos->[0]->{file_id}, "GetFile() result check/4");
>+is($a123_file_1->{file_content}, "file contents\n1111\n", "GetFile() result check/5");
>+
>+## MergeFileRecIds() tests
>+$mf_a_123->MergeFileRecIds(123,221);
>+$files_a_123_infos = $mf_a_123->GetFilesInfo();
>+is(scalar @$files_a_123_infos, 2, "GetFilesInfo() result count after dummy MergeFileRecIds()");
>+$mf_a_123->MergeFileRecIds(124);
>+$files_a_123_infos = $mf_a_123->GetFilesInfo();
>+is(scalar @$files_a_123_infos, 4, "GetFilesInfo() result count after MergeFileRecIds()/1");
>+is(scalar @{$mf_a_124->GetFilesInfo()}, 0, "GetFilesInfo() result count after MergeFileRecIds()/2");
>+is($files_a_123_infos->[-1]->{file_name}, 'File_name_4.txt', "GetFilesInfo() result check after MergeFileRecIds()");
>+
>+## DelFile() test
>+$mf_a_123->DelFile(id => $files_a_123_infos->[-1]->{file_id});
>+$files_a_123_infos = $mf_a_123->GetFilesInfo();
>+is(scalar @$files_a_123_infos, 3, "GetFilesInfo() result count after DelFile()");
>+
>+## DelAllFiles() tests
>+$mf_a_123->DelAllFiles();
>+$files_a_123_infos = $mf_a_123->GetFilesInfo();
>+is(scalar @$files_a_123_infos, 0, "GetFilesInfo() result count after DelAllFiles()/1");
>+$mf_b_221->DelAllFiles();
>+is(scalar @{$mf_b_221->GetFilesInfo()}, 0, "GetFilesInfo() result count after DelAllFiles()/2");
>+
>+$dbh->rollback;
>+
>+1;
>-- 
>1.7.10.4

Actions: View | Diff | Splinter Review

Attachments on bug 3050: 25253 | 25306 | 26724 | 28123 | 28124 | 28125 | 28362 | 28363 | 28364 | 28391 | 28392 | 28393 | 28394