1 files changed, 71 insertions, 26 deletions

diff --git a/LedgerSMB/IS.pm b/LedgerSMB/IS.pm
index b0f2e4d7..8a7dd8b2 100755
--- a/LedgerSMB/IS.pm
+++ b/LedgerSMB/IS.pm
@@ -1510,6 +1510,12 @@ sub process_assembly {
 }
 
 sub cogs {
+    # This is nearly entirely rewritten since 1.2.8 based in part on the works
+    # of Victor Sterpu and Dieter Simader (see CONTRIBUTORS for more 
+    # information).  However, there are a number of areas where I have 
+    # substantially rewritten the logic.  This function is heavily annotated 
+    # largely because COGS/invoices are still scheduled to be re-engineered in
+    # 1.4 so it is a good idea to have records of opinions in the code.-- CT
     my ( $dbh2, $form, $id, $totalqty, $project_id, $sellprice) = @_;
     my $dbh   = $form->{dbh};
     my $query;
@@ -1614,40 +1620,79 @@ sub cogs {
                             $dbh, "invoice", "allocated", 
                             qq|id = $ref->{id}|, $qty * -1 
             );
-            $allocated += $qty;
-            my $linetotal = $qty*$ref->{sellprice};
-            $query = qq|
-		INSERT INTO acc_trans 
-		            (trans_id, chart_id, amount, 
-                             transdate, project_id, invoice_id) 
-		     VALUES (?, ?, ?, ?, ?, ?)|;
-
-            my $sth1 = $dbh->prepare($query);
-            $sth1->execute(
-                         $form->{id}, $ref->{"expense_accno_id"}, 
-                         $linetotal, $form->{transdate}, 
-                         $project_id, $ref->{id}
-            ) || $form->dberror($query);
 
-            $query = qq|
-		INSERT INTO acc_trans 
-		            (trans_id, chart_id, amount, transdate, 
-		             project_id, invoice_id) 
-		     VALUES (?, ?, ?, ?, ?, ?)|;
-
-            $sth1 = $dbh->prepare($query);
-            $sth1->execute(
-                 $form->{id}, $ref->{"inventory_accno_id"}, 
-                 -$linetotal, $form->{transdate}, 
-                 $project_id, $ref->{id}
-            ) || $form->dberror($query);
+            # Note:  No COGS calculations on reversed short sale invoices.  
+            # This merely prevents COGS calculations in the future agaisnt
+            # such short invoices.  -- CT
 
             $totalqty -= $qty;
             last if $totalqty == 0;
         }
+        # If the total quantity is still less than zero, we must assume that
+        # this is just an invoice which has been voided or products returns 
+        # but is not merely representing a voided short sale, and therefore 
+        # we need to unallocate the items from AP.  There has been some debate
+        # as to how to approach this, and I think it is safest to unallocate
+        # the most recently allocated AP items of the same type regardless of
+        # the relevant dates of the invoices.  I can see cases where this 
+        # might require adjustments, however.  -- CT
+
+        if ($totalqty < 0){
+            $query = qq|
+		  SELECT i.allocated, i.sellprice, p.inventory_accno_id, 
+		         p.expense_accno_id 
+		    FROM invoice i
+		    JOIN parts p ON (i.parts_id = p.id)
+		    JOIN ap ON (i.trans_id = a.id)
+		   WHERE (i.allocated + i.qty) < 0
+		         AND i.parts_id = ?
+		ORDER BY a.transdate DESC, a.id DESC
+            |;
+
+            my $sth = $dbh->prepare($query);
+            $sth->execute($id);
+
+            while (my $ref = $sth->fetchrow_hashref(NAME_lc)){
+                my $qty = $ref->{allocated};
+
+                %qty = ($qty > $totalqty) ? $totalqty : $qty;
+
+                $allocated += $qty;
+                my $linetotal = $qty*$ref->{sellprice};
+                push @{ $form->{acc_trans}{lineitems} },
+                  {
+                    chart_id   => $ref->{expense_accno_id},
+                    amount     => $linetotal,
+                    project_id => $project_id,
+                    invoice_id => $ref->{id}
+                  };
+
+                push @{ $form->{acc_trans}{lineitems} },
+                  {
+                    chart_id   => $ref->{inventory_accno_id},
+                    amount     => -$linetotal,
+                    project_id => $project_id,
+                    invoice_id => $ref->{id}
+                  };
+
+                $totalqty -= $qty;
+                $allocated += $qty;
+
+                last if $totalqty == 0;
+            }
+        }
+
+        # If we still have less than 0 total quantity, this is not a return
+        # or a void.  Throw an error.  If there are valid workflows that throw
+        # this error, they will require more work to address and will not work
+        # safely with the current system.  -- CT
         if ($totalqty < 0){
             $form->error("Too many reversed items on an invoice");
         }
+        elsif ($totalqty > 0){
+            $form->error("Unexpected and invalid quantity allocated.".
+                   "  Aborting.");
+        }
     }
 
     return $allocated;