// clean code && fix norm

classes/Validate.php

@@ -654,7 +654,7 @@ class ValidateCore
 	}
 
 	/**
-	 * Check url valdity (disallowed empty string)
+	 * Check url validity (disallowed empty string)
 	 *
 	 * @param string $url Url to validate
 	 * @return boolean Validity is ok or not
@@ -664,6 +664,17 @@ class ValidateCore
 		return preg_match('/^[~:#,%&_=\(\)\.\? \+\-@\/a-zA-Z0-9]+$/', $url);
 	}
 
+	/**
+	 * Check tracking number validity (disallowed empty string)
+	 *
+	 * @param string $tracking_number Tracking number to validate
+	 * @return boolean Validity is ok or not
+	 */
+	public static function isTrackingNumber($tracking_number)
+	{
+		return preg_match('/^[~:#,%&_=\(\)\.\? \+\-@\/a-zA-Z0-9]+$/', $tracking_number);
+	}
+
 	/**
 	 * Check url validity (allowed empty string)
 	 *

classes/order/Order.php

@@ -76,7 +76,11 @@ class OrderCore extends ObjectModel
 	/** @var string Gift message if specified */
 	public $gift_message;
 
-	/** @var string Shipping number */
+	/**
+	 * @var string Shipping number
+	 * @deprecated 1.5.0.4
+	 * @see OrderCarrier->tracking_number
+	 */
 	public $shipping_number;
 
 	/** @var float Discounts total */
@@ -189,7 +193,7 @@ class OrderCore extends ObjectModel
 			'total_wrapping' => 			array('type' => self::TYPE_FLOAT, 'validate' => 'isPrice'),
 			'total_wrapping_tax_incl' =>	array('type' => self::TYPE_FLOAT, 'validate' => 'isPrice'),
 			'total_wrapping_tax_excl' =>	array('type' => self::TYPE_FLOAT, 'validate' => 'isPrice'),
-			'shipping_number' => 			array('type' => self::TYPE_STRING, 'validate' => 'isUrl'),
+			'shipping_number' => 			array('type' => self::TYPE_STRING, 'validate' => 'isTrackingNumber'),
 			'conversion_rate' => 			array('type' => self::TYPE_FLOAT, 'validate' => 'isFloat', 'required' => true),
 			'invoice_number' => 			array('type' => self::TYPE_INT),
 			'delivery_number' => 			array('type' => self::TYPE_INT),
@@ -1505,7 +1509,7 @@ class OrderCore extends ObjectModel
 				if ($currency->id == Configuration::get('PS_DEFAULT_CURRENCY'))
 					$total += $amount;
 				else
-					$total += Tool::convertPrice($amount, $currency->id, true);
+					$total += Tools::convertPrice($amount, $currency->id, true);
 			}
 		}
 

classes/order/OrderCarrier.php

@@ -67,7 +67,7 @@ class OrderCarrierCore extends ObjectModel
 			'weight' => 				array('type' => self::TYPE_FLOAT, 'validate' => 'isFloat'),
 			'shipping_cost_tax_excl' => array('type' => self::TYPE_FLOAT, 'validate' => 'isFloat'),
 			'shipping_cost_tax_incl' => array('type' => self::TYPE_FLOAT, 'validate' => 'isFloat'),
-			'tracking_number' => 		array('type' => self::TYPE_STRING, 'validate' => 'isAnything'),
+			'tracking_number' => 		array('type' => self::TYPE_STRING, 'validate' => 'isTrackingNumber'),
 			'date_add' => 				array('type' => self::TYPE_DATE, 'validate' => 'isDate'),
 		),
 	);

classes/stock/StockMvt.php

