Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ShippingMethodOption error breaking checkout #1378

Closed
keyurshah opened this issue Apr 13, 2020 · 5 comments
Closed

ShippingMethodOption error breaking checkout #1378

keyurshah opened this issue Apr 13, 2020 · 5 comments
Assignees

Comments

@keyurshah
Copy link

keyurshah commented Apr 13, 2020

Description

It seems with the latest craft 3.1.1, an error is being caused during shipping calculation.

This error is critical and prevents adding to cart and checking out.

"Setting unknown property: craft\commerce\models\ShippingMethodOption::provider"

This error does not always occur though. It seems to happen when user is logged in.

We are using verbb/postie to handle shipping.

verbb/postie#27

It seems that it is with this commit

4f139cd

Steps to reproduce

  1. Add an item to cart
  2. Error is returned from json

Additional info

  • Craft commerce version: 3.1.1
  • PHP version: 7.4
@keyurshah keyurshah changed the title ShippingMethodOption Error ShippingMethodOption error breaking checkout Apr 13, 2020
@engram-design
Copy link
Contributor

engram-design commented Apr 13, 2020

As @keyurshah states this is from 4f139cd and specifically 4f139cd#diff-a71a5371ab7c5db4ebcffcab49d15a0fR1573 where its using all the attributes in a shipping method class, and trying to set the respective attribute on the ShippingMethodOption class.

This issue with Postie, and I'm sure any other custom shipping method, is that there will almost certainly additional attributes on classes. Basically, it means we can't have any public attributes on the shipping method class that doesn't exist in the ShippingMethodOption class.

Probably needs to be smarter than $method->attributes - unless I can do something? I can't make the attributes in my shipping method class private/protected because they need public access.

Let me know if I should be doing something on Postie's end?

Refer to verbb/postie#27

@lukeholder
Copy link
Member

Yeah looks like a bug. Will fix shortly.

@lukeholder
Copy link
Member

lukeholder commented Apr 14, 2020

This has been fixed for the next release.

Thank you for bringing this to our attention.

A fix has been pushed and will be included in the next release.

To get this early, change your craftcms/commerce requirement in composer.json to:

"require": {
  "craftcms/commerce": "dev-develop#09f66725e201f28bb2b59c65afc374329c1c0952 as 3.1.1",
  "...": "..."
}

Then run composer update

Thanks.

@engram-design
Copy link
Contributor

@lukeholder Tested and working, nice one!

@engram-design
Copy link
Contributor

@lukeholder Something seems to have changed to make this no longer work.

Again, Postie provides its own verbb\postie\models\ShippingMethod model which represents an option for people to pick from a remote provider. These get created as ShippingMethodOption models via Commerce, which essentially guts anything Postie does in its model class, and swaps for a craft\commerce\models\ShippingMethodOption model, which inherits from craft\commerce\base\ShippingMethod (which our Postie classes also do).

This population of a new model is where things fall over. Because these new ShippingMethodOption classes don't use our Postie-provided shipping method classes, a lot of the extra data is lost.

Might I recommend storing the shipping method that's being used to create the shipping method option? I know that one extends the other, but it doesn't factor in any third-party setups.

$option->shippingMethod = $method;

Or, maybe another approach is to add an event for us to hook into which is a before/after populate/create ShippingMethodOption where we can add our own data? It'd still require properties to store that stuff.

Lastly, we could try and figure out a way for Postie to implement it's own ShippingMethodOption (probably needs to be a new base class and interface) that gets used instead of the Commerce ShippingMethodOption class. This would bring things back to the Commerce 3.x method, where Postie can actually control the options used for users to pick shipping options for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants