Problem/Motivation

I originally thought this was similar to #3207548: Recalculate shipping methods when manually creating a shipment in the backend, but that's a feature request. Shipping recalculation on the admin shipment form does not take the currently selected shipping profile and shipping rates aren't valid or returned at all.

Steps to reproduce

Edit an order for a customer that has multiple shipping profiles. Attempt to select a different profile and recalculate shipping. When the shipping rates plugin gets the shipment object, the profile value is empty or incorrect.

Proposed resolution

Review the patch and suggest how this should be handled.

CommentFileSizeAuthor
#6 interdiff_5-6.txt630 bytesjsacksick
#6 3242737-6.patch4.5 KBjsacksick
#5 interdiff_4-5.txt690 bytesjsacksick
#5 3242737-5.patch4.46 KBjsacksick
#4 3242737-4.patch4.4 KBjsacksick
commerce_shipping-admin_shipment_recalc-3207548.patch3.25 KBandyg5000
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

andyg5000 created an issue. See original summary.

andyg5000’s picture

andyg5000
he/him
Primary language English
Location North Carolina, USA
CreditAttribution: andyg5000 commented

I'm also noticing that with the patch above that fixes my issue... When I click recalculate multiple times, the profile id is removed from the widget and replaced with _original. The widget then defaults back to the first profile since the selected profile id is no longer an option.

SocialNicheGuru’s picture

SocialNicheGuru CreditAttribution: SocialNicheGuru commented
Status: Active » Needs review
jsacksick’s picture

jsacksick CreditAttribution: jsacksick at Centarro commented
FileSize
3242737-4.patch4.4 KB

Seems to work much better with the attached patch!

It actually fixes several bugs, manage to get an "Illegal choice detected" error when entering a new address, and then selecting a different profile.

This kind of shows that we have insufficient tests coverage.

But, before the pach, wasn't getting updated rates, now I'm am. Also backported a fix that was applied to the shipping info pane. (Selecting a different address now refreshes the rate without having to click on "Recalculate shipping", this is probably related to #3207548: Recalculate shipping methods when manually creating a shipment in the backend.

jsacksick’s picture

jsacksick CreditAttribution: jsacksick at Centarro commented
FileSize
3242737-5.patch4.46 KB
interdiff_4-5.txt690 bytes

Ok, after switching profiles back and forth, switching between a profile and entering a new address, realized the rates sticked when they shouldn't.

That is because, we don't update the address if it's NULL, but I think we should.

jsacksick’s picture

jsacksick CreditAttribution: jsacksick at Centarro commented
FileSize
3242737-6.patch4.5 KB
interdiff_5-6.txt630 bytes

The patch from #5 introduced a regression (when the "_original" option is selected), the attached patch is fixing that while preserving the fix introduced in #5 as well.

The last submitted patch, 5: 3242737-5.patch, failed testing. View results

jsacksick’s picture

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

Went ahead and committed the fix.

  • jsacksick committed 3f492dc on 8.x-2.x 
    Issue #3242737 by jsacksick, andyg5000: Shipping profile element doesn't...
jsacksick’s picture

jsacksick CreditAttribution: jsacksick at Centarro commented

It's still not "perfect" as filling all the required fields of an address doesn't trigger a recalculation, but that should happen in a followup.

Status: Fixed » Closed (fixed)

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