@@ -185,7 +185,7 @@ class StockMvtCore extends ObjectModel
 	 */
 	public static function getNegativeStockMvts($id_order, $id_product, $id_product_attribute, $quantity, $id_warehouse = null)
 	{
-		$mvts = array();
+		$movements = array();
 		$quantity_total = 0;
 
 		$query = new DbQuery();
@@ -205,10 +205,10 @@ class StockMvtCore extends ObjectModel
 			if ($quantity_total >= $quantity)
 				break;
 			$quantity_total += (int)$row['physical_quantity'];
-			$mvts[] = $row;
+			$movements[] = $row;
 		}
 
-		return $mvts;
+		return $movements;
 	}
 
 	/**

controllers/admin/AdminLoginController.php

@@ -68,8 +68,6 @@ class AdminLoginControllerCore extends AdminController
 			}
 		}
 
-		
-		
 		if (file_exists(_PS_ADMIN_DIR_.'/../install') || file_exists(_PS_ADMIN_DIR_.'/../admin')
 		|| (file_exists(_PS_ADMIN_DIR_.'/../install-dev') && (!defined('_PS_MODE_DEV_') || !_PS_MODE_DEV_)))
 			$this->context->smarty->assign(

controllers/admin/AdminOrdersController.php

@@ -78,10 +78,15 @@ class AdminOrdersControllerCore extends AdminController
 		$this->shopLinkType = 'shop';
 		$this->shopShareDatas = Shop::SHARE_ORDER;
 
+		if (Tools::isSubmit('id_order'))
+		{
 			// Save context (in order to apply cart rule)
 			$order = new Order((int)Tools::getValue('id_order'));
+			if (!Validate::isLoadedObject($order))
+				throw new PrestaShopException('Cannot load Order object');
 			$this->context->cart = new Cart($order->id_cart);
 			$this->context->customer = new Customer($order->id_customer);
+		}
 
 		parent::__construct();
 	}
@@ -99,7 +104,7 @@ class AdminOrdersControllerCore extends AdminController
 			'currencies' => Currency::getCurrencies(),
 			'langs' => Language::getLanguages(true, Context::getContext()->shop->id),
 			'payment_modules' => PaymentModule::getInstalledPaymentModules(),
-			'order_states' => OrderState::getOrderStates((int)Context::getContext()->cookie->id_lang),
+			'order_states' => OrderState::getOrderStates((int)Context::getContext()->language->id),
 			'show_toolbar' => $this->show_toolbar,
 			'toolbar_btn' => $this->toolbar_btn,
 			'toolbar_fix' => $this->toolbar_fix,
@@ -189,7 +194,7 @@ class AdminOrdersControllerCore extends AdminController
 				$order_carrier = new OrderCarrier(Tools::getValue('id_order_carrier'));
 				if (!Validate::isLoadedObject($order_carrier))
 					$this->errors[] = Tools::displayError('Order carrier ID is invalid');
-				elseif (!Validate::isUrl(Tools::getValue('tracking_number')))
+				elseif (!Validate::isTrackingNumber(Tools::getValue('tracking_number')))
 					$this->errors[] = Tools::displayError('Tracking number is incorrect');
 				else
 				{
@@ -215,12 +220,16 @@ class AdminOrdersControllerCore extends AdminController
 							'{lastname}' => $customer->lastname,
 							'{id_order}' => $order->id
 						);
-						@Mail::Send((int)$order->id_lang, 'in_transit', Mail::l('Package in transit', (int)$order->id_lang), $templateVars,
+						if (@Mail::Send((int)$order->id_lang, 'in_transit', Mail::l('Package in transit', (int)$order->id_lang), $templateVars,
 							$customer->email, $customer->firstname.' '.$customer->lastname, null, null, null, null,
-							_PS_MAIL_DIR_, true);
+							_PS_MAIL_DIR_, true))
+						{
 							Hook::exec('actionAdminOrdersTrackingNumberUpdate', array('order' => $order));
 							Tools::redirectAdmin(self::$currentIndex.'&id_order='.$order->id.'&vieworder&conf=4&token='.$this->token);
 						}
+						else
+							$this->errors[] = Tools::displayError('An error occurred while sending e-mail to customer.');
+					}
 					else
 						$this->errors[] = Tools::displayError('Order carrier can\'t be updated');
 				}
@@ -290,7 +299,6 @@ class AdminOrdersControllerCore extends AdminController
 				else
 				{
 					/* Get message rules and and check fields validity */
