From 6eb22276f35c1508f76e7dc3df912c791d033ca5 Mon Sep 17 00:00:00 2001 From: objecttothis Date: Tue, 23 Jul 2024 17:52:21 +0400 Subject: [PATCH] Locale handling of decimals in attribute saves - Added check in controller to convert locale-specific decimal formats to use a period decimal separator. - Added PHPdoc explanation Signed-off-by: objecttothis Add TODO to clarify workaround Signed-off-by: objecttothis Fixed bugs in SQL - Added checks before attempting to delete non-existing values. - Corrected function which deletes duplicate attribute values and replaces the attribute_ids Signed-off-by: objecttothis --- app/Controllers/Items.php | 5 ++ app/Controllers/Persons.php | 2 +- .../20210422000000_database_optimizations.php | 87 ++++++++++++------- .../3.4.0_database_optimizations.sql | 4 +- app/Helpers/locale_helper.php | 3 +- 5 files changed, 67 insertions(+), 34 deletions(-) diff --git a/app/Controllers/Items.php b/app/Controllers/Items.php index b95e0d081..ea295f273 100644 --- a/app/Controllers/Items.php +++ b/app/Controllers/Items.php @@ -779,6 +779,11 @@ class Items extends Secure_Controller { $definition_type = $this->attribute->get_info($definition_id)->definition_type; + if($definition_type = DECIMAL) + { + $attribute_value = prepare_decimal($attribute_value); + } + $attribute_id = $definition_type === DROPDOWN ? $attribute_value : $this->attribute->save_value($attribute_value, $definition_id, $item_id, $attribute_ids[$definition_id], $definition_type); diff --git a/app/Controllers/Persons.php b/app/Controllers/Persons.php index d8e0a0b18..351515c82 100644 --- a/app/Controllers/Persons.php +++ b/app/Controllers/Persons.php @@ -65,7 +65,7 @@ abstract class Persons extends Secure_Controller { $adjusted_name = str_name_case($input); - // Use preg_replace to match HTML entities and convert them to lowercase. + //TODO:Use preg_replace to match HTML entities and convert them to lowercase. This is a workaround for https://github.com/tamtamchik/namecase/issues/20 return preg_replace_callback('/&[a-zA-Z0-9#]+;/', function($matches) { return strtolower($matches[0]); }, $adjusted_name); } } diff --git a/app/Database/Migrations/20210422000000_database_optimizations.php b/app/Database/Migrations/20210422000000_database_optimizations.php index 48e1c3095..347499018 100644 --- a/app/Database/Migrations/20210422000000_database_optimizations.php +++ b/app/Database/Migrations/20210422000000_database_optimizations.php @@ -5,6 +5,7 @@ namespace App\Database\Migrations; use CodeIgniter\Database\Migration; use CodeIgniter\Database\ResultInterface; use App\Models\Attribute; +use Config\Database; use Config\OSPOS; use DateTime; @@ -45,46 +46,64 @@ class Migration_database_optimizations extends Migration $builder = $this->db->table('attribute_values'); $builder->delete(['attribute_id' => $attribute_value['attribute_id']]); - //TODO: This should be converted to using CI4 QueryBuilder - $query = 'SELECT links.definition_id, links.item_id, links.attribute_id, defs.definition_type' - . ' FROM ospos_attribute_links links' - . ' JOIN ospos_attribute_definitions defs ON defs.definition_id = links.definition_id' - . ' WHERE attribute_id = ' . $attribute_value['attribute_id']; - $attribute_links = $this->db->query($query); - $builder = $this->db->table('attribute_links'); + $builder->select('links.definition_id, links.item_id, links.attribute_id, defs.definition_type'); + $builder->join('attribute_definitions defs', 'defs.definition_id = links.definition_id'); + $builder->where('attribute_id', $attribute_value['attribute_id']); + $attribute_links = $builder->get(); - foreach($attribute_links->getResultArray() as $attribute_link) + if($attribute_links) { - $builder->where('attribute_id', $attribute_link['attribute_id']); - $builder->where('item_id', $attribute_link['item_id']); - $builder->delete(); + $builder = $this->db->table('attribute_links'); - switch($attribute_link['definition_type']) + foreach($attribute_links->getResultArray() as $attribute_link) { - case DECIMAL: - $value = $attribute_value['attribute_decimal']; - break; - case DATE: - $config = config(OSPOS::class)->settings; - $attribute_date = DateTime::createFromFormat('Y-m-d', $attribute_value['attribute_date']); - $value = $attribute_date->format($config['dateformat']); - break; - default: - $value = $attribute_value['attribute_value']; - break; - } + $builder->where('attribute_id', $attribute_link['attribute_id']); + $builder->where('item_id', $attribute_link['item_id']); + $builder->delete(); - $attribute->save_value($value, $attribute_link['definition_id'], $attribute_link['item_id'], false, $attribute_link['definition_type']); + switch($attribute_link['definition_type']) + { + case DECIMAL: + $value = $attribute_value['attribute_decimal']; + break; + case DATE: + $config = config(OSPOS::class)->settings; + $attribute_date = DateTime::createFromFormat('Y-m-d', $attribute_value['attribute_date']); + $value = $attribute_date->format($config['dateformat']); + break; + default: + $value = $attribute_value['attribute_value']; + break; + } + + $attribute->save_value($value, $attribute_link['definition_id'], $attribute_link['item_id'], false, $attribute_link['definition_type']); + } } } $this->db->transComplete(); + $this->delete_index('customers', 'person_id'); + $this->delete_index('employees', 'person_id'); + $this->delete_index('suppliers', 'person_id'); + helper('migration'); execute_script(APPPATH . 'Database/Migrations/sqlscripts/3.4.0_database_optimizations.sql'); error_log('Migrating database_optimizations completed'); } + private function delete_index(string $table, string $index): void + { + $result = $this->db->query('SELECT COUNT(*) FROM information_schema.statistics WHERE table_schema = DATABASE() AND table_name = \'' . $this->db->getPrefix() . "$table' AND index_name = '$index'"); + $index_exists = $result->getRowArray()['COUNT(*)'] > 0; + + if($index_exists) + { + $forge = Database::forge(); + $forge->dropKey($table, $index, false); + } + } + /** * Given the type of attribute, deletes any duplicates it finds in the attribute_values table and reassigns those */ @@ -96,15 +115,22 @@ class Migration_database_optimizations extends Migration $column = 'attribute_' . strtolower($attribute_type); $builder = $this->db->table('attribute_values'); - $builder->select("$column, attribute_id"); + $builder->select("$column"); $builder->groupBy($column); $builder->having("COUNT($column) > 1"); $duplicated_values = $builder->get(); foreach($duplicated_values->getResultArray() as $duplicated_value) { + $subquery_builder = $this->db->table('attribute_values'); + $subquery_builder->select('attribute_id'); + $subquery_builder->where($column, $duplicated_value[$column]); + $subquery = $subquery_builder->getCompiledSelect(); + + $builder = $this->db->table('attribute_values'); $builder->select('attribute_id'); $builder->where($column, $duplicated_value[$column]); + $builder->where("attribute_id IN ($subquery)", null, false); $attribute_ids_to_fix = $builder->get(); $this->reassign_duplicate_attribute_values($attribute_ids_to_fix, $duplicated_value); @@ -121,15 +147,18 @@ class Migration_database_optimizations extends Migration */ private function reassign_duplicate_attribute_values(ResultInterface $attribute_ids_to_fix, array $attribute_value): void { - foreach($attribute_ids_to_fix->getResultArray() as $attribute_id) + $attribute_ids = $attribute_ids_to_fix->getResultArray(); + $retain_attribute_id = $attribute_ids[0]['attribute_id']; + + foreach($attribute_ids as $attribute_id) { //Update attribute_link with the attribute_id we are keeping $builder = $this->db->table('attribute_links'); $builder->where('attribute_id', $attribute_id['attribute_id']); - $builder->update(['attribute_id' => $attribute_value['attribute_id']]); + $builder->update(['attribute_id' => $retain_attribute_id]); //Delete the row from attribute_values if it isn't our keeper - if($attribute_id['attribute_id'] !== $attribute_value['attribute_id']) + if($attribute_id['attribute_id'] !== $retain_attribute_id) { $builder = $this->db->table('attribute_values'); $builder->delete(['attribute_id' => $attribute_id['attribute_id']]); diff --git a/app/Database/Migrations/sqlscripts/3.4.0_database_optimizations.sql b/app/Database/Migrations/sqlscripts/3.4.0_database_optimizations.sql index 8306d019f..c716e8dd7 100644 --- a/app/Database/Migrations/sqlscripts/3.4.0_database_optimizations.sql +++ b/app/Database/Migrations/sqlscripts/3.4.0_database_optimizations.sql @@ -75,6 +75,7 @@ ALTER TABLE `ospos_expense_categories` ADD INDEX(`category_description`); ALTER TABLE `ospos_giftcards` MODIFY `deleted` tinyint(1) DEFAULT 0 NOT NULL; #ospos_items table +ALTER TABLE `ospos_items` DROP FOREIGN KEY `ospos_items_ibfk_1`; ALTER TABLE `ospos_items` MODIFY `deleted` tinyint(1) DEFAULT 0 NOT NULL; ALTER TABLE `ospos_items` MODIFY `stock_type` tinyint(1) DEFAULT 0 NOT NULL; ALTER TABLE `ospos_items` MODIFY `item_type` tinyint(1) DEFAULT 0 NOT NULL; @@ -140,9 +141,6 @@ ALTER TABLE `ospos_suppliers` ADD CONSTRAINT `ospos_suppliers_ibfk_1` FOREIGN KE ALTER TABLE `ospos_tax_categories` MODIFY `deleted` tinyint(1) DEFAULT 0 NOT NULL; ALTER TABLE `ospos_tax_categories` MODIFY `tax_group_sequence` tinyint(1) NOT NULL; -#ospos_tax_codes table -ALTER TABLE `ospos_tax_codes` MODIFY `deleted` tinyint(1) DEFAULT 0 NOT NULL; - #ospos_tax_jurisdictions table ALTER TABLE `ospos_tax_jurisdictions` MODIFY `deleted` tinyint(1) DEFAULT 0 NOT NULL; ALTER TABLE `ospos_tax_jurisdictions` MODIFY `tax_group_sequence` tinyint(1) DEFAULT 0 NOT NULL; diff --git a/app/Helpers/locale_helper.php b/app/Helpers/locale_helper.php index 8028ae9de..31849fcde 100644 --- a/app/Helpers/locale_helper.php +++ b/app/Helpers/locale_helper.php @@ -428,7 +428,8 @@ function to_quantity_decimals(?string $number): string } /** - * @param string|null $number + * Converts a float to locale-specific number format for display. + * * @param string|null $decimals * @param int $type * @return string