Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple fixes and enhancements #41

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Multiple fixes and enhancements #41

wants to merge 13 commits into from

Conversation

MaWoScha
Copy link

@MaWoScha MaWoScha commented Dec 6, 2017

Fix localization issues
Add locale 'en_GB' and 'pt_PT'
SR-714 allow capture offline'
Fixed 'Display Zero Fee' in customer account order view (#21)
Add 'cod_fee', 'base_cod_fee' attributes to webservices (#33)
Update to better fit ECG/PSR-2
Removed not used observer method, Renamed observer methods
Hide config options if module is disabled
Add 'Create Invoice Option'
Improve locale 'it_IT'

Copy link
Collaborator

@PHOENIX-MEDIA PHOENIX-MEDIA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please review some of the commits and make appropriate changes. Thanks for your support!

@@ -44,7 +44,7 @@ public function initTotals()
{
$this->_prepareTotals();

if ($this->_totalObject && $this->_totalObject->getCodFee()) {
if ($this->_totalObject && $this->_totalObject->getCodFee() && Mage::getStoreConfigFlag('payment/phoenix_cashondelivery/display_zero_fee')) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will not work... If codFee check returns false the last && will not be validated. If the codFee Check returns true and display_zero_fee is set to false the total block is not added even if a fee is set.

// If order is instance of Phoenix_CashOnDelivery_Model_CashOnDelivery
if ($order->getPayment()->getMethodInstance()->getCode() == 'phoenix_cashondelivery' &&
// Can be invoiced
$order->canvoice() &&
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: canvoice() should be canInvoice()

@@ -48,15 +48,15 @@
<show_in_website>1</show_in_website>
<show_in_store>1</show_in_store>
<fields>
<active translate="label">
<enable translate="label">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mage_Payment_Model_Method_Abstract::isAvailable() checks for "active" config path, not "enable".

<frontend_type>multiselect</frontend_type>
<sort_order>70</sort_order>
<source_model>adminhtml/system_config_source_country</source_model>
<show_in_default>1</show_in_default>
<show_in_website>1</show_in_website>
<show_in_store>1</show_in_store>
<depends>
<enable>1</enable>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hiding config data for the payment method when disabled is not something we want to add. Displaying the configuration is often desired even if the method is (temporarily) disabled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants