First of all, I would like to thank you for making this plug-in available to the public. I think it is an simple and elegant solution.
There is a few questions that raised for me after giving this a try and reading the source code.
In your report, you are directly accessing WooCommerce properties which is considered bad practice and will actually throw a lot of notices in the admin area (“doing it wrong”). Instead of $order->billing_first_name you should use $order->get_billing_first_name() and so forth. This will ensure the plug-in to continue working if WooCommerce decides to change these properties. Apparently this is also relevant for caching.
You are using woocommerce_form_field( ) to build the select field. Then further down, you are building the field yourself. Is there a reasoning behind it? (I’m not saying it is not legit, just curious if this was done on purpose)
You are defining your class as “final” which means that it can not be extended and methods can not be altered. While I’m sure this has a reasoning, it defeats the purpose of OOP.
Lastly, In terms of transliteration, there’s a bunch of strings which are unnecessary to include into your textdomain, such as Report, Setting, Export, Customer Name, Order Status etc. Instead you may use WooCommerce’s or WordPress’ textdomain:
__( 'Customer Name', 'woocommerce-admin' ), __( 'Orders', 'woocommerce-admin' ), __( 'Other', 'woocommerce-admin' ) __( 'Other', 'woocommerce' ) __( 'Select an option…', 'woocommerce' ) __( 'Customer', 'woocommerce' )
I don’t fault you for this, in fact I’m seeing this a lot of times in a lot of Plug-Ins but I think with a bit of research in our IDE we can often find strings which already have been included in core or contributed plug-ins. While we can’t always depend on other plugins in our translation, it is the case here because we are extending WooCommerce functionality.This way it will be easier to maintain and more languages will be supported in the first place.
I hope you will find this review helpful. Thanks again for sharing your code.
- The topic ‘Good approach but several technical flaws’ is closed to new replies.