1 (edited by apmuthu 02/26/2015 10:42:54 am)

Topic: Import Items $mb_flag 'S' wrong

The file includes/manufacturing.inc had the acceptable values of the $mb_flag variable / mb_flag field:
M => Manufactured
B => Purchased or Bought
D => Services - do not know the rationale for this assignment. Possibly DeStocked or Discard Inventory.

The extensions file modules/import_items/import_items.php lines 246-247:

if ($mb_flag != "S") {
    $sql = "INSERT INTO ".TB_PREF."loc_stock (loc_code, stock_id) VALUES ('{$_POST['location']}', '$id')";

Should we change the D => S in the core file includes/manufacturing.inc or S => D in the extension file import_items.php ?

Re: Import Items $mb_flag 'S' wrong

Probably a typo, then. The question is which one is correct. The S certainly makes more sense.

Generally, modules should adapt to the core. Not the other way around.

But if there are no other occurrences, I would change the core here.

Re: Import Items $mb_flag 'S' wrong

@joe: Shall we change the core - what of the community and existing users data?

Re: Import Items $mb_flag 'S' wrong

Found this. Do imported items actually get stored with an S? Then I guess they will be treated as inventory items.

includes/db/inventory_db.inc:

function is_inventory_item($stock_id)
{
$sql = "SELECT stock_id FROM ".TB_PREF."stock_master
WHERE stock_id=".db_escape($stock_id)." AND mb_flag <> 'D'";
$result = db_query($sql, "Cannot query is inventory item or not");
return db_num_rows($result) > 0;
}


If we have data stored with an S, it is already broken. If not, changing the core will not break anything. But we can't have two different representations for the same thing and only test for one. It should be simple enough for an upgrade script to change the database values.

Re: Import Items $mb_flag 'S' wrong

There are quite a few placed where D is hard coded in the core and in the extension files.

A brief history of mb_flag changes:

2014-09-12: Removed redundant mb_flag 'A'.
2009-05-18: Added mb_flag 'M' in demand checks
2009-05-17: Removed obsolete 'K' mb_flag checks.

Extensions having mb_flag and referencing 'D' or 'S':

Extensions/api/sync_db.inc : Lines 393, 418, 1069, 1109, 1152 => D
Extensions/import_items/import_items.php : Line 246 => S
Extensions/import_items/readme.txt : Line 111, 163 => S

Core files having mb_flag and referencing 'D' or 'S':

includes/data_checks.inc : Line 333 => D
includes/manufacturing.inc : Line 27 => D
includes/db/inventory_db.inc : Line 114 => D
includes/ui/ui_lists.inc : Lines 915, 924 => D
manufacturing/work_order_add_finished.php : Line 139 => D
reporting/rep301.php : Line 113 => D
reporting/rep307.php : Line 40 => D
reporting/rep308.php : Line 70 => D

Re: Import Items $mb_flag 'S' wrong

Well... D it is, then.

Looking at the import_items extension, it seems to insert and update tables with whatever $mb_flag it gets from the CSV. And the documentation suggests using S, which means service items will be stored as inventory items. (Or, rather, read back as such when retrieved from the database later.)

So it's broken. People already have items in their databases with an invalid mb_flag, which can be easily corrected by the upgrade script. But to prevent such data from being written in the future, the extension has to handle both cases, rewriting S to D before feeding anything to the database. It's not ideal, but there will be other scripts preparing the data for import, which have been written according to the instructions in the readme file. At least we don't need to put THAT in the core...

Re: Import Items $mb_flag 'S' wrong

Committed the fix to the extension's files in my GitHub repo.

Re: Import Items $mb_flag 'S' wrong

Hmm... But that still puts the "S" into stock_master.

My idea was adding

if ($mb_flag == "S")
  $mb_flag ="D";

after

 
$mb_flag = strtoupper($mb_flag);

Re: Import Items $mb_flag 'S' wrong

@tm: That's a fine way to tolerate old habits. Thanks. Committed in my GitHub Repo.