-
Notifications
You must be signed in to change notification settings - Fork 170
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
[4.x]: Shipping Method Options no longer allow extra properties/data #3271
Comments
johnnynotsolucky
added a commit
to johnnynotsolucky/commerce
that referenced
this issue
Sep 12, 2023
a ShippingMethodOption Resolves craftcms#3271 Craft Commerce's ShippingMethodOption erases the information of any custom ShippingMethod's that were used to create them. While not ideal, this change sets a new field, shippingMethod on the ShippingMethodOption class and also proxies the call to getShippingRules to use the attached ShippingMethod if it is present. Of course, other functions such as getName, getType, etc will not be overridden and would need to be accessed like $option->shippingMethod->getType().
johnnynotsolucky
added a commit
to johnnynotsolucky/commerce
that referenced
this issue
Sep 12, 2023
a ShippingMethodOption Resolves craftcms#3271 Craft Commerce's ShippingMethodOption erases the information of any custom ShippingMethod's that were used to create them. While not ideal, this change sets a new field, shippingMethod on the ShippingMethodOption class and also proxies the call to getShippingRules to use the attached ShippingMethod if it is present. Of course, other functions such as getName, getType, etc will not be overridden and would need to be accessed like $option->shippingMethod->getType().
Fixed in 531a4d3 To get the fix early, change your "require": {
"craftcms/commerce": "dev-develop#531a4d3c8019a87657b553d35273549d29acd7c2 as 4.3.0",
"...": "..."
} Then run Thanks! |
Commerce |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
What happened?
Referencing this issue and the Postie plugin.
Postie registers shipping methods, that are attached with
ShippingMethods::EVENT_REGISTER_AVAILABLE_SHIPPING_METHODS
. We use our ownShippingMethod
class that extends the CommerceShippingMethod
class. In this, we store information about the carrier used (like UPS), and for each rate and service returned from the API, we store additional information like estimated delivery - not just the price.What Commerce does is grab all available shipping methods, and create
ShippingMethodOption
models. I'm not sure why this is done, but it's been this way since Commerce 3.x I believe. Users then usecart.availableShippingMethodOptions
on their shipping page.The issue we have is that through this translation of
ShippingMethod
toShippingMethodOption
, we lose Postie's extra information. DespiteShippingMethodOption
also extending from aShippingMethod
class, it's the Commerce class that's extended, and not Postie's. As such, any reference to the extra things Postie does is lost.Might I suggest recording the original
ShippingMethod
that was used to create theShippingMethodOption
be added? We could add the following here:Which would store Postie's shipping method against the
ShippingMethodOption
for people to access in their templates.Other options are:
data
for storing arbitrary data for aShippingMethodOption
Craft CMS version
4.5.3
Craft Commerce version
4.2.11
PHP version
No response
Operating system and version
No response
Database type and version
No response
Image driver and version
No response
Installed plugins and versions
The text was updated successfully, but these errors were encountered: