[Koha-bugs] [Bug 14399] New: Remove "missingitem" code from inventory.pl

[bugzilla-daemon at bugs.koha-community.org]

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

            Bug ID: 14399
           Summary: Remove "missingitem" code from inventory.pl
 Change sponsored?: ---
           Product: Koha
           Version: master
          Hardware: All
                OS: All
            Status: NEW
          Severity: normal
          Priority: P5 - low
         Component: Tools
          Assignee: gmcharlt at gmail.com
          Reporter: dcook at prosentient.com.au
        QA Contact: testopia at bugs.koha-community.org

At the moment, there are two different states (described as "problems") in the
code of inventory.pl.

One is for "missingitem" and the other is for "not_scanned".

After much whingeing and discussion, I decided that these are basically the
same state, although they're obtained in slightly different ways:

1) "not_scanned" is determined by comparing the scanned barcodes against the
inventory list.
2) "missingitem" is determined by comparing the inventory's "datelastseen"
against the inventory list's items' "datelastseen". 

The first one seems logical. It's "not_scanned" if it wasn't scanned.

However, the second one is strange. It's not even in relation to the actual
task of taking inventory. This state also isn't determined "after" running the
inventory, which might almost make sense. No, it's right after creating an
inventory list. On top of this weirdness, the code also has an unintentional
bug. Observe the following block of code:

# If "compare barcodes list to results" has been checked, we want to alert for
missing items
if ( $compareinv2barcd ) {
    # set "missing" flags for all items with a datelastseen (dls) before the
choosen datelastseen (cdls)
    my $dls = output_pref( { dt => dt_from_string( $datelastseen ),
                             dateformat => 'iso' } );
    foreach my $item ( @$inventorylist ) {
        my $cdls = output_pref( { dt => dt_from_string( $_->{datelastseen} ),
                                  dateformat => 'iso' } );
        if ( $cdls lt $dls ) {
            $item->{problem} = 'missingitem';
            # We have to push a copy of the item, not the reference
            push @items_with_problems, { %$item };
        }
    }
}

Firstly, contrary to the comment, "dls" is actually the inventory's
"datelastseen", while "cdls" is the "datelastseen" of the item. Annoying? Yes,
but not super important.

The important thing here is "my $cdls = output_pref( { dt => dt_from_string(
$_->{datelastseen} ),".

It turns out that when you use "foreach my $item ( @$inventorylist )" instead
of "foreach ( @$inventorylist )", "$_" will actually be null and not the
element from the list!

So "dt_from_string" seems to create a DateTime object with today's date. So
$cdls will always have a date of today. The only way to get "missingitem" to
appear is if your "Inventory date:" (not to be mistaken with "Set inventory
date to:" inventory date is in the future. Which... would be weird to do... but
it's possible.

And because this only happens when you're comparing the uploaded barcodes
against the inventory list... and because missingitem is done on the inventory
list... everything is going to be marked missing in your inventory report!

-- 
You are receiving this mail because:
You are watching all bug changes.