-					// TODO review
 					$rules = call_user_func(array('Message', 'getValidationRules'), 'Message');
 					foreach ($rules['required'] as $field)
 						if (($value = Tools::getValue($field)) == false && (string)$value != '0')
@@ -331,7 +339,7 @@ class AdminOrdersControllerCore extends AdminController
 						$customer_message->private = Tools::getValue('visibility');
 
 						if (!$customer_message->add())
-							$this->errors[] = Tools::displayError('An error occurred while sending message.');
+							$this->errors[] = Tools::displayError('An error occurred while saving message.');
 						elseif ($customer_message->private)
 							Tools::redirectAdmin(self::$currentIndex.'&id_order='.(int)$order->id.'&vieworder&conf=11&token='.$this->token);
 						else
@@ -363,6 +371,8 @@ class AdminOrdersControllerCore extends AdminController
 		elseif (Tools::isSubmit('partialRefund') && isset($order))
 		{
 			if ($this->tabAccess['edit'] == '1')
+			{
+				if (is_array($_POST['partialRefundProduct']))
 				{
 					$amount = 0;
 					$order_detail_list = array();
@@ -389,6 +399,9 @@ class AdminOrdersControllerCore extends AdminController
 					if (!count($this->errors))
 						Tools::redirectAdmin(self::$currentIndex.'&id_order='.$order->id.'&vieworder&conf=24&token='.$this->token);
 				}
+				else
+					$this->errors[] = Tools::displayError('Partial refund data are incorrect');
+			}
 			else
 				$this->errors[] = Tools::displayError('You do not have permission to delete here.');
 		}
