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

[4.0.3] UPS Rates Different / Service detail pages broken #132

Open
RitterKnightCreative opened this issue Jun 28, 2024 · 8 comments
Open

Comments

@RitterKnightCreative
Copy link

Describe the bug

Working through the process of upgrading Postie from 3 -> 4.

The upgrade itself seemed to work, though I had to put in the carrier (UPS), box details, and credentials all over again. Not sure if that was supposed to taken care of in the migration or not? No big deal either way.

However, I also noticed that when I go into each service (eg Ground), I'm getting a 404 not found error. Maybe a known issue?

More imporantly, I'm getting vastly different rates between versions, as if our clients' negotiated rates aren't showing up. Assuming the box packer itself hasn't been tweaked, I'm not sure what else I'm missing?

Screenshot 2024-06-28 at 3 11 25 PM

Current Prodution (Postie 3)
Screenshot 2024-06-28 at 3 05 23 PM

Current testing (Postie 4)
Screenshot 2024-06-28 at 3 08 15 PM

Steps to reproduce

  1. Upgrade Postie from 3 to 4.
  2. Setup a new carrier (UPS), box sizes, etc.

Craft CMS version

4.10.3

Plugin version

4.0.3

Multi-site?

Yes

Additional context

No response

@engram-design
Copy link
Member

engram-design commented Jun 29, 2024

Ah, sorry about that. I didn't factor in the already-updated OAuth-based provider, just the older non-OAuth one. Updated for the next release.

As for the 404, that's because we switched handles for services back to their numeric ones. Other providers use strings, and that's causing issues for the route matching.

Fixed for the next release. To get this early, run composer require verbb/postie:"dev-craft-4 as 4.0.3".

That's quite interesting! Can you send through the test info you're using (from, to, dimensions/weights). Seems like you're using the front-end to test, so either the from/to address and the contents of your cart, or send through a screenshot of the same info in the "Rates Testing" tab in the control panel for the provider.

I don't think I'll need your connection credentials and account number for negotiated rates yet. You're also welcome to send via [email protected] if you're rather send anything privately.

@RitterKnightCreative
Copy link
Author

Excellent, thank you!

I did a little more digging and I'm wondering if UPS is changing the address classification? I'm getting this returned from UPS:
[Code] => 110920 [Description] => Ship To Address Classification is changed from Commercial to Residential

The weird thing is I'm not getting that with the older version. Wonder if there was any address correction going on? (I'd have to test with a commercial address to see.)

This might also explain why my client had to put a 65% markup on the current site if the rate getting spit out is too low...

RESPONSE.txt
REQUEST.txt

@engram-design
Copy link
Member

That's a little odd, as while we support setting an address as residential it's pretty much always false, so I'm unsure why it's switching that back from Commercial to Residential.

But from a quick Google at other implementations out there, it seems to be common when coming from the XML API to REST API. I did just push a slight change to shippy that was enforcing residential = false that might be something to do with it.

@johnnynotsolucky
Copy link
Contributor

@engram-design #133 should fix the rates issue reported in this issue:

More imporantly, I'm getting vastly different rates between versions, as if our clients' negotiated rates aren't showing up. Assuming the box packer itself hasn't been tweaked, I'm not sure what else I'm missing?

For the alert "Ship To Address Classification is changed from Commercial to Residential", we're seeing this too, but I haven't been able to work out why yet.

According to the API docs, the correct field to set in the payload to UPS is ResidentialAddressIndicator, however, I've tested setting that in the payload too and I'm still seeing the same alerts.
image

After my change in #133, the difference in rates is less significant, instead its a few dollars on each rate that's returned. I suspect that that difference is possibly due to the address classification alert.

@engram-design
Copy link
Member

This might also be fixed in the latest shippy updates, which you can get early, run composer require verbb/postie:"dev-craft-4 as 4.0.3".

@johnnynotsolucky
Copy link
Contributor

@engram-design, even with the change from #133, and the update for the address classification, I'm still seeing the "Ship To Address Classification is changed from Commercial to Residential" message. There is also still a slight difference in the rate compared to the pre-Shippy version of Postie.

I've traced both of these to a difference in how rates are fetched from the pre-Shippy version of Postie.

Previously, the Ship To address didn't include the street lines:

{
	"ShipTo": {
		"Address": {
			"City": "Los Angeles",
			"StateProvinceCode": "CA",
			"PostalCode": "90026",
			"CountryCode": "US"
		}
	}
}

However, the street lines are included when fetching rate information now:

{
	"ShipTo": {
		"Name": "Tyrone",
		"AttentionName": "Tyrone",
		"Address": {
			"AddressLine": [
				"1514 Maltman Ave"
			],
			"City": "Los Angeles",
			"StateProvinceCode": "CA",
			"PostalCode": "90026",
			"CountryCode": "US"
		},
		"EMailAddress": "[email protected]"
	}
}

If I remove the AddressLine property, then the rates are as they were before.

It seems like the rates are then more accurate since the actual address is included, not just the area information which is why there is a few dollars difference in rates now. But I can't work out why the alert is showing up when the address lines are included.

In the above example, the rate for UPS Ground without address lines is $16.90, and with address lines, it is $19.10. That looks like it should be correct to me.

@engram-design
Copy link
Member

Thanks for investigating. I suppose we possibly should've always used the full address where provided for more accurate rates in the first place. While new rates are higher, this seems more accurate? We certainly need them accurate now in order to generate shipments and labels.

I would have thought omitting a reference to an address being residential would mean UPS set it to "auto", but that same warning keeps cropping up. I'm not sure if it's something that we can ignore or not?

@johnnynotsolucky
Copy link
Contributor

I would have thought omitting a reference to an address being residential would mean UPS set it to "auto", but that same warning keeps cropping up. I'm not sure if it's something that we can ignore or not?

Also not sure at the moment 😅

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

No branches or pull requests

3 participants