[interchange-bugs] [Bug 532] New: Minor Bug in mv_order_* Stacking

[bugzilla-daemon at icdevgroup.org]

http://www.icdevgroup.org/bugs/show_bug.cgi?id=532

           Summary: Minor Bug in mv_order_* Stacking
           Product: Interchange
           Version: 4.9.7
          Platform: PC
            Status: NEW
          Severity: normal
          Priority: P3
         Component: Core
        AssignedTo: mike at valuemedia.com
        ReportedBy: brian at groxis.com


I found a minor bug (at least I think it's a bug) in Vend::Order::add_items 
that was causing me problems with stacking.  It uses perl split with a limit 
of -1 to create the arrays of items, quantities, etc to be stacked.  However, 
in my application, I found that this resulted in empty values at the ends of 
these arrays, causing unpredictable results.  

For example, if @items has more entries than @quantities, an item whose index 
is outside the bounds of @quantites is given a quantity of 1 (logical and 
desired bahavior in my case).  However, if the last entry in @quantites is 
empty (from the -1 split limit) the item with that index is given an empty 
quantity (not 1).  Further down in the code, an item with no quantity is 
omitted, and so that line would not be added to the cart.

Tricky.

I removed the limits of -1 for the splits in add_items, which seemed to have 
solved my problem.

# diff Order.pm.orig Order.pm
2242,2244c2242,2244
<       @items      = split /\0/, ($items), -1;
<       @quantities = split /\0/, ($quantities || delete $CGI::values
{mv_order_quantity} || ''), -1;
<       @bases      = split /\0/, delete $CGI::values{mv_order_mv_ib}, -1
---
>       @items      = split /\0/, ($items);
>       @quantities = split /\0/, ($quantities || delete $CGI::values
{mv_order_quantity} || '');
>       @bases      = split /\0/, delete $CGI::values{mv_order_mv_ib}
2246c2246
<       @lines      = split /\0/, delete $CGI::values{mv_orderline}, -1
---
>       @lines      = split /\0/, delete $CGI::values{mv_orderline}
2254c2254
<                       @fly = split /\0/, $CGI::values{mv_order_fly}, -1;
---
>                       @fly = split /\0/, $CGI::values{mv_order_fly};
2267c2267
<               my @sku = split /\0/, $CGI::values{mv_sku}, -1;
---
>               my @sku = split /\0/, $CGI::values{mv_sku};
2279c2279
<                       @{$attr{$attr}} = split /\0/, $CGI::values
{"mv_order_$attr"}, -1;
---
>                       @{$attr{$attr}} = split /\0/, $CGI::values
{"mv_order_$attr"};
2305c2305
<       @group   = split /\0/, (delete $CGI::values{mv_order_group} || ''), -1;
---
>       @group   = split /\0/, (delete $CGI::values{mv_order_group} || '');


Although I am only exercising @items and @quantities at this time, I decided 
it was best to remove all the -1 limits for consistency, as I did not see a 
good reason to include any empty trailing fields in this application.  Mike 
Heins suggested that the -1 limits were included to avoid warnings in older 
versions of Perl.  You might see fit to resolve it in a different way, or not 
at all.



------- You are receiving this mail because: -------
You reported the bug, or are watching the reporter.