search

spree-contrib / spree_active_shipping

Spree integration for Shopify's active_shipping gem.
http://guides.spreecommerce.org
134 stars 207 forks source link

unit_multiplier cannot be decimal + possible documentation bug #188

Open traboukos opened 10 years ago

traboukos commented 10 years ago

Hi,

I am trying to use ups shipping calculators with the Spree 2.0 branch and weight units in grams and I am having some problems. The documentation in the README states.

It is important to note that by default this variable is set to have a value of 16 expecting weights to be entered in lbs

Say you have your weights in kg you would have to set the multiplier to 0.0283495

Spree::ActiveShipping::Config[:unit_multiplier] = 0.0283495

If I have product weights in lbs the multiplier should be 16. 1 lbs is equal to 16 oz. According to the previous sentence if my weights are in kilos then the multiplier should be 35.274 since 1 kg is equal to 35.274 oz. However the example in the documentation uses a multiplier of 0.0283495 which is the inverse. It is actually converting from oz to kg.

Furthermore and to the actual problem decimals cannot be entered in the multiplier value due to the fact that unit_multiplier is defined as an integer property. https://github.com/spree/spree_active_shipping/blob/master/lib/spree/active_shipping_configuration.rb#L18

In my case I store my product weights in grams so my multiplier should be 0.035274.

mrpollo commented 10 years ago

I can confirm the unit_multiplier should allow decimals to enable conversions from other units other than pounds, would you be able to send a PR to fix this preference and the README?

traboukos commented 10 years ago

I seem to have fixed it locally by overriding the files in my application. I will be glad to send a pull request but there is another issue. Making the field decimal is not enough since decimal preferences in Spree are rounded to 2 places. In order for this to be able to work I had to override Spree::Preferences::Preferable and Spree::Preference classes that both convert the value to the respective type and allow room for 8 digits.

mrpollo commented 10 years ago

@traboukos see https://github.com/spree/spree/issues/4719

mrpollo commented 10 years ago

BTW can you submit a PR for the README? , nice find, thanks!

dpmccabe commented 9 years ago

It looks like the documentation is still wrong. Still says 0.02... for the kg conversion when it should be 35. Or am I'm confused?

jspizziri commented 9 years ago

@mrpollo, did this ever get fixed?