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

Create remaining configuration settings #16 #37

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

Conversation

nikolalardev
Copy link

Story: Create remaining configuration settings #16

  • Fix for modman file
  • Create Advanced Settings in admin panel for sales -> liqpay

use Magento\Framework\Data\OptionSourceInterface;

/**
* Class ConnectionType
Copy link
Owner

Choose a reason for hiding this comment

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

Please fix the PHPDoc

Copy link
Author

Choose a reason for hiding this comment

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

Fixed.

*/
class Currencies implements OptionSourceInterface
{
const USD = 'USD';
Copy link
Owner

Choose a reason for hiding this comment

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

Please move these settings to the config.xml/di.xml file

@@ -36,6 +36,11 @@
<source_model>Pronko\LiqPayAdmin\Source\ConnectionType</source_model>
<config_path>payment/pronko_liqpay/connection_type</config_path>
</field>
<field id="payment_action" translate="label" type="select" sortOrder="30" showInDefault="1" showInWebsite="1" showInStore="0" canRestore="1">
<label>Payment Action</label>
<source_model>Magento\AuthorizenetAcceptjs\Model\Adminhtml\Source\PaymentAction</source_model>
Copy link
Owner

Choose a reason for hiding this comment

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

Payment Action should be created in the Pronko_LiqPayAdmin module

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

@@ -36,6 +36,11 @@
<source_model>Pronko\LiqPayAdmin\Source\ConnectionType</source_model>
<config_path>payment/pronko_liqpay/connection_type</config_path>
</field>
<field id="payment_action" translate="label" type="select" sortOrder="30" showInDefault="1" showInWebsite="1" showInStore="0" canRestore="1">
Copy link
Owner

Choose a reason for hiding this comment

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

ShowInStore should be set to 1

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

<config_path>payment/pronko_liqpay/maximum_order_total</config_path>
</field>
<field id="sort_order" translate="label" type="text" sortOrder="30" showInDefault="1" showInWebsite="1" showInStore="1">
<label>Sort Total</label>
Copy link
Owner

Choose a reason for hiding this comment

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

Sort Order

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

</pronko_liqpay_section>
</payment>
</default>
</config>
Copy link
Owner

Choose a reason for hiding this comment

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

Empty line is missing.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

<module name="Pronko_LiqPayAdmin" />
<module name="Pronko_LiqPayAdmin">
<sequence>
<module name="Magento_Sales"/>
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think we should add sequence for Sales module

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

*/
public function getAllowedTypes()
{
return ['VI', 'MC', 'AE', 'DI', 'JCB', 'DN'];
Copy link
Owner

@mcspronko mcspronko Aug 29, 2019

Choose a reason for hiding this comment

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

Please see cctypes in the LiqPayCardGateway/etc/config.xml.
There are only 2 currencies supported.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

<group id="pronko_liqpay" translate="label" type="text" sortOrder="100" showInDefault="1" showInWebsite="1" showInStore="1">
<label>LiqPay</label>
<fieldset_css>complex</fieldset_css>
<attribute type="displayIn">other_payment_methods</attribute>
Copy link
Owner

Choose a reason for hiding this comment

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

attribute node can be removed.

Copy link
Author

Choose a reason for hiding this comment

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

After discussion we decide to leave them for now.

<label>Card Payment</label>
<attribute type="expanded">1</attribute>
<frontend_model>Magento\Config\Block\System\Config\Form\Fieldset</frontend_model>
<field id="cctypes" translate="label" type="multiselect" sortOrder="10" showInDefault="1" showInWebsite="1" showInStore="0" canRestore="1">
Copy link
Owner

Choose a reason for hiding this comment

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

show in store = 1

Copy link
Author

Choose a reason for hiding this comment

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

Fixed.

</field>
<field id="order_status" translate="label" type="select" sortOrder="40" showInDefault="1" showInWebsite="1" showInStore="0" canRestore="1">
<label>New Order Status</label>
<source_model>Magento\Sales\Model\Config\Source\Order\Status\Processing</source_model>
Copy link
Owner

Choose a reason for hiding this comment

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

Please use Magento\Sales\Model\Config\Source\Order\Status

Copy link
Author

Choose a reason for hiding this comment

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

Fixed.

@mcspronko
Copy link
Owner

@nikolalardev hi there, any news on this pull request? Do you need help with this? please let us know.

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