From cf73ffa8252add719cf47d0cbcf8ac356336badb Mon Sep 17 00:00:00 2001 From: jekkos Date: Thu, 13 Feb 2025 00:13:56 +0100 Subject: [PATCH] Fix attribute dropdown delete (#4176) --- app/Controllers/Attributes.php | 9 ++-- ...0240826000000_fix_duplicate_attributes.php | 48 ++++--------------- ...000000_fix_attributes_cascading_delete.php | 29 +++++++++++ app/Helpers/migration_helper.php | 35 +++++++++++++- 4 files changed, 77 insertions(+), 44 deletions(-) create mode 100644 app/Database/Migrations/20250213000000_fix_attributes_cascading_delete.php diff --git a/app/Controllers/Attributes.php b/app/Controllers/Attributes.php index 6cb483c70..a16401b68 100644 --- a/app/Controllers/Attributes.php +++ b/app/Controllers/Attributes.php @@ -231,14 +231,15 @@ class Attributes extends Secure_Controller } /** - * 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 + * AJAX called function to delete an attribute value. This is called when a dropdown item is removed. + * + * @param string $attribute_value * @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? + public function delete_value(string $attribute_value): bool { - return $this->attribute->delete_value($attribute_id, NO_DEFINITION_ID); + return $this->attribute->delete_value($attribute_value, NO_DEFINITION_ID); } /** diff --git a/app/Database/Migrations/20240826000000_fix_duplicate_attributes.php b/app/Database/Migrations/20240826000000_fix_duplicate_attributes.php index 6c4ff8d99..ad80445d0 100644 --- a/app/Database/Migrations/20240826000000_fix_duplicate_attributes.php +++ b/app/Database/Migrations/20240826000000_fix_duplicate_attributes.php @@ -18,7 +18,15 @@ class fix_duplicate_attributes extends Migration helper('migration'); - $this->drop_foreign_key_constraints(); + $foreignKeys = [ + 'ospos_attribute_links_ibfk_1', + 'ospos_attribute_links_ibfk_2', + 'ospos_attribute_links_ibfk_3', + 'ospos_attribute_links_ibfk_4', + 'ospos_attribute_links_ibfk_5' + ]; + + drop_foreign_key_constraints($foreignKeys, 'ospos_attribute_links'); execute_script(APPPATH . 'Database/Migrations/sqlscripts/3.4.0_attribute_links_unique_constraint.sql'); } @@ -57,44 +65,6 @@ class fix_duplicate_attributes extends Migration } } - /** - * Drops the foreign key constraints from the attribute_links table. - * This is required to successfully create the generated unique constraint. - * - * @return void - */ - private function drop_foreign_key_constraints(): void - { - $foreignKeys = [ - 'ospos_attribute_links_ibfk_1', - 'ospos_attribute_links_ibfk_2', - 'ospos_attribute_links_ibfk_3', - 'ospos_attribute_links_ibfk_4', - 'ospos_attribute_links_ibfk_5' - ]; - - $current_prefix = $this->db->getPrefix(); - $this->db->setPrefix(''); - $database_name = $this->db->database; - - foreach ($foreignKeys as $fk) - { - $builder = $this->db->table('INFORMATION_SCHEMA.TABLE_CONSTRAINTS'); - $builder->select('CONSTRAINT_NAME'); - $builder->where('TABLE_SCHEMA', $database_name); - $builder->where('TABLE_NAME', 'ospos_attribute_links'); - $builder->where('CONSTRAINT_TYPE', 'FOREIGN KEY'); - $builder->where('CONSTRAINT_NAME', $fk); - $query = $builder->get(); - - if($query->getNumRows() > 0) - { - $this->db->query("ALTER TABLE `ospos_attribute_links` DROP FOREIGN KEY `$fk`"); - } - } - - $this->db->setPrefix($current_prefix); - } /** * Revert a migration step. diff --git a/app/Database/Migrations/20250213000000_fix_attributes_cascading_delete.php b/app/Database/Migrations/20250213000000_fix_attributes_cascading_delete.php new file mode 100644 index 000000000..a3e56d333 --- /dev/null +++ b/app/Database/Migrations/20250213000000_fix_attributes_cascading_delete.php @@ -0,0 +1,29 @@ +db->query("ALTER TABLE `ospos_attribute_links` ADD CONSTRAINT `ospos_attribute_links_ibfk_1` FOREIGN KEY (`definition_id`) REFERENCES `ospos_attribute_definitions` (`definition_id`) ON DELETE CASCADE;"); + $this->db->query("ALTER TABLE `ospos_attribute_links` ADD CONSTRAINT `ospos_attribute_links_ibfk_2` FOREIGN KEY (`attribute_id`) REFERENCES `ospos_attribute_values` (`attribute_id`) ON DELETE CASCADE;"); + } + + /** + * Revert a migration step. + */ + public function down(): void + { + + } +} diff --git a/app/Helpers/migration_helper.php b/app/Helpers/migration_helper.php index b610cf533..289ebcea4 100644 --- a/app/Helpers/migration_helper.php +++ b/app/Helpers/migration_helper.php @@ -30,4 +30,37 @@ function execute_script(string $path): void } error_log("Migrated to $version"); -} \ No newline at end of file +} + +/** + * Drops the foreign key constraints from the attribute_links table. + * This is required to successfully create the generated unique constraint. + * + * @return void + */ +function drop_foreign_key_constraints(array $foreignKeys, string $table): void +{ + $db = Database::connect(); + + $current_prefix = $db->getPrefix(); + $db->setPrefix(''); + $database_name = $db->database; + + foreach ($foreignKeys as $fk) + { + $builder = $db->table('INFORMATION_SCHEMA.TABLE_CONSTRAINTS'); + $builder->select('CONSTRAINT_NAME'); + $builder->where('TABLE_SCHEMA', $database_name); + $builder->where('TABLE_NAME', $table); + $builder->where('CONSTRAINT_TYPE', 'FOREIGN KEY'); + $builder->where('CONSTRAINT_NAME', $fk); + $query = $builder->get(); + + if($query->getNumRows() > 0) + { + $db->query("ALTER TABLE `$table` DROP FOREIGN KEY `$fk`"); + } + } + + $db->setPrefix($current_prefix); +}