verbb / postie

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

Added UPS Declared Value #116

Closed bryanredeagle closed 9 months ago

bryanredeagle commented 9 months ago

I have a client that ships a lot of expensive products. They declare the value when they ship and asked if they could have the same for their website.

I didn't see anywhere where the items in each box were totaled, and it didn't seem that the box packer had a hook to include that. So I edited the packed box list that was being sent to the UPS provider.

I also added a toggle to activate it, and made it disabled by default.

engram-design commented 9 months ago

We already set the itemValue for a box item when packing which is populated from the lineItem->price.

As such, you should already have access to it in the provider class. You can see how we deal with insurance in the same manner.

You're also undoing this behaviour here

bryanredeagle commented 9 months ago

I'm aware of itemValue. That's why I use it to find the value of a single box.

Unless I'm mistaken, I'm reading $packedBoxes->getTotalPrice() as the total price for all the items in all the boxes. UPS expects each box to have the value declared for only the items in that box.

I could not find where $packedBox->getBox()->getPrice() had any item prices added to it. The Box class and object have variables and functions to set/get the total price of items in the box, but nowhere can I find any prices being totaled. When I initially used it to set the declared value, the logs kept telling me the box price was zero.

engram-design commented 9 months ago

Ah, no you make a great point. The box needs to have its price set after the fact when the boxes have been packed with items. At that point, we created PackedBoxes. As such, I'd like to move this price population to the constructor for the moment, which I've done.

This will change your PR to not needing the change in src/models/PackedBoxes.php but the rest should be fine.

If you can adjust that, happy to merge!

bryanredeagle commented 9 months ago

I don't do a lot of fancy stuff in git, so it took me a moment to back out all the changes and just keep the UPS stuff. But it should be good to go.

Thanks!

engram-design commented 9 months ago

Thanks!