verbb / postie

A Craft Commerce shipping calculator plugin.
Other
12 stars 18 forks source link

Shipping Options not showing up. #112

Open thomasdefilippis opened 10 months ago

thomasdefilippis commented 10 months ago

Describe the bug

In the documentation, it says that the following code will return shipping method options:

{% for handle, method in cart.availableShippingMethodOptions %}
    <div class="shipping-select">
        <label>
            <input type="radio" name="shippingMethodHandle" value="{{ handle }}" {% if handle == cart.shippingMethodHandle %}checked{% endif %} />
            <strong>{{ method.name }}</strong>

            {# Get the first shipping rule for this method #}
            {% set shippingRule = method.getShippingRules()[0] ?? [] %}

            {% if shippingRule %}
                <code>{{ dump(shippingRule.options) }}</code>
            {% endif %}

            <span class="price">
                {{ method.getPrice() | commerceCurrency(cart.currency) }}
            </span>
        </label>
    </div>
{% endfor %}

however, it returns an empty array with ups and usps options selected.

In the function getShippingRules() in the file vendor/verbb/postie/src/models/ShippingMethod.php, I logged the rate options
CRAFT::info($this->rateOptions, 'shippingRul323')

and I got a bunch of responses like this in the web.log:

2023-08-22 20:32:10 [web.INFO] [shippingRul323] [
    'guaranteedDaysToDelivery' => '1',
    'scheduledDeliveryTime' => '10:30 A.M.',
    'rateShipmentWarning' => 'Your invoice may vary from the displayed reference rates',
    'surCharges' => [],
    'timeInTransit' => '',
] {"memory":7651928} 

This seems to be what I want to display, but after the $modifyRuleEvent is called, I logged:

CRAFT::info($modifyRuleEvent->shippingRule, 'shippingRul329');

and I got large object in the web.log that is very large but does not have those options as listed before (I can share on request, but it is very large).

$modifyRuleEvent->shippingRule is what gets returned at the end and there is nothing in it.

return [$modifyRuleEvent->shippingRule];

Any advice or pointers on what is the problem or whether I am incorrectly using the code?

Steps to reproduce

  1. use the following in a template
{% for handle, method in cart.availableShippingMethodOptions %}
    <div class="shipping-select">
        <label>
            <input type="radio" name="shippingMethodHandle" value="{{ handle }}" {% if handle == cart.shippingMethodHandle %}checked{% endif %} />
            <strong>{{ method.name }}</strong>

            {# Get the first shipping rule for this method #}
            {% set shippingRule = method.getShippingRules()[0] ?? [] %}

            {% if shippingRule %}
                <code>{{ dump(shippingRule.options) }}</code>
            {% endif %}

            <span class="price">
                {{ method.getPrice() | commerceCurrency(cart.currency) }}
            </span>
        </label>
    </div>
{% endfor %}

Craft CMS version

4.3.5

Plugin version

3.1.1

Multi-site?

No

Additional context

Happens on external staging site and my local

engram-design commented 10 months ago

The only thing I can think of with this is that you might be restricting services, and those services don't exist in the payload coming back from the APIs? Can you comment if you have this enabled:

image

I would recommend disabling it to ensure any services are reported, and go from there.

thomasdefilippis commented 10 months ago

Yes, that light switch was turned on for all of my providers (ups and usps) because I am only using 5-8 different shipping methods. Let me be clear, the shipping method names and prices are all working for me. It is just shipping rules and options.

So, I turned them all off, and I still didn't get any other service information in the templates and when I log the modifyRuleEvent. I get a large object which ends

"rateOptions";a:0:{}s:24:"shippingMethodCategories";N;}s:7:"options";a:0:{}}') {"memory":7324912} 

the rateOptions has zero in it. Is this function getShippingRules() the correct one to look at?

Are you experiencing the same issue? or is this the first example that you are hearing of?

Thanks for your help

engram-design commented 10 months ago

Oh, my mistake, I misread your request.

Yes, it looks like the Commerce team has changed something which means this no longer works. Refer to https://github.com/craftcms/commerce/issues/1378#issuecomment-1675674813 where I'm hoping this can be fixed.

But the short of it is, Postie creates ShippingMethod models for each available services to pick. Commerce then creates ShippingMethodOption models for each of those, which is a derivative of a ShippingMethod. I'm not sure why they do this, but the change was made in Commerce 4.x. The issue is that it's a derivative class of their ShippingMethod model, and not ours, which contains useful things.

For example, their ShippingMethod class will return an empty array for getShippingRules() unless it's a static (created in the control panel) shipping method. There's no way for us to override that at the ShippingMethodOption level.

What the issue is, is that the ShippingMethodOption contains no reference to our ShippingMethod, which stores extra information about the rate, which you can access. Now, that extra data is inaccessible.

thomasdefilippis commented 10 months ago

Okay, good to know.

I will hardcode some values for my client until this is figured out.

Thanks for your time and info.

thomasdefilippis commented 10 months ago

For anybody interested in a temporary solution to this problem. I created a superTableField called shippingMethodInfo as follows:

