From 3f5d7edf0b75fa2d0ec8d523a6ef2c000f61301d Mon Sep 17 00:00:00 2001 From: aFolletete Date: Tue, 14 Feb 2012 15:08:40 +0000 Subject: [PATCH] // fixed bug #PSTEST-552 // Clean code git-svn-id: http://dev.prestashop.com/svn/v1/branches/1.5.x@13312 b9a71923-0436-4b27-9f14-aed3839534dd --- classes/Carrier.php | 106 +++++++++++++----- classes/controller/AdminController.php | 9 +- controllers/admin/AdminCarriersController.php | 84 +++++++------- 3 files changed, 121 insertions(+), 78 deletions(-) diff --git a/classes/Carrier.php b/classes/Carrier.php index 8531ed449..978a923b1 100644 --- a/classes/Carrier.php +++ b/classes/Carrier.php @@ -628,9 +628,32 @@ class CarrierCore extends ObjectModel */ public function addZone($id_zone) { - return Db::getInstance()->execute(' + if (Db::getInstance()->execute(' INSERT INTO `'._DB_PREFIX_.'carrier_zone` (`id_carrier` , `id_zone`) - VALUES ('.(int)$this->id.', '.(int)$id_zone.')'); + VALUES ('.(int)$this->id.', '.(int)$id_zone.') + ')) + { + // Get all ranges for this carrier + $ranges_price = RangePrice::getRanges($this->id); + $ranges_weight = RangeWeight::getRanges($this->id); + // Create row in ps_delivery table + if (count($ranges_price) || count($ranges_weight)) + { + $sql = 'INSERT INTO `'._DB_PREFIX_.'delivery` (`id_carrier`, `id_range_price`, `id_range_weight`, `id_zone`, `price`) VALUES '; + if (count($ranges_price)) + foreach ($ranges_price as $range) + $sql .= '('.(int)$this->id.', '.(int)$range['id_range_price'].', 0, '.(int)$id_zone.', 0),'; + + if (count($ranges_weight)) + foreach ($ranges_weight as $range) + $sql .= '('.(int)$this->id.', 0, '.(int)$range['id_range_weight'].', '.(int)$id_zone.', 0),'; + $sql = rtrim($sql, ','); + + return Db::getInstance()->execute($sql); + } + return true; + } + return false; } /** @@ -638,10 +661,18 @@ class CarrierCore extends ObjectModel */ public function deleteZone($id_zone) { - return Db::getInstance()->execute(' + if (Db::getInstance()->execute(' DELETE FROM `'._DB_PREFIX_.'carrier_zone` WHERE `id_carrier` = '.(int)$this->id.' - AND `id_zone` = '.(int)$id_zone.' LIMIT 1'); + AND `id_zone` = '.(int)$id_zone.' LIMIT 1 + ')) + { + return Db::getInstance()->execute(' + DELETE FROM `'._DB_PREFIX_.'delivery` + WHERE `id_carrier` = '.(int)$this->id.' + AND `id_zone` = '.(int)$id_zone); + } + return false; } /** @@ -738,7 +769,7 @@ class CarrierCore extends ObjectModel public function copyCarrierData($old_id) { if (!Validate::isUnsignedId($old_id)) - die(Tools::displayError()); + throw new PrestaShopException('Incorrect identifier for carrier'); if (!$this->id) return false; @@ -754,43 +785,56 @@ class CarrierCore extends ObjectModel // Copy existing ranges price foreach (array('range_price', 'range_weight') as $range) { - $sql = 'SELECT id_'.$range.' id_range, delimiter1, delimiter2 FROM `'._DB_PREFIX_.$range.'` - WHERE id_carrier = '.(int)$old_id; - $res = Db::getInstance()->executeS($sql); - foreach ($res as $val) - { - $sql = 'INSERT INTO `'._DB_PREFIX_.$range.'` (`id_carrier`, `delimiter1`, `delimiter2`) - VALUES ('.$this->id.','.(float)$val['delimiter1'].','.(float)$val['delimiter2'].')'; - Db::getInstance()->execute($sql); - $range_id = (int)Db::getInstance()->Insert_ID(); + $res = Db::getInstance()->executeS(' + SELECT `id_'.$range.'` as id_range, `delimiter1`, `delimiter2` + FROM `'._DB_PREFIX_.$range.'` + WHERE `id_carrier` = '.(int)$old_id); + if (count($res)) + foreach ($res as $val) + { + Db::getInstance()->execute(' + INSERT INTO `'._DB_PREFIX_.$range.'` (`id_carrier`, `delimiter1`, `delimiter2`) + VALUES ('.$this->id.','.(float)$val['delimiter1'].','.(float)$val['delimiter2'].')'); + $range_id = (int)Db::getInstance()->Insert_ID(); - $range_price_id = ($range == 'range_price') ? $range_id : 'NULL'; - $range_weight_id = ($range == 'range_weight') ? $range_id : 'NULL'; - $sql = 'INSERT INTO '._DB_PREFIX_.'delivery (id_carrier, id_range_price, id_range_weight, id_zone, price) - SELECT '.$this->id.', '.$range_price_id.', '.$range_weight_id.', id_zone, price FROM '._DB_PREFIX_.'delivery - WHERE id_carrier = '.(int)$old_id.' AND '.(($range == 'range_price') ? 'id_range_price' : 'id_range_weight').' = '.$val['id_range']; + $range_price_id = ($range == 'range_price') ? $range_id : 'NULL'; + $range_weight_id = ($range == 'range_weight') ? $range_id : 'NULL'; - Db::getInstance()->execute($sql); - } + Db::getInstance()->execute(' + INSERT INTO `'._DB_PREFIX_.'delivery` (`id_carrier`, `id_shop`, `id_group_shop`, `id_range_price`, `id_range_weight`, `id_zone`, `price`) ( + SELECT '.(int)$this->id.', `id_shop`, `id_group_shop`, '.(int)$range_price_id.', '.(int)$range_weight_id.', `id_zone`, `price` + FROM `'._DB_PREFIX_.'delivery` + WHERE `id_carrier` = '.(int)$old_id.' + AND `id_'.$range.'` = '.(int)$val['id_range'].' + ) + '); + } } // Copy existing zones - $res = Db::getInstance()->executeS('SELECT * FROM `'._DB_PREFIX_.'carrier_zone` + $res = Db::getInstance()->executeS(' + SELECT * + FROM `'._DB_PREFIX_.'carrier_zone` WHERE id_carrier = '.(int)$old_id); foreach ($res as $val) Db::getInstance()->execute(' - INSERT INTO `'._DB_PREFIX_.'carrier_zone` (`id_carrier`, `id_zone`) - VALUES ('.$this->id.','.(int)$val['id_zone'].')'); + INSERT INTO `'._DB_PREFIX_.'carrier_zone` (`id_carrier`, `id_zone`) + VALUES ('.$this->id.','.(int)$val['id_zone'].') + '); //Copy default carrier - if ((int)Configuration::get('PS_CARRIER_DEFAULT') == $old_id) + if (Configuration::get('PS_CARRIER_DEFAULT') == $old_id) Configuration::updateValue('PS_CARRIER_DEFAULT', (int)$this->id); // Copy reference - $id_reference = Db::getInstance()->getValue('SELECT `id_reference` FROM `'._DB_PREFIX_.$this->def['table'].'` WHERE id_carrier = '.$old_id); - Db::getInstance()->execute('UPDATE `'._DB_PREFIX_.$this->def['table'].'` - SET `id_reference` = '.$id_reference.' - WHERE `id_carrier` = '.$this->id); + $id_reference = Db::getInstance()->getValue(' + SELECT `id_reference` + FROM `'._DB_PREFIX_.$this->def['table'].'` + WHERE id_carrier = '.(int)$old_id); + Db::getInstance()->execute(' + UPDATE `'._DB_PREFIX_.$this->def['table'].'` + SET `id_reference` = '.(int)$id_reference.' + WHERE `id_carrier` = '.(int)$this->id); } /** @@ -845,7 +889,7 @@ class CarrierCore extends ObjectModel $shipping_method = $this->getShippingMethod(); if ($shipping_method == Carrier::SHIPPING_METHOD_WEIGHT) return 'range_weight'; - else if ($shipping_method == Carrier::SHIPPING_METHOD_PRICE) + elseif ($shipping_method == Carrier::SHIPPING_METHOD_PRICE) return 'range_price'; return false; } @@ -855,7 +899,7 @@ class CarrierCore extends ObjectModel $shipping_method = $this->getShippingMethod(); if ($shipping_method == Carrier::SHIPPING_METHOD_WEIGHT) return new RangeWeight(); - else if ($shipping_method == Carrier::SHIPPING_METHOD_PRICE) + elseif ($shipping_method == Carrier::SHIPPING_METHOD_PRICE) return new RangePrice(); return false; } diff --git a/classes/controller/AdminController.php b/classes/controller/AdminController.php index b9846ee6c..82fd596d8 100644 --- a/classes/controller/AdminController.php +++ b/classes/controller/AdminController.php @@ -2310,11 +2310,14 @@ class AdminControllerCore extends Controller if (!$type) return; - Db::getInstance()->execute('DELETE FROM '._DB_PREFIX_.$this->table.'_'.$type.($id_object ? ' WHERE `'.$this->identifier.'`='.(int)$id_object : '')); + Db::getInstance()->execute(' + DELETE FROM '._DB_PREFIX_.$this->table.'_'.$type.($id_object ? ' + WHERE `'.$this->identifier.'`='.(int)$id_object : '')); foreach ($assos as $asso) - Db::getInstance()->execute('INSERT INTO '._DB_PREFIX_.$this->table.'_'.$type.' (`'.pSQL($this->identifier).'`, id_'.$type.') - VALUES('.($new_id_object ? $new_id_object : (int)$asso['id_object']).', '.(int)$asso['id_'.$type].')'); + Db::getInstance()->execute(' + INSERT INTO '._DB_PREFIX_.$this->table.'_'.$type.' (`'.pSQL($this->identifier).'`, id_'.$type.') + VALUES('.($new_id_object ? $new_id_object : (int)$asso['id_object']).', '.(int)$asso['id_'.$type].')'); } protected function validateField($value, $field) diff --git a/controllers/admin/AdminCarriersController.php b/controllers/admin/AdminCarriersController.php index 5e5ade8b2..2141f8f56 100644 --- a/controllers/admin/AdminCarriersController.php +++ b/controllers/admin/AdminCarriersController.php @@ -460,37 +460,35 @@ class AdminCarriersControllerCore extends AdminController { if ($this->tabAccess['edit'] === '1') { - $object = new $this->className($id); - if (Validate::isLoadedObject($object)) + $current_carrier = new Carrier($id); + if (!Validate::isLoadedObject($current_carrier)) + throw new PrestaShopException('Cannot load Carrier object'); + // Set flag deteled to true for historization + $current_carrier->deleted = true; + $current_carrier->update(); + + // Create new carrier + $new_carrier = new Carrier(); + // Fill the new carrier object + $this->copyFromPost($new_carrier, $this->table); + $new_carrier->position = $current_carrier->position; + if ($new_carrier->add()) { - Db::getInstance()->execute('DELETE FROM '._DB_PREFIX_.'carrier_group WHERE id_carrier = '.(int)$id); - $object->deleted = 1; - $object->update(); - $object_new = new $this->className(); - $this->copyFromPost($object_new, $this->table); - $object_new->position = $object->position; - $result = $object_new->add(); - $this->updateAssoShop($object->id, $object_new->id); - if (Validate::isLoadedObject($object_new)) - { - $this->afterDelete($object_new, $object->id); - Hook::exec('actionCarrierUpdate', array( - 'id_carrier' => (int)$object->id, - 'carrier' => $object_new, - )); - } - $this->changeGroups($object_new->id); - if (!$result) - $this->errors[] = Tools::displayError('An error occurred while updating object.').' '.$this->table.''; - else if ($this->postImage($object_new->id)) - { - $this->changeZones($object_new->id); - Tools::redirectAdmin(self::$currentIndex.'&id_'.$this->table.'='.$object->id.'&conf=4&token='.$this->token); - } + $this->updateAssoShop($current_carrier->id, $new_carrier->id); + $new_carrier->copyCarrierData((int)$current_carrier->id); + $this->changeGroups($new_carrier->id); + // Call of hooks + Hook::exec('actionCarrierUpdate', array( + 'id_carrier' => (int)$current_carrier->id, + 'carrier' => $new_carrier + )); + + $this->postImage($new_carrier->id); + $this->changeZones($new_carrier->id); + Tools::redirectAdmin(self::$currentIndex.'&id_'.$this->table.'='.$current_carrier->id.'&conf=4&token='.$this->token); } else - $this->errors[] = Tools::displayError('An error occurred while updating object.').' '. - $this->table.' '.Tools::displayError('(cannot load object)'); + $this->errors[] = Tools::displayError('An error occurred while updating object.').' '.$this->table.''; } else $this->errors[] = Tools::displayError('You do not have permission to edit here.'); @@ -501,18 +499,22 @@ class AdminCarriersControllerCore extends AdminController { if ($this->tabAccess['add'] === '1') { - $object = new $this->className(); - $this->copyFromPost($object, $this->table); - $object->position = Carrier::getHigherPosition() + 1; - if (!$object->add()) - $this->errors[] = Tools::displayError('An error occurred while creating object.').' '.$this->table.''; - else if (($_POST['id_'.$this->table] = $object->id /* voluntary */) && $this->postImage($object->id) && $this->_redirect) + // Create new Carrier + $carrier = new Carrier(); + $this->copyFromPost($carrier, $this->table); + $carrier->position = Carrier::getHigherPosition() + 1; + if ($carrier->add()) { - $this->changeZones($object->id); - $this->changeGroups($object->id); - $this->updateAssoShop($object->id); - Tools::redirectAdmin(self::$currentIndex.'&id_'.$this->table.'='.$object->id.'&conf=3&token='.$this->token); + if (($_POST['id_'.$this->table] = $carrier->id /* voluntary */) && $this->postImage($carrier->id) && $this->_redirect) + { + $this->changeZones($carrier->id); + $this->changeGroups($carrier->id); + $this->updateAssoShop($carrier->id); + Tools::redirectAdmin(self::$currentIndex.'&id_'.$this->table.'='.$carrier->id.'&conf=3&token='.$this->token); + } } + else + $this->errors[] = Tools::displayError('An error occurred while creating object.').' '.$this->table.''; } else $this->errors[] = Tools::displayError('You do not have permission to add here.'); @@ -594,11 +596,6 @@ class AdminCarriersControllerCore extends AdminController return $object->isUsed(); } - protected function afterDelete($object, $old_id) - { - $object->copyCarrierData((int)$old_id); - } - protected function changeGroups($id_carrier, $delete = true) { if ($delete) @@ -612,7 +609,6 @@ class AdminCarriersControllerCore extends AdminController '); } - public function changeZones($id) { $carrier = new $this->className($id);