Calculators API
Marcin Raczkowski
After talk with schof on IRC we reached conclusion that having basic API
for calculators is good idea :)
I've spiked simple selector for currently available calculators, so
at least you won't have to type in class name manually :). results can
be seen in
http://github.com/swistak/spree/tree/566-select-for-choosing-shipping_calculator
LH ticket:
http://railsdog.lighthouseapp.com/projects/31096-spree/tickets/566-select-for-choosing-shipping_calculator
My method is basing configuration on preferences on shipping_method.
Alternatively we could create table for each shiping extension and
connect them 1-1 with shiping_method, so each instance of Calculator
would keep settings in table.
Any calculator extension is responsible to provide constant
Spree::NameOfTheExtensionWithoutExtensionAtTheEnd::Calculator
Object present at that constant has to respond to following methods:
.name - name of the calculator class(current behaviour)
.description - provides shot description on how this plugin operates. To
be show in select/radio when configuring shipping_method
.required_preferences - list of preferences that shiping_method instance
must have. eg. for FlatRateCalculator that would be simply "rate".
.calculate(shippment) - should calculate charge based on shipment and
shiping_method
This allows for some flexability, but doesn't solve all problems. We can
calculate charge (with negative amount) for discount and add it to
order. Then we calculate tax and shipment cost separatly with that
charge included, and finally add them both to order at the same time.
One real life problem I'm currently facing, I have shop that sells
sports equipment - everything from training gloves and pilates mats, to
pool tables, and heavy gym equipment.
They are using system where you pay some basic fee, then for 5 next item
it steadilly incrases, then it stops, and no mater how much you order
you pay the same amount. But they offer two delivery options (so
shippment_methods) which have different rates, so I need separate
settings for the same calculator based on shipment_method, not so hard
so far right?
Nope, they also have heavy stuff - like for example pool table, wchich
they ALWAYS charge flat rate to deliver. So that's the job for
shipment_category right?
So mayby we need ability to choose calculator based on combination of
shiping_method and schiping_category ?
I'll be happy to hear all suggestions, questions, and feature requests.
What's more there are good chances I'll implement this feature soon
(becouse that's what they pay me for :) ), so please don't be shy and
tell me what you need.
Sean Schofield
ShippingCalculator < Extension. Then you can check
extension.is_a?(ShippingCalculator) rather then relying on a naming
convention.
The new person writing a shipping calculator for the first time will
likely look at one of the existing ones. When they see extending
ShippingCalculator that is more obvious IMO. Not sure about
preferences yet, I'll chat with you in IRC about them.
Sean
Jeff Seibert
slouchback.com:
In our case, we wanted to use the same ShippingCalculator with a bunch
of different shipping methods, so we could offer UPS Ground, UPS 2
Day, etc all to the same zone.
When the calculator was being called, however, it could not figure out
which shipping method it was supposed to be computing, because it was
passed only the shipment as it's parameter.
I don't know if there is a preferred way to do this, but I was forced
to change the calculate_shipping definition to:
def calculate_shipping(shipment, shipping_method)
...
end
And then change the Spree code to pass the correct method for every
calculation. Is this going to be supported with these changes?
Thanks,
Jeff
Sean Schofield
> which shipping method it was supposed to be computing, because it was
> passed only the shipment as it's parameter.
>
> I don't know if there is a preferred way to do this, but I was forced
> to change the calculate_shipping definition to:
>
> def calculate_shipping(shipment, shipping_method)
> ...
> end
>
> And then change the Spree code to pass the correct method for every
> calculation. Is this going to be supported with these changes?
If you have access to the shipment object you should have access to
shipping_method (shipment.shipping_method). That said, I'm open to
suggestions on improving things.
> Thanks,
>
> Jeff
Sean
Jeff Seibert
is called.
Jeff
>
>> Thanks,
>>
>> Jeff
>
> Sean
>
> >
Sean Schofield
part referring to a call for each shipping method? Did you guys use
the edge code (I seem to recall you did not.) I think things have
improved in the edge to make this easier. We're now using "fake
shipments"[1] to calculate the rates and we don't actually calculate
the shipping charge until a shipping method is explicitly chosen[2].
Not sure if these developments make things any easier for you.
Sean
[1] http://github.com/railsdog/spree/blob/7c669e8ff15e8032958ccf5f783094276506a859/app/controllers/checkouts_controller.rb#L75
[2] http://github.com/railsdog/spree/blob/2b4bf0251bdedda1c22e60500154a4aab6f47f6f/app/models/checkout.rb#L36
Paul Callaghan
earlier discussion (on #463) about the interface to shipping
calculators.
What's the minimum that calculators should require? I think it is just
the set of line items and an address, and indeed, that's exactly what
the checkout code passes in its "faked" shipment objects. The fact
that it's wrapped up in a shipment object could lead to
misunderstandings, eg where's the shipping method info gone?
Some sites might also restrict shipping to a single country, meaning
the address can be optional (remote Scottish islands being an
exception - we had one this week (-:), but also meaning that it's
feasible to calculate and show delivery estimates before the start of
checkout.
I think it's worth switching to an interface of line items plus
(optional) address if we're doing some refactoring in the area. It
could also simplify the implementation of multiple shipments, in the
sense of needing less juggling of fake shipment objects...
Paul
Paul Callaghan
> In our case, we wanted to use the same ShippingCalculator with a bunch
> of different shipping methods, so we could offer UPS Ground, UPS 2
> Day, etc all to the same zone.
>
> When the calculator was being called, however, it could not figure out
> which shipping method it was supposed to be computing, because it was
> passed only the shipment as it's parameter.
Design-wise, I think it's cleaner to have a separate calculator class
for each of the shipping methods you have declared, rather than trying
to get a single calculator implementation to decide which method it
should be calculating for.
That is, a UPS_Ground calculator, a UPS_2Day calculator, ...
Paul
William Emerson
to expect, is a way to get a shipping estimate before checkout. Of
course, depending on the shipping method, you may need to get the
country and postal code and then do the calculation. Since Spree
creates the order when the customer puts items in the cart, it should
be fairly easy to calculate shipping at that point, yes?. Some systems
have problems because the cart items are separate from the order items
and not all info is available before checkout. Do you see any problems
implementing this? I am working my way through Spree's code and will
be working on shipping soon. I have implemented a rails system using
active_shipping and one based on order total. Next is one using Spree
with a weight-based system. I will create it as an extension and let
you know how it goes.
Keep up the great work,
Will Emerson
Marcin Raczkowski
Most people need shipping estaminate before placing order. I've run into
same problem, I've fixed it by creating default address based on default
country(set in Spree::Config) then adding it to fake shipment.
This way I could calculate shipments costs for all shipping methods, and
display it to the user so he can choose knowing the estaminate.
I agree with paul that it would be best to use separate calcualtors, but
sometimes It's to much work if you for example need to add static amount
to each order if its' "express".
Today I'll get Address book extension by Steph merge it with my changes
to calculator and format a patch for trunk.
It should include:
- default adresses (based on country and/or address book)
- calculating delivery costs for default adresses, and recounting when
adress is known.
- better handling of one zone (skip some choices, and assume default if
only one zone is present)
- well defined API for calculators (mayby with some kind of lint)
- better choosing of calculators from admin interface.
I'll try to acomodate all cases you reported. Thank you for your
suggestions, if you have more don't hesitate :).
Paul Callaghan
> sometimes It's to much work if you for example need to add static amount
> to each order if its' "express".
I don't follow. Maybe this is solved by having a superclass do the
main work and just have subclasses with small bits of config?
Paul
Marcin Raczkowski
results can be seen here:
http://github.com/swistak/spree/tree/579-allow-calcualtor-configuraiton-shipping-coupons-etc-via-admin-interface
What you might want to look at:
+++ b/app/controllers/admin/configurations_controller.rb
^ method to make adding configuration links easier
+++ b/app/helpers/configurable_field_helper.rb
^ helper to display optional fields
+++ b/app/models/calculator_model.rb
^ abstract model for calculators
+++ b/test/unit/model_configuration_test.rb
+++ b/app/models/model_configuration.rb
+++ b/lib/configurable.rb
^ per instance configuration
b/vendor/extensions/flat_rate_shipping/
^ new experimental version of flat rate shipment extension
Current problems:
one table for all calculators +single table inheritance + custom
settings via "configurable" VS. table for each calculator
first solution:
+ one table, less duplication
+ easy extensability for extensions
+ inheritance provides many nice defaults including stock admin interface
- slightly more complicated
second solution:
+ slightly easier to port exisiting extensions
+ more customization options
- one table per calculator,
- have to create migrations
- harder customization
Me and Schof(if I understand him correctly) are siding with first one.
Current solution is somwhere in the middle but will have to swing one or
another.
Second problem:
do we depend only on shipping_method and let algorithm decide if he
wants to honor shipping_category, or do we make calculator instance
dependant on both.
Thrid:
as alternative to second, do we allow to choose which
shipping_categories calculator supports?
I'm open for suggestions