@@ -397,6 +410,12 @@ class AdminOrdersControllerCore extends AdminController
 		elseif (Tools::isSubmit('cancelProduct') && isset($order))
 		{
 		 	if ($this->tabAccess['delete'] === '1')
+			{
+				if (!Tools::isSubmit('id_order_detail'))
+					$this->errors[] = Tools::displayError('You must select a product');
+				elseif (!Tools::isSubmit('cancelQuantity'))
+					$this->errors[] = Tools::displayError('You must enter a quantity');
+				else
 				{
 					$productList = Tools::getValue('id_order_detail');
 					$customizationList = Tools::getValue('id_customization');
@@ -426,7 +445,6 @@ class AdminOrdersControllerCore extends AdminController
 								if (!$qtyCancelProduct)
 									$this->errors[] = Tools::displayError('No quantity selected for product.');
 
-							// check actionable quantity
 								$order_detail = new OrderDetail($id_order_detail);
 								$customization_quantity = 0;
 								if (array_key_exists($order_detail->product_id, $customization_quantities) && array_key_exists($order_detail->product_attribute_id, $customization_quantities[$order_detail->product_id]))
@@ -478,22 +496,22 @@ class AdminOrdersControllerCore extends AdminController
 
 										$manager = StockManagerFactory::getManager();
 
-									$mvts = StockMvt::getNegativeStockMvts(
+										$movements = StockMvt::getNegativeStockMvts(
 											$order_detail->id_order,
 											$order_detail->product_id,
 											$order_detail->product_attribute_id,
 											$quantity_to_reinject
 										);
 
-									foreach ($mvts as $mvt)
+										foreach ($movements as $movement)
 										{
 											$manager->addProduct(
 												$order_detail->product_id,
 												$order_detail->product_attribute_id,
-											new Warehouse($mvt['id_warehouse']),
+												new Warehouse($movement['id_warehouse']),
-											$mvt['physical_quantity'],
+												$movement['physical_quantity'],
 												null,
-											$mvt['price_te'],
+												$movement['price_te'],
 												true
 											);
 										}
@@ -527,7 +545,7 @@ class AdminOrdersControllerCore extends AdminController
 									$this->errors[] = Tools::displayError('An error occurred during deletion of product customization.').' '.$id_customization;
 							}
 						// E-mail params
-					if ((isset($_POST['generateCreditSlip']) || isset($_POST['generateDiscount'])) && !count($this->errors))
+						if ((Tools::isSubmit('generateCreditSlip') || Tools::isSubmit('generateDiscount')) && !count($this->errors))
 						{
 							$customer = new Customer((int)($order->id_customer));
 							$params['{lastname}'] = $customer->lastname;
@@ -536,9 +554,9 @@ class AdminOrdersControllerCore extends AdminController
 						}
 
 						// Generate credit slip
-					if (isset($_POST['generateCreditSlip']) && !count($this->errors))
+						if (Tools::isSubmit('generateCreditSlip') && !count($this->errors))
 						{
-						if (!OrderSlip::createOrderSlip($order, $full_product_list, $full_quantity_list, isset($_POST['shippingBack'])))
+							if (!OrderSlip::createOrderSlip($order, $full_product_list, $full_quantity_list, Tools::isSubmit('shippingBack')))
 								$this->errors[] = Tools::displayError('Cannot generate credit slip');
 							else
 							{
@@ -561,10 +579,10 @@ class AdminOrdersControllerCore extends AdminController
 						}
 
 						// Generate voucher
-					if (isset($_POST['generateDiscount']) && !count($this->errors))
+						if (Tools::isSubmit('generateDiscount') && !count($this->errors))
 						{
 							// @todo generate a voucher using cartrules
-						if (true || !$voucher = Discount::createOrderDiscount($order, $full_product_list, $full_quantity_list, $this->l('Credit Slip concerning the order #'), isset($_POST['shippingBack'])))
+							if (true || !$voucher = Discount::createOrderDiscount($order, $full_product_list, $full_quantity_list, $this->l('Credit Slip concerning the order #'), Tools::isSubmit('shippingBack')))
 								$this->errors[] = Tools::displayError('Cannot generate voucher');
 							else
 							{
@@ -584,21 +602,24 @@ class AdminOrdersControllerCore extends AdminController
 					if (!count($this->errors))
 						Tools::redirectAdmin(self::$currentIndex.'&id_order='.$order->id.'&vieworder&conf=24&token='.$this->token);
 				}
+			}
 			else
 				$this->errors[] = Tools::displayError('You do not have permission to delete here.');
 		}
 		elseif (Tools::isSubmit('messageReaded'))
-			Message::markAsReaded($_GET['messageReaded'], $this->context->employee->id);
+			Message::markAsReaded(Tools::getValue('messageReaded'), $this->context->employee->id);
 		elseif (Tools::isSubmit('submitAddPayment') && isset($order))
 		{
 			if ($this->tabAccess['edit'] === '1')
 			{
 				$amount = str_replace(',', '.', Tools::getValue('payment_amount'));
 				$currency = new Currency(Tools::getValue('payment_currency'));
-				if ($order->hasInvoice())
+				$order_has_invoice = $order->hasInvoice();
+				if ($order_has_invoice)
 					$order_invoice = new OrderInvoice(Tools::getValue('payment_invoice'));
 				else
 					$order_invoice = null;
+
 				if (!Validate::isLoadedObject($order))
 					$this->errors[] = Tools::displayError('Order can\'t be found');
 				elseif (!Validate::isPrice($amount))
@@ -609,14 +630,14 @@ class AdminOrdersControllerCore extends AdminController
 					$this->errors[] = Tools::displayError('Transaction ID is invalid');
 				elseif (!Validate::isLoadedObject($currency))
 					$this->errors[] = Tools::displayError('Currency is invalid');
-				elseif ($order->hasInvoice() && !Validate::isLoadedObject($order_invoice))
+				elseif ($order_has_invoice && !Validate::isLoadedObject($order_invoice))
 					$this->errors[] = Tools::displayError('Invoice is invalid');
 				elseif (!Validate::isDate(Tools::getValue('payment_date')))
 					$this->errors[] = Tools::displayError('Date is invalid');
 				else
 				{
 					if (!$order->addOrderPayment($amount, Tools::getValue('payment_method'), Tools::getValue('payment_transaction_id'), $currency, Tools::getValue('payment_date'), $order_invoice))
-						$this->errors[] = Tools::displayError('An error occured on adding of order payment');
+						$this->errors[] = Tools::displayError('An error occurred on adding of order payment');
 					else
 						Tools::redirectAdmin(self::$currentIndex.'&id_order='.$order->id.'&vieworder&conf=4&token='.$this->token);
 				}
@@ -626,9 +647,8 @@ class AdminOrdersControllerCore extends AdminController
 		}
 		elseif (Tools::isSubmit('submitEditNote'))
 		{
-			$id_order_invoice = (int)Tools::getValue('id_order_invoice');
 			$note = Tools::getValue('note');
-			$order_invoice = new OrderInvoice($id_order_invoice);
+			$order_invoice = new OrderInvoice((int)Tools::getValue('id_order_invoice'));
 			if (Validate::isLoadedObject($order_invoice) && Validate::isCleanHtml($note))
 			{
 				if ($this->tabAccess['edit'] === '1')
@@ -645,7 +665,9 @@ class AdminOrdersControllerCore extends AdminController
 			else
 				$this->errors[] = Tools::displayError('Unable to load invoice for edit note.');
 		}
-		elseif (Tools::isSubmit('submitAddOrder') == 1 && ($id_cart = Tools::getValue('id_cart')) && ($module_name = pSQL(Tools::getValue('payment_module_name'))) && ($id_order_state = Tools::getValue('id_order_state')))
+		elseif (Tools::isSubmit('submitAddOrder') && ($id_cart = Tools::getValue('id_cart')) &&
+			($module_name = Tools::getValue('payment_module_name')) &&
+			($id_order_state = Tools::getValue('id_order_state')))
 		{
 			if ($this->tabAccess['edit'] === '1')
 			{
@@ -706,21 +728,13 @@ class AdminOrdersControllerCore extends AdminController
 						$order_detail->update();
 					}
 
-					// Update order payment amount
-					foreach ($order->getOrderPaymentCollection() as $payment)
-					{
-						$payment->id_currency = (int)$currency->id;
-						$payment->amount = Tools::convertPriceFull((float)$payment->amount, $old_currency, $currency);
-						$payment->update();
-					}
-
 					$id_order_carrier = Db::getInstance()->getValue('
 						SELECT `id_order_carrier`
 						FROM `'._DB_PREFIX_.'order_carrier`
 						WHERE `id_order` = '.(int)$order->id);
 					$order_carrier = new OrderCarrier($id_order_carrier);
-					$order_carrier->shipping_cost_tax_excl = (float)Tools::convertPriceFull($order_carrier['shipping_cost_tax_excl'], $old_currency, $currency);
+					$order_carrier->shipping_cost_tax_excl = (float)Tools::convertPriceFull($order_carrier->shipping_cost_tax_excl, $old_currency, $currency);
-					$order_carrier->shipping_cost_tax_incl = (float)Tools::convertPriceFull($order_carrier['shipping_cost_tax_incl'], $old_currency, $currency);
+					$order_carrier->shipping_cost_tax_incl = (float)Tools::convertPriceFull($order_carrier->shipping_cost_tax_incl, $old_currency, $currency);
 					$order_carrier->update();
 
 					// Update order amount