Formatting and naming refactor

- Added `@noinspection PhpUnused` tags to PHPdocs for functions which are called via AJAX. - removed conversion to array in getResult in favor of returning an array to begin with. - Refactored variable for clarity. - declared variable in view coming from controller - Added PHPdocs - Refactored nested if/else statements into ternary notation. - Corrected tab type - added missing model declaration in view - Modified query builder to extrapolate out the set() command for clarity
2026-04-27 10:31:50 -04:00 · 2023-11-17 18:24:27 +04:00
parent c971e025b8
commit 2bb4b7c865
5 changed files with 153 additions and 118 deletions
--- a/app/Controllers/Attributes.php
+++ b/app/Controllers/Attributes.php
@@ -58,7 +58,9 @@ class Attributes extends Secure_Controller
 	}

 	/**
+	 * AJAX called function which saves the attribute value sent via POST by using the model save function.
 	 * @return void
+	 * @noinspection PhpUnused
 	 */
 	public function postSaveAttributeValue(): void
 	{
@@ -73,7 +75,9 @@ class Attributes extends Secure_Controller
 	}

 	/**
+	 * AJAX called function deleting an attribute value using the model delete function.
 	 * @return void
+	 * @noinspection PhpUnused
 	 */
 	public function postDelete_attribute_value(): void
 	{
@@ -86,8 +90,11 @@ class Attributes extends Secure_Controller
 	}

 	/**
+	 * AJAX called function which saves the attribute definition.
+	 *
 	 * @param int $definition_id
 	 * @return void
+	 * @noinspection PhpUnused
 	 */
 	public function postSaveDefinition(int $definition_id = NO_DEFINITION_ID): void
 	{
@@ -155,8 +162,10 @@ class Attributes extends Secure_Controller
 	}

 	/**
+	 *
 	 * @param int $definition_id
 	 * @return void
+	 * @noinspection PhpUnused
 	 */
 	public function getSuggestAttribute(int $definition_id): void
 	{
@@ -213,11 +222,21 @@ class Attributes extends Secure_Controller
 		echo view('attributes/form', $data);
 	}

-	public function postDelete_value(int $attribute_id): bool	//TODO: This function appears to never be used in the codebase.  Is it needed?
+	/**
+	 * AJAX called function to delete an attribute value. This is never called in the code. Perhaps it was boiler plate code that just isn't needed?
+	 * @param int $attribute_id
+	 * @return bool
+	 * @noinspection PhpUnused
+	 */
+	public function delete_value(int $attribute_id): bool	//TODO: This function appears to never be used in the codebase.  Is it needed?
 	{
 		return $this->attribute->delete_value($attribute_id, NO_DEFINITION_ID);
 	}

+	/**
+	 * Deletes an attribute definition
+	 * @return void
+	 */
 	public function postDelete(): void
 	{
 		$attributes_to_delete = $this->request->getPost('ids', FILTER_SANITIZE_FULL_SPECIAL_CHARS);
--- a/app/Controllers/Items.php
+++ b/app/Controllers/Items.php
@@ -283,23 +283,7 @@ class Items extends Secure_Controller

 		$item_info = $this->item->get_info($item_id);

-		if($data['allow_temp_item'] === 1)
-		{
-			if($item_id !== NEW_ENTRY)
-			{
-				if($item_info->item_type != ITEM_TEMP)
-				{
-					$data['allow_temp_item'] = 0;
-				}
-			}
-		}
-		else
-		{
-			if($item_info->item_type == ITEM_TEMP)
-			{
-				$data['allow_temp_item'] = 1;
-			}
-		}
+		$data['allow_temp_item'] = ($data['allow_temp_item'] === 1 && $item_id !== NEW_ENTRY && $item_info->item_type != ITEM_TEMP) ? 0 : 1;

 		$use_destination_based_tax = (boolean)$this->config['use_destination_based_tax'];
 		$data['include_hsn'] = $this->config['include_hsn'] === '1';
@@ -342,7 +326,6 @@ class Items extends Secure_Controller
 			&& !($this->config['derive_sale_quantity'] === '1')
 		);

