CSV import optimizations and code cleanup (#3150)

Optimizations and CSV Import Rework
- Replaced " with ' where possible to prevent the parser from being called when not needed.
- Replaced == and != with === and !== where possible for bug prevention and speed.
- Replaced -1 with NEW_ITEM global constant for code clarity.
- Added NEW_ITEM global constant to constants.php.
- Refactored CSV import function names for clarity.
- Added capability to import a CSV file containing updates.
- Replaced array() with [] for speed and consistency.
- Removed hungarian notation from two private functions.
- Refactored QueryBuilder functions to place table name in the get() function call.
- Replaced (int) cast with call to intval() for speed.
- Replaced == and != with === and !== where possible to prevent bugs and for speed.
- Replaced array() with [] for speed and consistency.
- Fixed search_custom call

Optimizations and bugfixes for attributes used in csv_import
- Reordered where statements in queries to match composite index on attribute_links table.
- fixed value_exists() to account for different attribute types.
- Removed hungarian notation on private function.
- Replaced array() with [] for speed and consistency.
- Replaced != with <> in SQL for consistency.
- Removed from() calls in querybuilder where possible to reduce function calls.
- Add get_items_by_value()
- Reworked check_data_validity()
- Remove unneeded comments
- Refactor functions for code clarity.
- Use $this->db->dbprefix() where possible instead of hand-writing ospos_...
- Removed unneeded column from query.
- Replaced (int) cast with intval() call for speed.
- Added get_attribute_values()
- Fixed issue with date format locale not being used
- Refactored save_value to respect different attribute_types
- Added delete_orphaned_links() to remove attribute_links that are no longer linked to any items
- Added get_attributes_by_definition()
- Added attribute_cleanup()

Optimizations used in csv_import
- replaced array() with [] for consistency and speed.
- Removed hungarian notation in private functions.
- Replaced " with ' where possible to prevent the parser from being called.
- Minor formatting
- Refactored if statement to tertiary notation for cleaner implementation.
- Replaced " for ' where possible to prevent the parser from being called.
- Added the Id column in the generate_import_items_csv() template so that users can submit an update to an existing item.
- Removed unused key=>value pairs in foreach loops for speed.
- Removed unneeded comments where the function name was self-explanatory.
- Rework get_csv_file() for speed.
- Rework bom_exists() for speed.
- Replaced array() with [] for speed and consistency.
- Replaced == with === where possible to prevent bugs and for speed.
- Reworked valid_date() and valid_decimal helper functions for speed and accuracy according to the locale_format instead of a fixed format.
- Minor Reformatting for clarity.
- Replaced " for ' to prevent the parser from being called.
- Refactored function call names to reflect new names.
- Added missing ; in &nbsp;
- Used String interpolation where useful.

- Spelling fix in comment

Requested Review Changes
- Fixed indentation in Items.php
- Fixed indentation in Attribute.php
- Refactored variable out of long line of code to make it more readable.

application/helpers/importfile_helper.php

@@ -1,30 +1,20 @@
 <?php if (!defined('BASEPATH')) exit('No direct script access allowed');
 
-/**
- * Generates the header content for the import_items.csv file
- *
- * @return	string						Comma separated headers for the CSV file
- */
 function generate_import_items_csv($stock_locations,$attributes)
 {
-	$csv_headers = pack("CCC",0xef,0xbb,0xbf);	//Encode the Byte-Order Mark (BOM) so that UTF-8 File headers display properly in Microsoft Excel
-	$csv_headers .= 'Barcode,"Item Name",Category,"Supplier ID","Cost Price","Unit Price","Tax 1 Name","Tax 1 Percent","Tax 2 Name","Tax 2 Percent","Reorder Level",Description,"Allow Alt Description","Item has Serial Number",item_image,HSN';
+	$csv_headers = pack('CCC',0xef,0xbb,0xbf);	//Encode the Byte-Order Mark (BOM) so that UTF-8 File headers display properly in Microsoft Excel
+	$csv_headers .= 'Id,Barcode,"Item Name",Category,"Supplier ID","Cost Price","Unit Price","Tax 1 Name","Tax 1 Percent","Tax 2 Name","Tax 2 Percent","Reorder Level",Description,"Allow Alt Description","Item has Serial Number",Image,HSN';
 	$csv_headers .= generate_stock_location_headers($stock_locations);
 	$csv_headers .= generate_attribute_headers($attributes);
 
 	return $csv_headers;
 }
 
-/**
- * Generates a list of stock location names as a string
- *
- * @return string					Comma-separated list of stock location names
- */
 function generate_stock_location_headers($locations)
 {
-	$location_headers = "";
+	$location_headers = '';
 
-	foreach($locations as $location_id => $location_name)
+	foreach($locations as $location_name)
 	{
 		$location_headers .= ',"location_' . $location_name . '"';
 	}
@@ -32,14 +22,9 @@ function generate_stock_location_headers($locations)
 	return $location_headers;
 }
 
-/**
- * Generates a list of attribute names as a string
- *
- * @return string					Comma-separated list of attribute names
- */
 function generate_attribute_headers($attribute_names)
 {
-	$attribute_headers = "";
+	$attribute_headers = '';
 	unset($attribute_names[-1]);
 
 	foreach($attribute_names as $attribute_name)
@@ -50,61 +35,59 @@ function generate_attribute_headers($attribute_names)
 	return $attribute_headers;
 }
 
-/**
- * Read the contents of a given CSV formatted file into a two-dimensional array
- *
- * @param	string				$file_name	Name of the file to read.
- * @return	boolean|array[][]				two-dimensional array with the file contents or FALSE on failure.
- */
 function get_csv_file($file_name)
 {
-	ini_set("auto_detect_line_endings", true);
+//TODO: current implementation reads the entire file in.  This is memory intensive for large files.
+//We may want to rework the CSV import feature to read the file in chunks, process it and continue.
+//It must be done in a way that does not significantly negatively affect performance.
+	ini_set('auto_detect_line_endings', true);
+
+	$csv_rows = FALSE;
 
 	if(($csv_file = fopen($file_name,'r')) !== FALSE)
 	{
+		$CI =& get_instance();
+		$CI->load->helper('security');
+
+		$csv_rows = [];
+
 		//Skip Byte-Order Mark
 		if(bom_exists($csv_file) === TRUE)
 		{
 			fseek($csv_file, 3);
 		}
 
-		while (($data = fgetcsv($csv_file)) !== FALSE)
+		$headers = fgetcsv($csv_file);
+
+		while(($row = fgetcsv($csv_file)) !== FALSE)
 		{
-		//Skip empty lines
-			if(array(null) !== $data)
+			//Skip empty lines
+			if($row !== array(null))
 			{
-				$line_array[] = $data;
+				$csv_rows[] = array_combine($headers, $CI->security->xss_clean($row));
 			}
 		}
-	}
-	else
-	{
-		return FALSE;
+
+		fclose($csv_file);
 	}
 
-	return $line_array;
+	return $csv_rows;
 }
 
-/**
- * Checks the first three characters of a file for the Byte-Order Mark then returns the file position to the first character.
- *
- * @param	object $file_handle	File handle to check
- * @return	bool				Returns TRUE if the BOM exists and FALSE otherwise.
- */
 function bom_exists(&$file_handle)
 {
-	$str = fread($file_handle,3);
+	$result		= FALSE;
+	$candidate	= fread($file_handle, 3);
+
 	rewind($file_handle);
 
-	$bom = pack("CCC", 0xef, 0xbb, 0xbf);
+	$bom = pack('CCC', 0xef, 0xbb, 0xbf);
 
-	if (0 === strncmp($str, $bom, 3))
+	if (0 === strncmp($candidate, $bom, 3))
 	{
-		return TRUE;
-	}
-	else
-	{
-		return FALSE;
+		$result = TRUE;
 	}
+
+	return $result;
 }
 ?>

application/helpers/locale_helper.php

@@ -249,7 +249,7 @@ function get_payment_options()
 	$config = get_instance()->config;
 	$lang = get_instance()->lang;
 
-	$payments = array();
+	$payments = [];
 
 
 	if($config->item('payment_options_order') == 'debitcreditcash')
@@ -388,7 +388,7 @@ function to_quantity_decimals($number)
 	return to_decimals($number, 'quantity_decimals');
 }
 
-function to_decimals($number, $decimals=NULL, $type=\NumberFormatter::DECIMAL)
+function to_decimals($number, $decimals = NULL, $type=\NumberFormatter::DECIMAL)
 {
 	// ignore empty strings and return
 	// NOTE: do not change it to empty otherwise tables will show a 0 with no decimal nor currency symbol
@@ -424,7 +424,6 @@ function parse_tax($number)
 function parse_decimals($number, $decimals = NULL)
 {
 	// ignore empty strings and return
-
 	if(empty($number))
 	{
 		return $number;
@@ -442,7 +441,7 @@ function parse_decimals($number, $decimals = NULL)
 
 	$config = get_instance()->config;
 
-	if($decimals == NULL)
+	if($decimals === NULL)
 	{
 		$decimals = $config->item('currency_decimals');
 	}
@@ -600,12 +599,13 @@ function dateformat_bootstrap($php_format)
 
 function valid_date($date)
 {
-	return preg_match('/^([0-9]{2,4})-([0-1][0-9])-([0-3][0-9])(?:( [0-2][0-9]):([0-5][0-9]):([0-5][0-9]))?$/', $date);
+	$config = get_instance()->Appconfig;
+	return (DateTime::createFromFormat($config->get('dateformat'), $date));
 }
 
 function valid_decimal($decimal)
 {
-	return preg_match('/^(\d*\.)?\d+$/', $decimal);
+	return (preg_match('/^(\d*\.)?\d+$/', $decimal) === 1);
 }
 
 ?>