Basically I am in argument with my colleague, I say this template (view) has to be fully refactored and all instances of duplicate code has to be removed. He disagrees and don't want me to touch it. Can you please do a code review as 3rd party opinion? This is magento phtml template file for checkout review step.
<?php echo $this->getMessagesBlock()->getGroupedHtml() ?>
<span class="gift-message-hover-text">
<?php echo $this->getCmsBlock( 'securecheckout_gift_message_hover_text' ); ?>
</span>
<div style="clear:both;"></div>
<form action="<?php echo $this->getUrl( 'securecheckout/user/submitreview' ); ?>" method="POST" id="checkout_review">
<?php
//Init.
$oCustomerSession = $this->getCustomerSession();
// @TODO cleanup and finalize.
$aShippingData = $oCustomerSession[ 'shipping' ];
$oQuote = $this->getQuote();
$sCheckoutButtonHtml = '';
// Real quote stored data ready to become an order
$oShipping = $oQuote->getShippingAddress();
$oBilling = $oQuote->getBillingAddress();
$oShippingMethod = $oShipping->getShippingMethod();
$oPayment = $oQuote->getPayment();
// v( $oPayment );
$sPromo = $this->helper( 'checkout_securecheckout/promo' )->getPromo();
$sGiftCard = $this->helper( 'checkout_securecheckout/giftcard' )->getGiftCard();
// @IMPORTANT quote may not have all the address info.
// Either ensure quote is properly set or use customer address ids.
$iShippingAddressId = $oShipping->getData( 'customer_address_id' );
$iBillingAddressId = $oBilling->getData( 'customer_address_id' );
$oCustomerShipping = Mage::getModel( 'customer/address' )->load( $iShippingAddressId );
$oCustomerBilling = Mage::getModel( 'customer/address' )->load( $iBillingAddressId );
// v( $oShipping );
// v( $oBilling );
// v( $oCustomerShipping );
// v( $oCustomerBilling );
// v( $oShippingMethod );
// v( $oPayment );
// @TODO ensure checkout is allowed only once all of our data checks complete.
// @TODO handle virtual items/tbooks/gift cards (not yet but soon), donations?
if( !empty( $oShipping ) && !empty( $oBilling ) && !empty( $oShippingMethod ) && !empty( $oPayment ) )
{
$sCheckoutButtonHtml .= '';
$sCheckoutButtonHtml .= '<input type="checkbox" name="review[agree]" id="agree" required /><label for="agree" class="unselectable"> I Agree to the terms</label> ';
$sCheckoutButtonHtml .= '<span>';
$sCheckoutButtonHtml .= '<a href="//www.checkoutimes.com/content/help/rights/terms/terms-of-service.html" target="_blank">Terms of Service</a> ';
$sCheckoutButtonHtml .= '<a href="//www.checkoutimes.com/content/help/rights/privacy/policy/privacy-policy.html" target="_blank">Privacy Policy</a> ';
$sCheckoutButtonHtml .= '<a href="';
$sCheckoutButtonHtml .= $this->getUrl( 'store-policies' );
$sCheckoutButtonHtml .= '" target="_blank">Store Policies</a>';
$sCheckoutButtonHtml .= '<br />';
$sCheckoutButtonHtml .= '<button type="submit" name="continue" value="1">PLACE ORDER</button>';
$sCheckoutButtonHtml .= '</span>';
}
$sPromo = $this->helper( 'checkout_securecheckout/promo' )->getPromo();
$sEmail = ( ( $oShipping->getData( 'email' ) ) ? $oShipping->getData( 'email' ) : '' );
$sGiftCard = $this->helper( 'checkout_securecheckout/giftcard' )->getGiftCard();
?>
<div style="clear:both;"></div>
<div class="review-cart">
<?php
$oQuote = $this->getQuote();
$oCartItems = $oQuote->getAllItems();
$oOptionHelper = Mage::helper('catalog/product_configuration');
// @TODO avoid html tables when outputting product info.
foreach( $oCartItems as $oItem ):
$oProduct = $oItem->getProduct();
?>
<span>
<a class="product-image" href="<?php echo $oProduct->getProductUrl(); ?>" title="<?php echo $this->escapeHtml($oProduct->getName()); ?>">
<img src="<?php echo $this->helper('catalog/image')->init($oProduct, 'small_image')->resize(113, 113); ?>" width="113" height="113" alt="<?php echo $this->escapeHtml($oProduct->getName()); ?>" />
</a>
</span>
<span>
<h5 class="product-name"><a href="<?php echo $oProduct->getProductUrl(); ?>" title="<?php echo $this->escapeHtml($oProduct->getName()); ?>"><?php echo $this->escapeHtml($oProduct->getName()); ?></a></h5>
</span>
<span>
<?php echo $oItem->getData( 'qty' ) . ' x '; ?>
<?php echo '$' . sprintf( '%.2f', $oItem->getData( 'price' ) ); ?>
</span>
<span>
<div class="item-options">
<?php
$aOptions = $oOptionHelper->getCustomOptions( $oItem );
if( $aOptions ):
?>
<?php foreach( $aOptions as $aOption ): ?>
<?php $_formatedOptionValue = $this->getFormatedOptionValue( $aOption ) ?>
<div class="options">
<strong><?php echo $this->escapeHtml( $aOption['label'] ); ?> :</strong>
<?php echo $aOption[ 'value' ] ?>
</div>
<?php endforeach; ?>
<?php endif; ?>
</div>
</span>
<?php endforeach; ?>
</div>
<hr />
<?php // Order Shipping Address Section ?>
<div class="review-address">
<h5>Shipping Address</h5>
<a href="<?php echo $this->getUrl( 'securecheckout/guest/address' ); ?>">Edit</a>
<ul>
<li>
<?php echo ( empty( $oCustomerShipping[ 'firstname' ] ) ? '' : $oCustomerShipping[ 'firstname' ] ); ?>
<?php echo ' '; ?>
<?php echo ( empty( $oCustomerShipping[ 'lastname' ] ) ? '' : $oCustomerShipping[ 'lastname' ] ); ?>
</li>
<li>
<?php echo ( empty( $oCustomerShipping[ 'company' ] ) ? '' : $oCustomerShipping[ 'company' ] ); ?>
</li>
<li>
<?php echo ( ( $oCustomerShipping->getStreet( 1 ) ) ? $oCustomerShipping->getStreet( 1 ) : '' ); ?>
</li>
<li>
<?php echo ( ( $oCustomerShipping->getStreet( 2 ) ) ? $oCustomerShipping->getStreet( 2 ) : '' ); ?>
</li>
<li>
<?php echo ( empty( $oCustomerShipping[ 'city' ] ) ? '' : $oCustomerShipping[ 'city' ] ) . ', '; ?>
<?php echo ( empty( $oCustomerShipping[ 'region' ] ) ? '' : $oCustomerShipping[ 'region' ] ) . ' '; ?>
<?php echo ( empty( $oCustomerShipping[ 'postcode' ] ) ? '' : $oCustomerShipping[ 'postcode' ] ); ?>
</li>
<li>
<?php echo ( empty( $oCustomerShipping[ 'telephone' ] ) ? '' : $oCustomerShipping[ 'telephone' ] ); ?>
</li>
<li>
<?php // @TODO handle question mark hover on desktop as a helper, on mobile make it clickable ?>
<span>Gift Message (Optional)</span><a href="#" title="Give your gift a personal touch. Include a special message for your recipient on the order packing slip at no extra charge! (Packing slip does not show pricing information.)" class="tooltip checkout">?</a>
</li>
<li>
<textarea name="review[gift_message]" id="gift_message"><?php echo $this->helper( 'checkout_securecheckout/giftmessage' )->getGiftMessage(); ?></textarea>
</li>
</ul>
</div>
<hr />
<div class="review-shipping">
<h5>Shipping Method</h5>
<br />
<?php echo $oShipping->getShippingMethod(); ?>
<a href="<?php echo $this->getUrl( 'securecheckout/guest/shipping' ); ?>"> Edit</a>
<ul>
<li>
<ul>
<li>Have Promo Code</li>
<?php echo $sPromo; ?>
</ul>
</li>
<li>
<ul>
<li>Order Contact</li>
<?php echo $sEmail; ?>
<li><a href="<?php echo $this->getUrl( 'securecheckout/guest/shipping' ); ?>">Edit</a></li>
</ul>
</li>
</ul>
</div>
<hr />
<div class="review-payment">
<h5>Payment Method</h5>
<ul>
<li>
<?php if( $oPayment[ 'method' ] === 'purchaseorder' ): ?>
PO#: <?php echo ( empty( $oPayment[ 'po_number' ] ) ? '' : $oPayment[ 'po_number' ] ); ?>
<?php elseif( $oPayment[ 'method' ] === 'paypal_express' ): ?>
<img src="<?php echo Mage::getDesign()->getSkinUrl( 'images/paypal-review-logo.png' ); ?>" alt="PayPal Logo" />
<br />
<?php
echo ( ( isset( $oPayment[ 'additional_information' ][ 'paypal_payer_email' ] ) ) ? $oPayment[ 'additional_information' ][ 'paypal_payer_email' ] : '' );
?>
<?php elseif( $oPayment[ 'method' ] === 'paymentech' ): ?>
<?php echo $oPayment[ 'cc_type' ]; ?> ending in <?php echo $oPayment[ 'cc_last4' ]; ?>
<?php endif; ?>
</li>
<li>
Billing Address
</li>
<li>
<?php echo ( empty( $oCustomerBilling[ 'firstname' ] ) ? '' : $oCustomerBilling[ 'firstname' ] ); ?>
<?php echo ' '; ?>
<?php echo ( empty( $oCustomerBilling[ 'lastname' ] ) ? '' : $oCustomerBilling[ 'lastname' ] ); ?>
</li>
<li>
<?php echo ( empty( $oCustomerBilling[ 'company' ] ) ? '' : $oCustomerBilling[ 'company' ] ); ?>
</li>
<li>
<?php echo ( ( $oCustomerBilling->getStreet( 1 ) ) ? $oCustomerBilling->getStreet( 1 ) : '' ); ?>
</li>
<li>
<?php echo ( ( $oCustomerBilling->getStreet( 2 ) ) ? $oCustomerBilling->getStreet( 2 ) : '' ); ?>
</li>
<li>
<?php echo ( empty( $oCustomerBilling[ 'city' ] ) ? '' : $oCustomerBilling[ 'city' ] ) . ', '; ?>
<?php echo ( empty( $oCustomerBilling[ 'region' ] ) ? '' : $oCustomerBilling[ 'region' ] ) . ' '; ?>
<?php echo ( empty( $oCustomerBilling[ 'postcode' ] ) ? '' : $oCustomerBilling[ 'postcode' ] ); ?>
</li>
<li>
<?php echo ( empty( $oCustomerBilling[ 'telephone' ] ) ? '' : $oCustomerBilling[ 'telephone' ] ); ?>
</li>
<?php if( $sGiftCard ): ?>
<li>
Gift Card Information
</li>
<li>
<?php echo $sGiftCard; ?>
</li>
<?php endif; ?>
</ul>
</div>
<hr />
<?php echo $sCheckoutButtonHtml; ?>
</form>
<?=instead of<?php echo\$\endgroup\$