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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removes specific internet banking fee messages and replaces them with a generic one #144

Merged
merged 4 commits into from Aug 30, 2018

Conversation

Projects
None yet
3 participants
@jonrandy
Contributor

jonrandy commented Aug 29, 2018

1. Objective

Remove the specific fee information labels for internet banking since these fees are subject to change quite frequently (in fact they are already out of date). Replace them with a generic message telling the user that the bank may charge them a small fee for the transaction.

2. Description of change

The HTML for the internet banking block was altered as described above

3. Quality assurance

Checked to see it looked okay

4. Impact of the change

User now always receives appropriate information about charges, regardless if the banks change the charge amounts. Maintenance burden for this for us is also removed.

screen shot 2018-08-29 at 17 00 51

5. Priority of change

Normal

6. Additional Notes

馃挼 馃挼 馃挼

Show outdated Hide outdated ...ult/template/payment/form/omise/omiseoffsiteinternetbankingpayment.phtml
Show outdated Hide outdated ...ult/template/payment/form/omise/omiseoffsiteinternetbankingpayment.phtml
@@ -11,7 +11,7 @@ class Omise_Gateway_Block_Form_Offsiteinternetbankingpayment extends Mage_Paymen
protected function _construct()
{
parent::_construct();
$this->setTemplate('payment/form/omiseoffsiteinternetbankingpayment.phtml');
$this->setTemplate('payment/form/omise/omiseoffsiteinternetbankingpayment.phtml');

This comment has been minimized.

@jacekstan

jacekstan Aug 29, 2018

Contributor

is that part of another PR? that code?
you probably did not checkout from master this branch?

@jacekstan

jacekstan Aug 29, 2018

Contributor

is that part of another PR? that code?
you probably did not checkout from master this branch?

This comment has been minimized.

@jonrandy

jonrandy Aug 29, 2018

Contributor

I did, but another PR is incoming (waiting approval) that has that. My local project will not work without it so I needed it here too. It will all come out in the wash

@jonrandy

jonrandy Aug 29, 2018

Contributor

I did, but another PR is incoming (waiting approval) that has that. My local project will not work without it so I needed it here too. It will all come out in the wash

This comment has been minimized.

@jonrandy

jonrandy Aug 29, 2018

Contributor

I thought <?= might be dangerous as it used to be tied to the the short_tags PHP setting which we could not guarantee was switched on. However, I just checked, and since PHP 5.4 the short echo syntax is separate from short_tags and will always work. As a result, I'll use it. We could actually tidy a lot of templates with this 馃槃

@jonrandy

jonrandy Aug 29, 2018

Contributor

I thought <?= might be dangerous as it used to be tied to the the short_tags PHP setting which we could not guarantee was switched on. However, I just checked, and since PHP 5.4 the short echo syntax is separate from short_tags and will always work. As a result, I'll use it. We could actually tidy a lot of templates with this 馃槃

This comment has been minimized.

@jacekstan

jacekstan Aug 29, 2018

Contributor

After bit thinking if it is since 5.4 I think we should keep:
<?php echo "text" ?>
Because we have no idea.. what kind of servers are used by customers.
And since it is Magento 1, I think better to play it safe.

And additionally I have checked magento 1.9.2.4 and there is no single place where they use construction <?= ?>

@jacekstan

jacekstan Aug 29, 2018

Contributor

After bit thinking if it is since 5.4 I think we should keep:
<?php echo "text" ?>
Because we have no idea.. what kind of servers are used by customers.
And since it is Magento 1, I think better to play it safe.

And additionally I have checked magento 1.9.2.4 and there is no single place where they use construction <?= ?>

@jonrandy

This comment has been minimized.

Show comment
Hide comment
@jonrandy

jonrandy Aug 30, 2018

Contributor
Contributor

jonrandy commented Aug 30, 2018

@jonrandy

This comment has been minimized.

Show comment
Hide comment

@jonrandy jonrandy merged commit 5a3fa80 into 1-stable Aug 30, 2018

@jonrandy jonrandy deleted the fix-ib-fee-warnings branch Aug 30, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment