// Code Review Invoices V&V

admin-dev/pdf.php

@@ -32,7 +32,7 @@ include(_PS_ADMIN_DIR_.'/../config/config.inc.php');
 if (!Context::getContext()->employee->id)
 	Tools::redirectAdmin('index.php?controller=AdminLogin');
 
-$functionArray = array(
+$function_array = array(
 	'pdf' => 'generateInvoicePDF',
 	'id_order_slip' => 'generateOrderSlipPDF',
 	'id_delivery' => 'generateDeliverySlipPDF',
@@ -44,7 +44,7 @@ $functionArray = array(
 	'id_supply_order' => 'generateSupplyOrderFormPDF'
 );
 
-foreach ($functionArray as $var => $function)
+foreach ($function_array as $var => $function)
 	if (isset($_GET[$var]))
 	{
 		call_user_func($function);
@@ -53,10 +53,10 @@ foreach ($functionArray as $var => $function)
 
 function generateSupplyOrderFormPDF()
 {
-	if (!isset($_GET['id_supply_order']))
+	if (!Tools::isSubmit('id_supply_order'))
 		die (Tools::displayError('Missing supply order ID'));
 
-	$id_supply_order = (int)$_GET['id_supply_order'];
+	$id_supply_order = (int)Tools::getValue('id_supply_order');
 	$supply_order = new SupplyOrder($id_supply_order);
 
 	if (!Validate::isLoadedObject($supply_order))
@@ -78,7 +78,7 @@ function generateInvoicePDF()
 
 function generateInvoicePDFByIdOrder($id_order)
 {
-	$order = new Order($id_order);
+	$order = new Order((int)$id_order);
 	if (!Validate::isLoadedObject($order))
 		die(Tools::displayError('Cannot find order in database'));
 
@@ -89,7 +89,7 @@ function generateInvoicePDFByIdOrder($id_order)
 
 function generateInvoicePDFByIdOrderInvoice($id_order_invoice)
 {
-	$order_invoice = new OrderInvoice($id_order_invoice);
+	$order_invoice = new OrderInvoice((int)$id_order_invoice);
 	if (!Validate::isLoadedObject($order_invoice))
 		die(Tools::displayError('Cannot find order invoice in database'));
 
@@ -99,12 +99,11 @@ function generateInvoicePDFByIdOrderInvoice($id_order_invoice)
 
 function generateOrderSlipPDF()
 {
-	$orderSlip = new OrderSlip((int)($_GET['id_order_slip']));
+	$orderSlip = new OrderSlip((int)Tools::getValue('id_order_slip'));
-	$order = new Order((int)($orderSlip->id_order));
+	$order = new Order((int)$orderSlip->id_order);
 	if (!Validate::isLoadedObject($order))
 		die(Tools::displayError('Cannot find order in database'));
 	$order->products = OrderSlip::getOrdersSlipProducts($orderSlip->id, $order);
-	$tmp = NULL;
 
 	generatePDF($orderSlip, PDF::TEMPLATE_ORDER_SLIP);
 }
@@ -112,13 +111,13 @@ function generateOrderSlipPDF()
 function generateDeliverySlipPDF()
 {
 	if (Tools::isSubmit('id_order'))
-		generateDeliverySlipPDFByIdOrder(Tools::getValue('id_order'));
+		generateDeliverySlipPDFByIdOrder((int)Tools::getValue('id_order'));
 	elseif (Tools::isSubmit('id_order_invoice'))
-		generateDeliverySlipPDFByIdOrderInvoice(Tools::getValue('id_order_invoice'));
+		generateDeliverySlipPDFByIdOrderInvoice((int)Tools::getValue('id_order_invoice'));
 	elseif (Tools::isSubmit('id_delivery'))
 	{
-		$order = Order::getByDelivery(Tools::getValue('id_delivery'));
+		$order = Order::getByDelivery((int)Tools::getValue('id_delivery'));
-		generateDeliverySlipPDFByIdOrder($order->id);
+		generateDeliverySlipPDFByIdOrder((int)$order->id);
 	}
 	else
 		die (Tools::displayError('Missing order ID or invoice order ID'));
@@ -127,7 +126,7 @@ function generateDeliverySlipPDF()
 
 function generateDeliverySlipPDFByIdOrder($id_order)
 {
-	$order = new Order($id_order);
+	$order = new Order((int)$id_order);
 	if (!Validate::isLoadedObject($order))
 		throw new PrestaShopException('Can\'t load Order object');
 
@@ -137,7 +136,7 @@ function generateDeliverySlipPDFByIdOrder($id_order)
 
 function generateDeliverySlipPDFByIdOrderInvoice($id_order_invoice)
 {
-	$order_invoice = new OrderInvoice($id_order_invoice);
+	$order_invoice = new OrderInvoice((int)$id_order_invoice);
 	if (!Validate::isLoadedObject($order_invoice))
 		throw new PrestaShopException('Can\'t load Order Invoice object');
 
@@ -146,9 +145,9 @@ function generateDeliverySlipPDFByIdOrderInvoice($id_order_invoice)
 
 function generateInvoicesPDF()
 {
-	$order_invoice_collection = OrderInvoice::getByDateInterval($_GET['date_from'], $_GET['date_to']);
+	$order_invoice_collection = OrderInvoice::getByDateInterval(Tools::getValue('date_from'), Tools::getValue('date_to'));
 
-	if (!sizeof($order_invoice_collection))
+	if (!count($order_invoice_collection))
 		die(Tools::displayError('No invoices found'));
 
 	generatePDF($order_invoice_collection, PDF::TEMPLATE_INVOICE);
@@ -158,10 +157,10 @@ function generateInvoicesPDF2()
 {
 	$order_invoice_collection = array();
 	foreach (explode('-', Tools::getValue('id_order_state')) as $id_order_state)
-		if (is_array($order_invoices = OrderInvoice::getByStatus($id_order_state)))
+		if (is_array($order_invoices = OrderInvoice::getByStatus((int)$id_order_state)))
 			$order_invoice_collection = array_merge($order_invoices, $order_invoice_collection);
 
-	if (!sizeof($order_invoice_collection))
+	if (!count($order_invoice_collection))
 		die(Tools::displayError('No invoices found'));
 
 	generatePDF($order_invoice_collection, PDF::TEMPLATE_INVOICE);
@@ -169,7 +168,7 @@ function generateInvoicesPDF2()
 
 function generateOrderSlipsPDF()
 {
-	$id_order_slips_list = OrderSlip::getSlipsIdByDate($_GET['date_from'], $_GET['date_to']);
+	$id_order_slips_list = OrderSlip::getSlipsIdByDate(Tools::getValue('date_from'), Tools::getValue('date_to'));
 	if (!count($id_order_slips_list))
 		die (Tools::displayError('No order slips found'));
 
@@ -184,7 +183,7 @@ function generateDeliverySlipsPDF()
 {
 	$order_invoice_collection = OrderInvoice::getByDeliveryDateInterval(Tools::getValue('date_from'), Tools::getValue('date_to'));
 
-	if (!sizeof($order_invoice_collection))
+	if (!count($order_invoice_collection))
 		die(Tools::displayError('No invoices found'));
 
 	generatePDF($order_invoice_collection, PDF::TEMPLATE_DELIVERY_SLIP);
@@ -192,7 +191,6 @@ function generateDeliverySlipsPDF()
 
 function generatePDF($object, $template)
 {
-	global $smarty;
+	$pdf = new PDF($object, $template, Context::getContext()->smarty);
-	$pdf = new PDF($object, $template, $smarty);
 	$pdf->render();
 }

classes/pdf/HTMLTemplate.php

@@ -75,6 +75,7 @@ abstract class HTMLTemplateCore
 			'shop_details' => Configuration::get('PS_SHOP_DETAILS'),
 			'free_text' => Configuration::get('PS_INVOICE_FREE_TEXT')
 		));
+
 		return $this->smarty->fetch(_PS_THEME_DIR_.'/pdf/footer.tpl');
 	}
 
@@ -122,7 +123,7 @@ abstract class HTMLTemplateCore
 		$hook_name = 'displayPDF'.$template;
 
 		$this->smarty->assign(array(
-			'HOOK_DISPLAY_PDF' => Hook::exec($hook_name, array('object' => $object)),
+			'HOOK_DISPLAY_PDF' => Hook::exec($hook_name, array('object' => $object))
 		));
 	}
 
@@ -145,9 +146,6 @@ abstract class HTMLTemplateCore
 	 */
 	abstract public function getBulkFilename();
 
-
-
-
 	/**
 	 * Translatation method
 	 * @param string $string
@@ -158,10 +156,11 @@ abstract class HTMLTemplateCore
 		$iso = Context::getContext()->language->iso_code;
 
 		if (!Validate::isLangIsoCode($iso))
-            die('Invalid iso lang ('.$iso.')');
+			Tools::displayError(sprintf('Invalid iso lang (%s)', Tools::safeOutput($iso)));
 
-		if (@!include(_PS_THEME_DIR_.'pdf/lang/'.$iso.'.php'))
+		$file_name = _PS_THEME_DIR_.'pdf/lang/'.$iso.'.php';
-			die('Cannot include PDF translation language file : '._PS_THEME_DIR_.'pdf/lang/'.$iso.'.php');
+		if (!file_exists($file_name) || !include($file_name))
+			Tools::displayError(sprintf('Cannot include PDF translation language file : %s', $file_name));
 
 		if (!isset($_LANGPDF) || !is_array($_LANGPDF))
 			return str_replace('"', '"', $string);

classes/pdf/HTMLTemplateDeliverySlip.php

@@ -52,20 +52,16 @@ class HTMLTemplateDeliverySlipCore extends HTMLTemplate
 	 */
 	public function getContent()
 	{
-		$country = new Country((int)$this->order->id_address_invoice);
-
 		$delivery_address = new Address((int)$this->order->id_address_delivery);
 		$formatted_delivery_address = AddressFormat::generateAddress($delivery_address, array(), '<br />', ' ');
 		$formatted_invoice_address = '';
 
 		if ($this->order->id_address_delivery != $this->order->id_address_invoice)
 		{
-			$invoice_address = new Address((int)$id_address_invoice);
+			$invoice_address = new Address((int)$this->order->id_address_invoice);
 			$formatted_invoice_address = AddressFormat::generateAddress($invoice_address, array(), '<br />', ' ');
 		}
 
-		$customer = new Customer($this->order->id_customer);
-
 		$this->smarty->assign(array(
 			'order' => $this->order,
 			'order_details' => $this->order_invoice->getProducts(),

classes/pdf/HTMLTemplateInvoice.php

@@ -31,19 +31,19 @@
 class HTMLTemplateInvoiceCore extends HTMLTemplate
 {
 	public $order;
-
 	public $available_in_your_account = false;
 
 	public function __construct(OrderInvoice $order_invoice, $smarty)
 	{
 		$this->order_invoice = $order_invoice;
-        $this->order = new Order($this->order_invoice->id_order);
+		$this->order = new Order((int)$this->order_invoice->id_order);
 		$this->smarty = $smarty;
 
 		// header informations
 		$this->date = Tools::displayDate($order_invoice->date_add, (int)$this->order->id_lang);
-		$this->title = HTMLTemplateInvoice::l('Invoice ').' #'.Configuration::get('PS_INVOICE_PREFIX', Context::getContext()->language->id).sprintf('%06d', $order_invoice->number);
 
+		$id_lang = Context::getContext()->language->id;
+		$this->title = HTMLTemplateInvoice::l('Invoice ').' #'.Configuration::get('PS_INVOICE_PREFIX', $id_lang).sprintf('%06d', $order_invoice->number);
 		// footer informations
 		$this->shop = new Shop((int)$this->order->id_shop);
 	}
@@ -65,7 +65,7 @@ class HTMLTemplateInvoiceCore extends HTMLTemplate
 			$formatted_delivery_address = AddressFormat::generateAddress($delivery_address, array(), '<br />', ' ');
 		}
 
-		$customer = new Customer($this->order->id_customer);
+		$customer = new Customer((int)$this->order->id_customer);
 
 		$this->smarty->assign(array(
 			'order' => $this->order,
@@ -87,8 +87,8 @@ class HTMLTemplateInvoiceCore extends HTMLTemplate
 	{
 			$invoice_address = new Address((int)$this->order->id_address_invoice);
 			$tax_exempt = Configuration::get('VATNUMBER_MANAGEMENT')
-								&& !empty($invoiceAddress->vat_number)
+								&& !empty($invoice_address->vat_number)
-								&& $invoiceAddress->id_country != Configuration::get('VATNUMBER_COUNTRY');
+								&& $invoice_address->id_country != Configuration::get('VATNUMBER_COUNTRY');
 
 			$this->smarty->assign(array(
 				'tax_exempt' => $tax_exempt,

classes/pdf/HTMLTemplateOrderReturn.php

@@ -41,7 +41,7 @@ class HTMLTemplateOrderReturnCore extends HTMLTemplate
 
 		// header informations
 		$this->date = Tools::displayDate($this->order->invoice_date, (int)$this->order->id_lang);
-		$this->title = HTMLTemplateOrderReturn::l('Order Return '.sprintf('%06d', $this->order_return->id)); // TODO
+		$this->title = HTMLTemplateOrderReturn::l('Order Return '.sprintf('%06d', $this->order_return->id));
 
 		// footer informations
 		$this->shop = new Shop((int)$this->order->id_shop);
@@ -59,14 +59,14 @@ class HTMLTemplateOrderReturnCore extends HTMLTemplate
 
 		if ($this->order->id_address_delivery != $this->order->id_address_invoice)
 		{
-			$invoice_address = new Address((int)$id_address_invoice);
+			$invoice_address = new Address((int)$this->order->id_address_invoice);
 			$formatted_invoice_address = AddressFormat::generateAddress($invoice_address, array(), '<br />', ' ');
 		}
 
 		$this->smarty->assign(array(
 			'order_return' => $this->order_return,
 			'return_nb_days' => (int)Configuration::get('PS_ORDER_RETURN_NB_DAYS'),
-			'products' => OrderReturn::getOrdersReturnProducts($this->order_return->id, $this->order),
+			'products' => OrderReturn::getOrdersReturnProducts((int)$this->order_return->id, $this->order),
 			'delivery_address' => $formatted_delivery_address,
 			'invoice_address' => $formatted_invoice_address,
 			'shop_address' => AddressFormat::generateAddress($this->address, array(), '<br />', ' ')
@@ -80,7 +80,7 @@ class HTMLTemplateOrderReturnCore extends HTMLTemplate
 	 */
 	public function getFilename()
 	{
-        return sprintf('%06d', $this->order_return->id).'.pdf'; // TODO
+		return sprintf('%06d', $this->order_return->id).'.pdf';
 	}
 
 	/**
@@ -92,4 +92,3 @@ class HTMLTemplateOrderReturnCore extends HTMLTemplate
 		return 'invoices.pdf';
 	}
 }
-

classes/pdf/HTMLTemplateOrderSlip.php

@@ -39,15 +39,15 @@ class HTMLTemplateOrderSlipCore extends HTMLTemplateInvoice
 		$this->order = new Order((int)$order_slip->id_order);
 
 		$products = OrderSlip::getOrdersSlipProducts($this->order_slip->id, $this->order);
-		$customizedDatas = Product::getAllCustomizedDatas((int)($this->order->id_cart));
+		$customized_datas = Product::getAllCustomizedDatas((int)$this->order->id_cart);
-		Product::addCustomizationPrice($products, $customizedDatas);
+		Product::addCustomizationPrice($products, $customized_datas);
 
 		$this->order->products = $products;
 		$this->smarty = $smarty;
 
 		// header informations
 		$this->date = Tools::displayDate($this->order->invoice_date, (int)$this->order->id_lang);
-		$this->title = HTMLTemplateOrderSlip::l('Slip #').sprintf('%06d', $this->order_slip->id);
+		$this->title = HTMLTemplateOrderSlip::l('Slip #').sprintf('%06d', (int)$this->order_slip->id);
 
 		// footer informations
 		$this->shop = new Shop((int)$this->order->id_shop);
@@ -59,7 +59,6 @@ class HTMLTemplateOrderSlipCore extends HTMLTemplateInvoice
 	 */
 	public function getContent()
 	{
-		$country = new Country((int)$this->order->id_address_invoice);
 		$invoice_address = new Address((int)$this->order->id_address_invoice);
 		$formatted_invoice_address = AddressFormat::generateAddress($invoice_address, array(), '<br />', ' ');
 		$formatted_delivery_address = '';
@@ -70,36 +69,34 @@ class HTMLTemplateOrderSlipCore extends HTMLTemplateInvoice
 			$formatted_delivery_address = AddressFormat::generateAddress($delivery_address, array(), '<br />', ' ');
 		}
 
-		$customer = new Customer($this->order->id_customer);
+		$customer = new Customer((int)$this->order->id_customer);
 
-
+		$this->order->total_products = $this->order->total_products_wt = 0;
-		$this->order->total_products = 0;
+		foreach ($this->order->products as &$product)
-		$this->order->total_products_wt = 0;
-		foreach ($this->order->products as $k => $p)
 		{
-			$this->order->products[$k]['total_price_tax_excl'] = $this->order->products[$k]['unit_price_tax_excl'] * $this->order->products[$k]['product_quantity'];
+			$product['total_price_tax_excl'] = $product['unit_price_tax_excl'] * $product['product_quantity'];
-			$this->order->products[$k]['total_price_tax_incl'] = $this->order->products[$k]['unit_price_tax_incl'] * $this->order->products[$k]['product_quantity'];
+			$product['total_price_tax_incl'] = $product['unit_price_tax_incl'] * $product['product_quantity'];
 			if ($this->order_slip->partial == 1)
 			{
-				$order_slip_detail = Db::getInstance()->getRow('SELECT * FROM `'._DB_PREFIX_.'order_slip_detail` WHERE `id_order_slip` = '.(int)$this->order_slip->id.' AND `id_order_detail` = '.(int)$p['id_order_detail']);
+				$order_slip_detail = Db::getInstance()->getRow('
-				$this->order->products[$k]['total_price_tax_excl'] = $order_slip_detail['amount_tax_excl'];
+					SELECT * FROM `'._DB_PREFIX_.'order_slip_detail`
-				$this->order->products[$k]['total_price_tax_incl'] = $order_slip_detail['amount_tax_incl'];
+					WHERE `id_order_slip` = '.(int)$this->order_slip->id.'
+					AND `id_order_detail` = '.(int)$product['id_order_detail']);
+				$product['total_price_tax_excl'] = $order_slip_detail['amount_tax_excl'];
+				$product['total_price_tax_incl'] = $order_slip_detail['amount_tax_incl'];
 			}
-			$this->order->total_products += $this->order->products[$k]['total_price_tax_excl'];
+			$this->order->total_products += $product['total_price_tax_excl'];
-			$this->order->total_products_wt += $this->order->products[$k]['total_price_tax_incl'];
+			$this->order->total_products_wt += $product['total_price_tax_incl'];
 			$this->order->total_paid_tax_excl = $this->order->total_products;
 			$this->order->total_paid_tax_incl = $this->order->total_products_wt;
 		}
+		unset($product); // remove reference
 		if ($this->order_slip->shipping_cost == 0)
-		{
+			$this->order->total_shipping_tax_incl = $this->order->total_shipping_tax_excl = 0;
-			$this->order->total_shipping_tax_incl = 0;
+
-			$this->order->total_shipping_tax_excl = 0;
-		}
 		if ($this->order_slip->partial == 1 && $this->order_slip->shipping_cost_amount > 0)
-		{
+			$this->order->total_shipping_tax_incl = $this->order->total_shipping_tax_excl = $this->order_slip->shipping_cost_amount;
-			$this->order->total_shipping_tax_incl = $this->order_slip->shipping_cost_amount;
+
-			$this->order->total_shipping_tax_excl = $this->order_slip->shipping_cost_amount;
-		}
 		$this->order->total_paid_tax_incl += $this->order->total_shipping_tax_incl;
 		$this->order->total_paid_tax_excl += $this->order->total_shipping_tax_excl;
 
@@ -109,7 +106,7 @@ class HTMLTemplateOrderSlipCore extends HTMLTemplateInvoice
 			'order_details' => $this->order->products,
 			'delivery_address' => $formatted_delivery_address,
 			'invoice_address' => $formatted_invoice_address,
-			'tax_excluded_display' => Group::getPriceDisplayMethod($customer->id_default_group),
+			'tax_excluded_display' => Group::getPriceDisplayMethod((int)$customer->id_default_group),
 			'tax_tab' => '',
 		));
 
@@ -134,4 +131,3 @@ class HTMLTemplateOrderSlipCore extends HTMLTemplateInvoice
 		return 'order-slip-'.sprintf('%06d', $this->order_slip->id).'.pdf';
 	}
 }
-

classes/pdf/HTMLTemplateSupplyOrderForm.php

@@ -41,9 +41,9 @@ class HTMLTemplateSupplyOrderFormCore extends HTMLTemplate
 		$this->supply_order = $supply_order;
 		$this->smarty = $smarty;
 		$this->context = Context::getContext();
-        $this->warehouse = new Warehouse($supply_order->id_warehouse);
+		$this->warehouse = new Warehouse((int)$supply_order->id_warehouse);
-        $this->address_warehouse = new Address($this->warehouse->id_address);
+		$this->address_warehouse = new Address((int)$this->warehouse->id_address);
-        $this->address_supplier = new Address(Address::getAddressIdBySupplierId($supply_order->id_supplier));
+		$this->address_supplier = new Address(Address::getAddressIdBySupplierId((int)$supply_order->id_supplier));
 
 		// header informations
 		$this->date = Tools::displayDate($supply_order->date_add, (int)$this->supply_order->id_lang);
@@ -55,13 +55,13 @@ class HTMLTemplateSupplyOrderFormCore extends HTMLTemplate
 	 */
 	public function getContent()
 	{
-		$supply_order_details = $this->supply_order->getEntriesCollection($this->supply_order->id_lang);
+		$supply_order_details = $this->supply_order->getEntriesCollection((int)$this->supply_order->id_lang);
 		$this->roundSupplyOrderDetails($supply_order_details);
 
 		$this->roundSupplyOrder($this->supply_order);
 
 		$tax_order_summary = $this->getTaxOrderSummary();
-		$currency = new Currency($this->supply_order->id_currency);
+		$currency = new Currency((int)$this->supply_order->id_currency);
 
 		$this->smarty->assign(array(
 			'warehouse' => $this->warehouse,
@@ -88,7 +88,7 @@ class HTMLTemplateSupplyOrderFormCore extends HTMLTemplate
 	 */
 	public function getFilename()
 	{
-    	return (self::l('SupplyOrderForm').sprintf('_%s', $this->supply_order->reference).'.pdf');
+		return self::l('SupplyOrderForm').sprintf('_%s', $this->supply_order->reference).'.pdf';
 	}
 
 	protected function getTaxOrderSummary()
@@ -111,6 +111,7 @@ class HTMLTemplateSupplyOrderFormCore extends HTMLTemplate
 			$result['tax_rate'] = Tools::ps_round($result['tax_rate'], 2);
 			$result['total_tax_value'] = Tools::ps_round($result['total_tax_value'], 2);
 		}
+		unset($result); // remove reference
 
 		return $results;
 	}

classes/pdf/PDF.php

@@ -99,4 +99,3 @@ class PDFCore
 		return $class;
 	}
 }
-

classes/pdf/PDFGenerator.php

@@ -94,7 +94,7 @@ class PDFGeneratorCore extends TCPDF
 	 */
 	public function setFontForLang($iso_lang)
 	{
-		$this->font = self::DEFAULT_FONT;
+		$this->font = PDFGenerator::DEFAULT_FONT;
 		if (array_key_exists($iso_lang, $this->font_by_lang))
 			$this->font = $this->font_by_lang[$iso_lang];
 
@@ -106,7 +106,7 @@ class PDFGeneratorCore extends TCPDF
 	 */
 	public function Header()
 	{
-		$this->writehtml($this->header);
+		$this->writeHTML($this->header);
 	}
 
 	/**
@@ -114,7 +114,7 @@ class PDFGeneratorCore extends TCPDF
 	 */
 	public function Footer()
 	{
-		$this->writehtml($this->footer);
+		$this->writeHTML($this->footer);
 	}
 
 	/**
@@ -147,7 +147,6 @@ class PDFGeneratorCore extends TCPDF
 
 		$this->AddPage();
 
-		$this->writehtml($this->content, true, false, true, false, '');
+		$this->writeHTML($this->content, true, false, true, false, '');
 	}
 }
-