My idea was adding

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

after

 
$mb_flag = strtoupper($mb_flag);

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...

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

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.

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.

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 ?