search

omise / omise-prestashop

Omise PrestaShop Plugin
https://docs.opn.ooo/prestashop-plugin
MIT License
4 stars 7 forks source link

Refactor admin #79

Closed jonrandy closed 4 years ago

jonrandy commented 4 years ago

1. Objective

Restructure a lot of the admin section's code to simplify the addition of new payment methods and remove repeated code from what was there already. Also removes pointless display of anything about payment methods not valid for the checkout currency, and does some general clean up of the codebase (removing dead tests)

2. Description of change

Layout of the admin page reconfigured somewhat so that it makes more sense and better reflects actual functionality (there was a setting before that claimed to switch the whole module on/off, when in reality it only toggled credit card payment). Moved from a single template to multiple templates - 1 for general settings, and others for individual payment methods (can also be re-used by multiple payment methods). Also added labels to show currency restrictions for individual payment methods if they have them, and stopped displaying anything about payment methods that are not available for the checkout currency.

3. Quality assurance

Re-checked all admin functionality working on both PrestaShop 1.7 & 1.6

4. Impact of the change

Admin page is now more sensible although no overall functionality has changed. It is also now easier to add new payment methods to the admin page.

5. Priority of change

Normal

6. Additional notes

C, E♭, G

jonrandy commented 4 years ago

Naming is a bit confuse.

  • For variables, I think it's best not to shorten it (hard to read).

What variables are you referring to?

  • For file, omise/payment_methods/templates/_admin.tpl looks a bit strange, maybe setting.tpl would be more clear (that would make sense for card_setting.tpl as well.

Or if we really prefer having admin, then maybe can consider of

omise/payment_methods/templates/admin/settings.tpl
omise/payment_methods/templates/admin/card_settings.tpl

I left the file setting.tpl as it was - I didn't pick the name.

I'm following a similar same convention to used previously. Starting with an underscore to keep it at the top of the file list. It's a generic template that is used for all payment methods. Possibly renaming to _all.php would be better (changed in current branch). There's no point having an admin folder since these are all admin related templates. Other non-admin templates are kept in default PS location to allow overriding by user if they so require. I've renamed folder admin_templates and dropped _admin from individual template names