Conversation
23ff9f5 to
ee84766
Compare
jonrandy
left a comment
There was a problem hiding this comment.
Can't give final approval yet as this looks like it really needs at least a quick test. Can re-review after that, but have left some comments in the mean time
| protected function initiate() { | ||
| public function init_error_messages(){ | ||
| ?> | ||
| <div class="error"> |
There was a problem hiding this comment.
Whoa! Inline HTML!?!?! Is this really how things are done in WooCommerce? Feels very wrong - especially the fact the HTML is totally hardcoded
There was a problem hiding this comment.
@jonrandy yeah.. it's WordPress nature of code..
I know how you feel..
| * Omise's charge id as a 'customed-post-meta' in the | ||
| * WooCommerce's 'order' post-type instead. | ||
| */ | ||
| public function register_post_types() { |
There was a problem hiding this comment.
If it is deprecated? Why is it here? Is it to maintain support for older versions?
There was a problem hiding this comment.
@jonrandy Not really for older versions but more like, for old-orders.
For example, those stores that have been using Omise legacy version and later updated to v3.x, those orders that have been created with Omise-WooCommerce below v3.0 will still refer to this custom-post-type.
We may have a migration script to migrate all those data into WooCommerce Order's post meta. But just it's not that high priority, so I choose to leave the code here instead.
…rated-concern individual methods.
…om 'Omise_Payment_Creditcard' class out to the core class. Here I am trying to separte between a code that taking care of the initiative part and a code that is not. As registering custom post type is not related and not require for any WooCommerce classes, so I'm relocating it out to the core class.
ee84766 to
363823f
Compare
1. Objective
Omise-WooCommerce has always been coupled with WooCommerce plugin, which the plugin cannot be used as a stand-alone plugin and always require WooCommerce plugin to be activated first.
However, the legacy-codebase itself wasn't really aware of a case where WooCommerce plugin is disabled as much as it should be.
Although we have prevented such cases #78, and #83. But by the current design of the plugin-initial step, it still remains some issue (#90) that could be ceased out by redesigning the initial-step with a proper awareness of WooCommerce plugin.
Related issue(s):
2. Description of change
Part 1
See, #94: "Refactoring plugin-initial code structure - part 1: Enhancing the behavior of checking dependency plugin"
Part 2
Chore works in part 2:
Relocating functions, renaming method names to be more generic, and grouping code to a proper place (method).
Part 3
See, #96: "Refactoring plugin-initial code structure - part 3: Organizing Omise_Admin class."
3. Quality assurance
🔧 Environments:
✏️ Details:
The main test-case will be separated into 3 cases as follows:
Omise-WooCommerce plugin can be activated, but all functions should be disabled, including no Omise menu at the WordPress admin-sidebar.
4. Impact of the change
No
5. Priority of change
Normal
6. Additional Notes
No