Create base classes for fundamental modifier categories (e.g., tax and shipping)

[GoogleCodeExporter]

It would be very helpful if common order modifier types each had a common 
base-class + API. For example, the Order class has a TaxInfo() function, that 
looks for an order modifier that is an instance of TaxModifier.** So long as a 
tax modifier is a child-class of TaxModifier, this function will find and 
return it.

Right now I'm working on getting Google Analytics ecommerce tracking up and 
running, and I very much wish that there were a ShippingModifier base class, so 
that I could easily get an order's shipping amount, and add it to the tracking 
data.

There may be other categories of modifiers where a common base class + API 
would be useful.

** NOTE: Sadly, not all tax related modifiers are child-classes of TaxModifier. 
Maybe I should submit a separate bug report for this...

Original issue reported on code.google.com by hjr29s...@gmail.com on 17 Oct 2012 at 3:33

[GoogleCodeExporter]

Hi Hans

I agree with your idea, but I would like to propose a different solution.  
Rather than a very strict sub-class, why not have a method that returns the 
type. That is a bit more flexible. 

Original comment by nfranc...@gmail.com on 30 Oct 2012 at 6:48

• Changed state: Accepted
• Added labels: Milestone-Release2.0, Priority-High

[GoogleCodeExporter]

Hi Nicolaas,

I don't think that your suggestion is a good idea, because it doesn't enforce a 
common API. The whole point of having categories/types is to have a common API. 
For example, if I have a tax modifier, then I want to have predefined methods 
for extracting the tax rate and amount details.

If you want something more flexible, then declare the API as an interface. An 
interface is more flexible than a class,** but still provides a common API. 
Plus, it works with instanceof, so it's still easy to determine if a class is 
the right type.

** A class can have only one parent class, but can implement multiple interfaces

Original comment by h...@keasigmadelta.co.nz on 30 Oct 2012 at 7:07

[GoogleCodeExporter]

Just to avoid confusion, the reply above is from me (Hans). I now have multiple 
Google accounts.

Original comment by hjr29s...@gmail.com on 30 Oct 2012 at 7:11

[GoogleCodeExporter]

yes, ok - I hear what you are saying.  Makes sense... 

It would be a big change so I am a bit hesitant.  The other thing I wonder 
about is:

What categories should we use.

What do you think about interfaces (e.g. tax_interface, delivery_interface, 
etc....)

Why exactly do we need it? Maybe that is a more pertinent question.

Original comment by nfranc...@gmail.com on 30 Oct 2012 at 7:40

[GoogleCodeExporter]

The purpose is to enable ecommerce modules to get additional information about 
an order when necessary. That way custom functionality can be added without 
hacking the core.

Here are the few examples that I can think of:
- Send the GST component of a sale to the transaction processing service (e.g., 
it looks better if this info is also on the PayPal invoice)
- Show the address and shipping costs on the PayPal invoice (and maybe other 
transaction services too)
- Extract the shipping country/region to add to the Google Analytics

It doesn't have to be a huge change. GST and shipping are the only two 
categories that I can think of. So, unless you can think of any more, those 
would be the only two categories. Plus, the GST case is actually already done 
via the TaxModifier class, which leaves just the shipping modifier category.

I suggest that you take a look at Order::TaxInfo(), which extracts the GST 
modifier by looking for an instance of TaxModifier. This is what I use in my 
custom PayPalExpressCheckoutPayment class. It's also what gave me the idea for 
this enhancement ticket.

Original comment by hjr29s...@gmail.com on 30 Oct 2012 at 9:44

[GoogleCodeExporter]

Thank you for your super clear concept description.

I can think of the following categories:

* electronicDocModifier
* priceChangeModifier
* taxModifier
* deliveryModifier
* manageOrderItemsModifier
but I am sure there are more. Maybe they go in OtherModifiers

What you are basically saying is that you want to be able to extract a bunch of 
variables from Order without having to put all the code in the order class 
(e.g. total tax, total shipping cost, region + country (I think this is already 
in core).

I think there might be other ways to achieve this. 

The basic idea is that you have

[ order] -> hasMany [objects]

and that you want to query [order] to get info that is stored in these objects. 

it is similar to 

"isSubmitted"
"shippingAddres"

etc....

I am just wondering if we could something more generic.

The other thing I am thinking about is to make Order Extendable.  That again, 
might be much easier. 

Basically, in the EcomConfig, we set the Order Class, which can be Order 
(default) or MyOrder (which may, but does not have to, extend Order).

We then create interfaces (basically rules for classes) that tell you what the 
Order should have (e.g. SubTotal, Total, bla bla). 

Original comment by nfranc...@gmail.com on 30 Oct 2012 at 10:24

[GoogleCodeExporter]

I'm not 100% sure that we're on the same wavelength with this. In essence, I'm 
suggesting following the Order::TaxInfo()->TaxModifier paradigm, and provide 
common APIs for extracting common modifier data irrespective of which specific 
modifiers are installed. 

Back to the shipping data => Google Ecommerce Analytics example. Right now I 
could create a solution that would work on my particular setup. However, it 
would be specific to the shipping modifiers that I happen to use. So, if I use 
the SimpleFlatShipping modifier, then the code wouldn't work for someone using 
the FastwayNZShippingModifier class. Defining a common interface for all 
shipping modifiers would enable any shipping data extraction code to work 
regardless of the shipping modifiers that are in use. 

If you can think of a more elegant way of achieving this, then great. So long 
as what I have described can be done efficiently and easily, I'm happy. Be 
careful about making it too generic though, as that can also make too 
complicated, too abstract, and difficult to use.

Original comment by hjr29s...@gmail.com on 30 Oct 2012 at 7:56

[GoogleCodeExporter]

It might be worth asking some other people for their input/opinions before 
making any design decisions. We're talking about defining common base APIs, so 
it's worth taking the time to make sure that they offer the right functionality.

Original comment by hjr29s...@gmail.com on 30 Oct 2012 at 8:03

[GoogleCodeExporter]

Hi Hans

I agree with your points.  I do think we are on the same wave-length in that 
you want to extract data from the Order Class about specific topics (Tax, 
shipping, etc...) irrespective of the OrderModifier (and I would say, OR 
OrderStatusLog) that is being used.  Is this right? 

Original comment by nfranc...@gmail.com on 31 Oct 2012 at 3:58

• Added labels: Priority-Critical
• Removed labels: Priority-High

[GoogleCodeExporter]

Yes, that is correct.

Original comment by hjr29s...@gmail.com on 31 Oct 2012 at 5:05

[GoogleCodeExporter]

So, a bit more about the requirements:

You want to able extract data from order in such a way that:

1. data you can rely on and it always there (e.g. what is the totaltax), 
irrespective of what Modifiers / OrderStatusLogs are there. 

2. customisable (e.g. total tax can be defined in a variety of ways, depending 
on the project)

3. saved to the database once the order has been submitted to (a) increase 
reporting / search speeds (b) to ensure that the amount stays the same even if 
the code changes.

Did I miss anything?

I think once we have the requirements super clear it will be easier to find the 
best way forward. 

Original comment by nfranc...@gmail.com on 31 Oct 2012 at 7:12

[GoogleCodeExporter]

Yes, that's it. I wasn't thinking about point number 3, but obviously the data 
does need to be recorded in some shape or form.

Original comment by hjr29s...@gmail.com on 31 Oct 2012 at 10:29

[GoogleCodeExporter]

About point number 3, isn't saving the data to the database up to each 
modifier? So long as there is a set API to extract common data, it shouldn't 
matter how it is stored in the database. For example, if there is a TotalTax() 
method in the tax interface, then a custom module wouldn't care if the tax 
modifier stored it directly, or calculated it from other stored data.

Original comment by hjr29s...@gmail.com on 1 Nov 2012 at 2:18

[GoogleCodeExporter]

About point number 3.... yes, that is true. 

Original comment by nfranc...@gmail.com on 1 Nov 2012 at 3:15

[GoogleCodeExporter]

This idea: common ancestors for common modifier classes should be implemented 
using Interfaces!

Original comment by nfranc...@gmail.com on 31 Jan 2013 at 11:59

[GoogleCodeExporter]

Original comment by nfranc...@gmail.com on 22 Aug 2013 at 7:09

• Added labels: Milestone-Release
• Removed labels: Milestone-Release2.0