Screenshot 2023-08-24 at 11 18 57 AM.

This could be a table field instead, but I thought I may add more complex fields to it later. Anyways, my fields are

methodHandle: this is the code displayed for the service provider, estimatedTimeMessage: this is a message that will display for time, Sage Code: this is an internal code for business purposes

Then in the template I did the following:

{% set globals = craft.entries.title('Globals').one() %}
{% set shippingMethodInfo = globals.shippingMethodInfo.all() %}
{% for handle, method in cart.availableShippingMethodOptions %}
    <div class="shipping-select">
        <label>
            <input type="radio" name="shippingMethodHandle" value="{{ handle }}" {% if handle == cart.shippingMethodHandle %}checked{% endif %} />
            <strong>{{ method.name }}</strong>

            {% if shippingMethodInfo %}
                {% set currentMethodInfo = shippingMethodInfo | filter(item => item.methodHandle == method.handle) | first %}
                <p>{{ currentMethodInfo.estimatedTimeMessage }}</p>
            {% endif %}

            <span class="price">
                {{ method.getPrice() | commerceCurrency(cart.currency) }}
            </span>
        </label>
    </div>
{% endfor %}

The globals section is just a single that I created to store global data since the global data type is going to eventually be removed by craft. Anyways, not perfect, but it works.

peteeveleigh commented 10 months ago

Is there any way we can access Postie's shipping options directly without going through Commerce's availableShippingMethods?

peteeveleigh commented 10 months ago

@thomasdefilippis I'm not clear on how your workaround actually works. I understand the principle of making a global/single with a table for the shipping methods but I'm not following how the Twig part works since it's looping over cart.availableShippingMethodOptions but the Postie options aren't available so won't appear in there anyway.

engram-design commented 10 months ago

@peteeveleigh Not at the moment, sorry. The shipping methods generated by Postie are done on the ShippingMethods::EVENT_REGISTER_AVAILABLE_SHIPPING_METHODS event from Commerce, so they're available to it when figuring out what shipping methods to offer.

We would need to add a method to expose the same methods generated by this event for you to dive into to grab the extra options for shipping methods. We'd just want to be careful about potentially generating those API calls multiple times.

It's also not my preferred approach, and I'd like this issue addressed within Commerce directly, as I think it's a bug. I've created a new issue for visibility, and it's been crickets on that closed issue. https://github.com/craftcms/commerce/issues/3271

peteeveleigh commented 10 months ago

@engram-design yeah I had a play around yesterday and just made a variable which would get the shipping methods. However the rates all came in at 0 and, besides, the shipping method can't be added to the order since it's not available. You're right, it's something that needs to be handled on the Craft/Commerce side.

Thanks for creating that new issue too 👍

thomasdefilippis commented 9 months ago

So @peteeveleigh, my solution is not a long term one, but just a way to manually set rate options until a better solution is found. It just has a search for a table row that I created with identical codes to the method handles. AvailableShippingMethodsOptions is an array with all the methods and handles that is in the cart. This is not broken but rather the shipingMethodRules are not able to work since craft does not allow you to modify anymore.

Refer to the section on rate options in this documentation to see what I am talking about: https://verbb.io/craft-plugins/postie/docs/setup-configuration/displaying-rates.

johnnynotsolucky commented 9 months ago

I have a patch branch that should work for y'all, https://github.com/johnnynotsolucky/commerce/tree/custom-shipping-methods, which is branched off the 4.2.11 release so shouldn't include any unreleased migrations/whatever.

composer require "craftcms/commerce:dev-custom-shipping-methods as 4.2.11"

It's not ideal - Commerce still erases most of the information that would be available already with methods like getType, etc, so you'd have to use $option->shippingMethod->getType().

It does proxy the getShippingRule() method so that the snippet mentioned in the first comment in this issue works still.

Related PR: https://github.com/craftcms/commerce/pull/3274

engram-design commented 9 months ago

Thanks for the PR for Commerce! I was keen to get their thoughts on maybe a better approach (as this was working in previous versions of Commerce), but let’s see what they want to do.

johnnynotsolucky commented 9 months ago

@engram-design it’s a quick fix at best, but we have a few dependencies on Postie so the patch at least is necessary.

peteeveleigh commented 9 months ago

@engram-design just to follow up on this, it seems that Postie is working fine with Commerce 4.2.11 and perhaps the patch isn't needed. The reason the shipping options weren't showing was because the packages were falling outside the dimensional/weight limits or didn't have any dimensional data set in Commerce.

Checking in Postie's logs we can see that UPS is returning an error message along those lines. Is there any way to get that error message from Postie and act on it? For instance, we could show a message explaining why no shipping methods are available and offer an alternative option.

engram-design commented 9 months ago

@peteeveleigh Postie should indeed work correctly with Commerce. The original issue was about the shipping rate information not appearing, which is something Commerce needs to fix.

As for being able to act on errors, that's a tricky one as on some occasions, you won't want to show that to a user (the actual error message might be related to your API credentials which you wouldn't want to expose). But that's something on the cards for Postie v4.