-
 		$data['item_info'] = $item_info;

 		$suppliers = ['' => lang('Items.none')];
@@ -438,7 +421,14 @@ class Items extends Secure_Controller
 		echo view('items/form', $data);
 	}

-	public function inventory(int $item_id = NEW_ENTRY): void
+	/**
+	 * AJAX called function which returns the update inventory form view for an item
+	 *
+	 * @param int $item_id
+	 * @return void
+	 * @noinspection PhpUnused
+	 */
+	public function getInventory(int $item_id = NEW_ENTRY): void
 	{
 		$item_info = $this->item->get_info($item_id);	//TODO: Duplicate code

--- a/app/Models/Attribute.php
+++ b/app/Models/Attribute.php
@@ -181,6 +181,12 @@ class Attribute extends Model
 		return $builder->get();
 	}

+	/**
+	 * Gets all attributes connected to an item given the item_id
+	 *
+	 * @param int $item_id
+	 * @return array
+	 */
 	public function get_attributes_by_item(int $item_id): array
 	{
 		$builder = $this->db->table('attribute_definitions');
@@ -452,14 +458,15 @@ class Attribute extends Model
 		$zero_attribute_id = $this->value_exists('0');
 		$one_attribute_id = $this->value_exists('1');

-		if($zero_attribute_id === FALSE)
+		if($zero_attribute_id === false)
 		{
-			$zero_attribute_id = $this->save_value('0', $definition_id, FALSE, FALSE, CHECKBOX);
+			$zero_attribute_id = $this->save_value('0', $definition_id, false, false, CHECKBOX);
 		}

-		if($one_attribute_id === FALSE)
+		if($one_attribute_id === false)
 		{
-			$one_attribute_id = $this->save_value('1', $definition_id, FALSE, FALSE, CHECKBOX);
+			$one_attribute_id = $this->save_value('1', $definition_id, false, false, CHECKBOX);
+			$one_attribute_id = $this->save_value('1', $definition_id, false, false, CHECKBOX);
 		}

 		return [$zero_attribute_id, $one_attribute_id];
@@ -551,11 +558,12 @@ class Attribute extends Model

 		if($this->link_exists($item_id, $definition_id))
 		{
+			$builder->set(['attribute_id' => $attribute_id]);
 			$builder->where('definition_id', $definition_id);
 			$builder->where('item_id', $item_id);
 			$builder->where('sale_id', null);
 			$builder->where('receiving_id', null);
-			$builder->update(['attribute_id' => $attribute_id]);
+			$builder->update();
 		}
 		else
 		{
@@ -705,6 +713,13 @@ class Attribute extends Model
 		$builder->ignore(true)->setQueryAsData(new RawSql($query), null, 'item_id, definition_id, attribute_id, '. $sale_receiving_fk )->insertBatch();
 	}

+	/**
+	 * Gets search suggestions (attribute values) for a specific attribute definition given a search term and definition_id
+	 *
+	 * @param int $definition_id
+	 * @param string $term
+	 * @return array
+	 */
 	public function get_suggestions(int $definition_id, string $term): array
 	{
 		$suggestions = [];
@@ -718,16 +733,23 @@ class Attribute extends Model
 		$builder->where('definition.definition_id', $definition_id);
 		$builder->orderBy('attribute_value','ASC');

-		foreach($builder->get()->getResult() as $row)
+		foreach($builder->get()->getResult('array') as $suggestion)
 		{
-			$row_array = (array)$row;
-			$suggestions[] = ['value' => $row_array['attribute_id'], 'label' => $row_array['attribute_value']];
+			$suggestions[] = ['value' => $suggestion['attribute_id'], 'label' => $suggestion['attribute_value']];
 		}

 		return $suggestions;
 	}

-	public function save_value(string $attribute_value, int $definition_id, $item_id = FALSE, $attribute_id = FALSE, string $definition_type = DROPDOWN): int
+	/**
+	 * @param string $attribute_value
+	 * @param int $definition_id
+	 * @param bool $item_id
+	 * @param bool $attribute_id
+	 * @param string $definition_type
+	 * @return int
+	 */
+	public function save_value(string $attribute_value, int $definition_id, $item_id = false, $attribute_id = false, string $definition_type = DROPDOWN): int
 	{
 		$config = config(OSPOS::class)->settings;
 		$locale_date_format = $config['dateformat'];
@@ -740,7 +762,7 @@ class Attribute extends Model
 		//Update attribute_value
 			$attribute_id = $this->value_exists($attribute_value, $definition_type);

-			if($attribute_id === FALSE)
+			if($attribute_id === false)
 			{
 				switch($definition_type)	//TODO: Duplicated code
 				{
@@ -758,18 +780,21 @@ class Attribute extends Model
 				}

 				$builder = $this->db->table('attribute_values');
-				$builder->insert(["attribute_$data_type" => $attribute_value]);
+				$builder->set(["attribute_$data_type" => $attribute_value]);
+				$builder->insert();

 				$attribute_id = $this->db->insertID();
 			}

-			$builder = $this->db->table('attribute_links');
 			$data = [
 				'attribute_id' => empty($attribute_id) ? NULL : $attribute_id,
 				'item_id' => empty($item_id) ? NULL : $item_id,
 				'definition_id' => $definition_id
 			];
-			$builder->insert($data);
+
+			$builder = $this->db->table('attribute_links');
+			$builder->set($data);
+			$builder->insert();
 		}
 		//Existing Attribute
 		else
@@ -790,9 +815,11 @@ class Attribute extends Model
 			}

 			$builder = $this->db->table('attribute_values');
+			$builder->set(["attribute_$data_type" => $attribute_value]);
 			$builder->where('attribute_id', $attribute_id);
-			$builder->update(["attribute_$data_type" => $attribute_value]);
+			$builder->update();
 		}
+		log_message('error', 'save_value result: ' . $this->db->transStatus());

 		$this->db->transComplete();

--- a/app/Views/attributes/item.php
+++ b/app/Views/attributes/item.php
@@ -3,6 +3,7 @@
 * @var array $definition_names
 * @var array $definition_values
 * @var int $item_id
+ * @var array $config
 */
 ?>
 <div class="form-group form-group-sm">
@@ -26,52 +27,50 @@ foreach($definition_values as $definition_id => $definition_value)
 				echo form_hidden("attribute_ids[$definition_id]", $definition_value['attribute_id']);
 				$attribute_value = $definition_value['attribute_value'];

-				if ($definition_value['definition_type'] == DATE)
+				switch($definition_value['definition_type'])
 				{
-					$value = (empty($attribute_value) || empty($attribute_value->attribute_date)) ? NOW : strtotime($attribute_value->attribute_date);
-					echo form_input ([
-						'name' => "attribute_links[$definition_id]",
-						'value' => to_date($value),
-						'class' => 'form-control input-sm datetime',
-						'data-definition-id' => $definition_id,
-						'readonly' => 'true'
-					]);
-				}
-				else if ($definition_value['definition_type'] == DROPDOWN)	//TODO: === ?
-				{
-					$selected_value = $definition_value['selected_value'];
-					echo form_dropdown("attribute_links[$definition_id]", $definition_value['values'], $selected_value, "class='form-control' data-definition-id='$definition_id'");
-				}
-				else if ($definition_value['definition_type'] == TEXT)	//TODO: === ?
-				{
-					$value = (empty($attribute_value) || empty($attribute_value->attribute_value)) ? $definition_value['selected_value'] : $attribute_value->attribute_value;
-					echo form_input("attribute_links[$definition_id]", $value, "class='form-control valid_chars' data-definition-id='$definition_id'");
-				}
-				else if ($definition_value['definition_type'] == DECIMAL)	//TODO: === ?
-				{
-					$value = (empty($attribute_value) || empty($attribute_value->attribute_decimal)) ? $definition_value['selected_value'] : $attribute_value->attribute_decimal;
-					echo form_input("attribute_links[$definition_id]", $value, "class='form-control valid_chars' data-definition-id='$definition_id'");
-				}
-				else if ($definition_value['definition_type'] == CHECKBOX)	//TODO: === ?
-				{
-					$value = (empty($attribute_value) || empty($attribute_value->attribute_value)) ? $definition_value['selected_value'] : $attribute_value->attribute_value;
+					case DATE:
+						$value = (empty($attribute_value) || empty($attribute_value->attribute_date)) ? NOW : strtotime($attribute_value->attribute_date);
+						echo form_input ([
+							'name' => "attribute_links[$definition_id]",
+							'value' => to_date($value),
+							'class' => 'form-control input-sm datetime',
+							'data-definition-id' => $definition_id,
+							'readonly' => 'true'
+						]);
+						break;
+					case DROPDOWN:
+						$selected_value = $definition_value['selected_value'];
+						echo form_dropdown("attribute_links[$definition_id]", $definition_value['values'], $selected_value, "class='form-control' data-definition-id='$definition_id'");
+						break;
+					case TEXT:
+						$value = (empty($attribute_value) || empty($attribute_value->attribute_value)) ? $definition_value['selected_value'] : $attribute_value->attribute_value;
+						echo form_input("attribute_links[$definition_id]", $value, "class='form-control valid_chars' data-definition-id='$definition_id'");
+						break;
+					case DECIMAL:
+						$value = (empty($attribute_value) || empty($attribute_value->attribute_decimal)) ? $definition_value['selected_value'] : $attribute_value->attribute_decimal;
+						echo form_input("attribute_links[$definition_id]", $value, "class='form-control valid_chars' data-definition-id='$definition_id'");
+						break;
+					case CHECKBOX:
+						$value = (empty($attribute_value) || empty($attribute_value->attribute_value)) ? $definition_value['selected_value'] : $attribute_value->attribute_value;

-				//Sends 0 if the box is unchecked instead of not sending anything.
-					echo form_input ([
-						'type' => 'hidden',
-						'name' => "attribute_links[$definition_id]",
-						'id' => "attribute_links[$definition_id]",
-						'value' => 0,
-						'data-definition-id' => $definition_id
-					]);
-					echo form_checkbox ([
-						'name' => "attribute_links[$definition_id]",
-						'id' => "attribute_links[$definition_id]",
-						'value' => 1,
-						'checked' => $value == 1,
-						'class' => 'checkbox-inline',
-						'data-definition-id' => $definition_id
-					]);
+						//Sends 0 if the box is unchecked instead of not sending anything.
+						echo form_input ([
+							'type' => 'hidden',
+							'name' => "attribute_links[$definition_id]",
+							'id' => "attribute_links[$definition_id]",
+							'value' => 0,
+							'data-definition-id' => $definition_id
+						]);
+						echo form_checkbox ([
+							'name' => "attribute_links[$definition_id]",
+							'id' => "attribute_links[$definition_id]",
+							'value' => 1,
+							'checked' => $value == 1,
+							'class' => 'checkbox-inline',
+							'data-definition-id' => $definition_id
+						]);
+						break;
 				}
 			?>
 			<span class="input-group-addon input-sm btn btn-default remove_attribute_btn"><span class="glyphicon glyphicon-trash"></span></span>
--- a/app/Views/items/form_count_details.php
+++ b/app/Views/items/form_count_details.php
@@ -6,6 +6,7 @@
 */

 use App\Models\Employee;
+use App\Models\Inventory;

 ?>
 <?php echo form_open('items', ['id' => 'item_form', 'class' => 'form-horizontal']) ?>
@@ -91,13 +92,12 @@ use App\Models\Employee;
 	</thead>
 	<tbody id="inventory_result">
 		<?php
-			/**
-			 * the tbody content of the table will be filled in by the javascript (see bottom of page)
-			 */
-			$inventory_array = $this->Inventory->get_inventory_data_for_item($item_info->item_id)->getResultArray();
-			$employee_name = [];
-
+			 //the tbody content of the table will be filled in by the javascript (see bottom of page)
 			$employee = model(Employee::class);
+			$inventory = model(Inventory::class);
+
+			$inventory_array = $inventory->get_inventory_data_for_item($item_info->item_id)->getResultArray();
+			$employee_name = [];

 			foreach($inventory_array as $row)
 			{
@@ -111,53 +111,53 @@ use App\Models\Employee;
 <script type="text/javascript">
 $(document).ready(function()
 {
-    display_stock(<?php echo json_encode(key(esc($stock_locations, 'raw'))) ?>);
+	display_stock(<?php echo json_encode(key(esc($stock_locations, 'raw'))) ?>);
 });

 function display_stock(location_id)
 {
-    var item_quantities = <?php echo json_encode(esc($item_quantities, 'raw')) ?>;
-    document.getElementById("quantity").value = parseFloat(item_quantities[location_id]).toFixed(<?php echo quantity_decimals() ?>);
+	var item_quantities = <?php echo json_encode(esc($item_quantities, 'raw')) ?>;
+	document.getElementById("quantity").value = parseFloat(item_quantities[location_id]).toFixed(<?php echo quantity_decimals() ?>);

-    var inventory_data = <?php echo json_encode(esc($inventory_array), 'raw') ?>;
-    var employee_data = <?php echo json_encode(esc($employee_name, 'raw')) ?>;
+	var inventory_data = <?php echo json_encode(esc($inventory_array, 'raw')) ?>;
+	var employee_data = <?php echo json_encode(esc($employee_name, 'raw')) ?>;

-    var table = document.getElementById("inventory_result");
+	var table = document.getElementById("inventory_result");

-    // Remove old query from tbody
-    var rowCount = table.rows.length;
-    for (var index = rowCount; index > 0; index--)
-    {
-        table.deleteRow(index-1);
-    }
+	// Remove old query from tbody
+	var rowCount = table.rows.length;
+	for (var index = rowCount; index > 0; index--)
+	{
+		table.deleteRow(index-1);
+	}

-    // Add new query to tbody
-    for (var index = 0; index < inventory_data.length; index++)
-    {
-        var data = inventory_data[index];
-        if(data['trans_location'] == location_id)
-        {
-            var tr = document.createElement('tr');
+	// Add new query to tbody
+	for (var index = 0; index < inventory_data.length; index++)
+	{
+		var data = inventory_data[index];
+		if(data['trans_location'] == location_id)
+		{
+			var tr = document.createElement('tr');

-            var td = document.createElement('td');
-            td.appendChild(document.createTextNode(data['trans_date']));
-            tr.appendChild(td);
+			var td = document.createElement('td');
+			td.appendChild(document.createTextNode(data['trans_date']));
+			tr.appendChild(td);

-            td = document.createElement('td');
-            td.appendChild(document.createTextNode(employee_data[index]));
-            tr.appendChild(td);
+			td = document.createElement('td');
+			td.appendChild(document.createTextNode(employee_data[index]));
+			tr.appendChild(td);

-            td = document.createElement('td');
-            td.appendChild(document.createTextNode(parseFloat(data['trans_inventory']).toFixed(<?php echo quantity_decimals() ?>)));
+			td = document.createElement('td');
+			td.appendChild(document.createTextNode(parseFloat(data['trans_inventory']).toFixed(<?php echo quantity_decimals() ?>)));
 			td.setAttribute("style", "text-align:center");
-            tr.appendChild(td);
+			tr.appendChild(td);

-            td = document.createElement('td');
-            td.appendChild(document.createTextNode(data['trans_comment']));
-            tr.appendChild(td);
+			td = document.createElement('td');
+			td.appendChild(document.createTextNode(data['trans_comment']));
+			tr.appendChild(td);

-            table.appendChild(tr);
-        }
-    }
+			table.appendChild(tr);
+		}
+	}
 }
 </script>