solidusio-contrib / solidus_active_shipping Goto Github PK

There is nothing in the README explaining how this affects the building of packages.

Spree::ActiveShipping::Config[:units] is something that is used in the base calculator to optionally specify :imperial as the units when creating ActiveShipping::Package instances.

This is currently not documented in the README, so we should add it and also ensure if that configuration value is used properly throughout the code base.

Curently package dimensions that are sent to active_shipping default to []

It looks like there has been a considerable amount of activity in the master branch of this repo, but there has only been one rubygems release. Can we get a new release cut that incorporates these changes?

Also, I don’t see branches for v2.1, v2.2, etc., as are present in most of the other Solidus gems and extensions. That might be another helpful thing for compatibility. As it stands, I’m running Solidus 2.1 in production and am not really sure what to be using -- whether the 1.0.0 gem, the master branch here, the last commit before 2.2 support was added, etc.

Cheers!

It appears that solidus_active_shipping 1.0.0 is not compatible with solidus 1.0.0 -- specifically, that it has a dependency on solidus > 1.0. This is surprising and counterintuitive to me. I’m working my way up the upgrade path from Spree now – how would I go about installing a solidus_active_shipping gem compatible with solidus 1.0.0?

Solidus seed scripts create a default stock_location with just a name 'default'. It is now no longer possible to initialize a new solidus instance with this gem bundled because of these validation rules.

This has caused me way more grief than having a stock location without address info would.

How to reproduce :

1. Set handling_fee to 1
2. Retrieve rate for service A
3. Set handling_fee to 0
4. Retrieve rate for service A, it's fetched from the cache and it will include the handling_fee from 1)

There are some bugs related to the max_weight by countries in USPS (mostly nil errors when the country is not in the list). In general I need to confirm that individual calculators outside of the base.rb are working properly

Currently the Base calculator handles the building of ActiveShipping::Package models, which should be extracted into one or more service objects. A big part of the logic is around building packages based on the maximum allowable weight for a package, which should still be configurable on the calculator level and passed to the service object responsible for building the packages.

There has been an effort here #10 to add better tests for the weight calculation logic which should help us ensure we preserve the existing logic.

@jhawthorn Now that in-memory preferences is default as of Solidus 2.4, I notice that preferences should no longer be editable from the admin backend. Instead, the new best practice is to include all preferences (e.g., FedEx credentials) in initializer files.

This creates a potentially problematic situation where the preferences displayed on the backend appear editable, but those changes will not persist after a deployment or a server reboot.

It seems to me that if a particular Solidus site is using in-memory preferences, we should either remove the ActiveShipping settings entirely, or make it read-only. Do you agree?

Happy to prepare a PR if so. Cheers!

This extension adds a ProductPackage model which possibly facilitates the ability to define how products should be broken down into packages when getting rates. This functionality is not documented anywhere in the README.

Per this line https://github.com/solidusio-contrib/solidus_active_shipping/blob/master/app/models/spree/calculator/shipping/active_shipping/base.rb#L136

This isn't always a hash, there may be an array of errors here, and thus the handler fails when trying to call key? on the array. We should either add further type checking, or use Array.wrap here.