In D7, most (if not all) shipping modules had a `hook_rates()` to allow modifying the rates for various reasons.

CommentFileSizeAuthor
#14 2991416=14-rates-event.patch12.32 KBbojanz
#8 commerce_shipping-dispatch-event-2991416-8.patch6.27 KBsah62
#7 interdiff-2-7.txt2.76 KBmitrpaka
#7 commerce_shipping-dispatch-event-2991416-7.patch6.15 KBmitrpaka
#5 commerce_shipping-dispatch-event-2991416-5.patch6.19 KBkrystalcode
#2 commerce_shipping-dispatch-event-2991416-2.patch5.2 KBthejacer87
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

thejacer87 created an issue. See original summary.

thejacer87’s picture

thejacer87 CreditAttribution: thejacer87 at Acro Commerce commented
Status: Active » Needs review
FileSize
commerce_shipping-dispatch-event-2991416-2.patch5.2 KB
krystalcode’s picture

krystalcode CreditAttribution: krystalcode at Acro Commerce commented
Status: Needs review » Needs work 
$shipping_rates = $shipping_method_plugin->calculateRates($shipment);
$event = new ShippingRatesPostCalculateEvent($shipping_rates);
$this->eventDispatcher->dispatch(ShippingEvents::SHIPPING_RATES_POST_CALCULATE, $event);
$shipping_rates = $event->getRates();

`$shipping_rates` is an array of `ShippingRate` objects, right? You shouldn't need to get them again from the event (`$shipping_rates = $event->getRates()` as objects are always passed by reference, including in arrays. You can remove that line, just test it works as expected after removing it.

thejacer87’s picture

thejacer87 CreditAttribution: thejacer87 at Acro Commerce commented

ya for w/e reason, it won't work like you expect

i looked around and saw other modules using the same pattern:

TaxTypeBase.php 

// A shipping profile is preferred, when available.
$event = new CustomerProfileEvent($customer_profile, $order_item);
$this->eventDispatcher->dispatch(TaxEvents::CUSTOMER_PROFILE, $event);
$customer_profile = $event->getCustomerProfile();

ProductVariationStorage.php

// Allow modules to apply own filtering (based on date, stock, etc).
$event = new FilterVariationsEvent($product, $enabled_variations);
$this->eventDispatcher->dispatch(ProductEvents::FILTER_VARIATIONS, $event);
$enabled_variations = $event->getVariations();

PluginItemDeriver

    // Core has no way to list plugin types, so each referenceable plugin
    // type needs to register itself via the event.
    $event = new ReferenceablePluginTypesEvent($plugin_types);
    $this->eventDispatcher->dispatch(CommerceEvents::REFERENCEABLE_PLUGIN_TYPES, $event);
    $plugin_types = $event->getPluginTypes();
krystalcode’s picture

krystalcode CreditAttribution: krystalcode at Acro Commerce commented
Status: Needs work » Needs review
FileSize
commerce_shipping-dispatch-event-2991416-5.patch6.19 KB

- Provided the shipping method to the event as well as the shipping rates do not provide any context.
- Improved docblocks.

Status: Needs review » Needs work

The last submitted patch, 5: commerce_shipping-dispatch-event-2991416-5.patch, failed testing. View results

mitrpaka’s picture

mitrpaka CreditAttribution: mitrpaka as a volunteer commented
Status: Needs work » Needs review
FileSize
commerce_shipping-dispatch-event-2991416-7.patch6.15 KB
interdiff-2-7.txt2.76 KB

Re-roll of previous patch file.

sah62’s picture

sah62 CreditAttribution: sah62 commented
FileSize
commerce_shipping-dispatch-event-2991416-8.patch6.27 KB

Reroll...

sah62’s picture

sah62 CreditAttribution: sah62 commented

I've implemented the latest patch on my test site and can confirm that it seems to be working fine. I can successfully subscribe to events and be notified when they fire. Can we get this committed and released?

Niklan’s picture

Niklan
Primary language Russian
Location Russia, Perm
CreditAttribution: Niklan as a volunteer commented
Status: Needs review » Reviewed & tested by the community

Tested it today. Everything works as expected. Thank you for patch!

jsacksick’s picture

jsacksick CreditAttribution: jsacksick at Centarro commented

An event is also being added as part of the patch from #3098140: Simplify calculating rates for a shipment.
This also adds a new service which has a method for calculating rates, so it makes more sense to have the event fired there (instead of the field widget).

bojanz credited jsacksick.

bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented
Title: Dispatch Event to allow modules to change the calculated shipping rates » Add an event for altering the calculated shipping rates
Status: Reviewed & tested by the community » Needs work

Retitling.

Patch doesn't apply to -dev.
I'll extract jsacksick's improvements from #3098140: Simplify calculating rates for a shipment back into a new patch.

bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented
Status: Needs work » Needs review
FileSize
2991416=14-rates-event.patch12.32 KB

Here's a rerolled patch.

The SHIPPING_RATES_POST_CALCULATE event has been renamed to SHIPPING_RATES, since that matches D7 and is shorter. We now $rate->fromArray() and $rate->toArray() methods to simplify altering, and we have test coverage that serves as an example.

  • bojanz committed f6985b1 on 8.x-2.x authored by thejacer87 
    Issue #2991416 by mitrpaka, thejacer87, krystalcode, sah62, bojanz,...
bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented
Status: Needs review » Fixed

We have random fails in our functional tests. I'll fix that in another issue.

Thanks, everyone!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.