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

--- Comment #6 from M. Tompsett <[email protected]> ---
Comment on attachment 43954
  --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=43954
Bug 14946 - Remove C4::Dates from files acqui/*.pl

Review of attachment 43954:
 --> 
(http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=14946&attachment=43954)
-----------------------------------------------------------------

Some of these comments happen multiple times in this patch. I just picked on a
couple.

::: acqui/finishreceive.pl
@@ +63,5 @@
>  my $new_ordernumber  = $ordernumber;
>  
> +$datereceived = eval { output_pref( { dt => dt_from_string( $datereceived) , 
> dateonly => 1, dateformat => 'iso' } ); };
> +$datereceived = output_pref( { dt => dt_from_string( $datereceived) , 
> dateonly => 1, dateformat => 'iso' } )
> +    if (! $datereceived );

Either both lines will succeed or both will fail. Why is there $datereceived in
the second line call to dt_from_string?

::: acqui/histsearch.pl
@@ +101,2 @@
>  }
> +if ( $d = $input->param('to') ) {

Why is this 'to' and the other is 'iso'? I don't see a template change in this
patch.

::: acqui/invoice.pl
@@ +84,5 @@
> +    my $billingdate        = '';
> +
> +    if ( $input->param('shipmentdate') ) {
> +        $shipmentdate = eval { dt_from_string( $input->param('shipmentdate') 
> ) };
> +        $shipmentdate = output_pref( { dt => $shipmentdate, dateformat => 
> 'iso',  dateonly => 1 } ) if ( $shipmentdate );

Post-fix if required because param('shipmentdate') could be a bad date?

::: acqui/orderreceive.pl
@@ +222,4 @@
>      invoiceid             => $invoice->{invoiceid},
>      invoice               => $invoice->{invoicenumber},
>      datereceived          => $datereceived,
> +    datereceived_iso      => output_pref( { dt => $datereceived, dateonly => 
> 1, dateformat => 'iso' } ),

Formatting a date to ISO seems like a GUI issue. Is there not a way to format
dates ISO in the Template? If not, ignore this. :)

::: acqui/parcels.pl
@@ +146,5 @@
>  my @parcels = GetInvoices(
>      supplierid => $booksellerid,
>      invoicenumber => $code,
> +    ( $datefrom ? ( eval { shipmentdatefrom => output_pref({ dt => 
> dt_from_string($datefrom), dateformat => 'iso' }); } ) : () ),
> +    ( $dateto   ? ( eval { shipmentdateto   => output_pref({ dt => 
> dt_from_string($dateto),   dateformat => 'iso' }); } )    : () ),

Nested code like this is difficult to read, and the ? makes it worse. Please
set some local variables outside the parameter list to use.

-- 
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[email protected]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

Reply via email to