scottmconway / shopgoodwill-scripts

A collection of scripts for programmatically interacting with Shopgoodwill
GNU General Public License v3.0
46 stars 17 forks source link

Investigate results from `get_item_shipping_estimate` #18

Open scottmconway opened 1 year ago

scottmconway commented 1 year ago

Some listings (such as this one) do not provide the HTML prompt to get the shipping/handling estimate, and instead have fixed costs listed. However, you can still call the shipping estimate endpoint and it will return a value that may differ from what the listing page states.

Additionally, the current implementation of get_item_shipping_estimate only returns the shipping cost, and does not take into account the handling cost (which I figure can vary wildly). @RagingRoosevelt do you see any value in retrieving the shipping cost on its own, without the handling cost? If not, I'll change the regex to grab the bold "Total Shipping and Handling" text from the estimate response page.

I'm poking at this so I can add a "max shipping cost" filter to alert_on_new_query_results.py.

RagingRoosevelt commented 1 year ago

The get_item_info endpoint gets handling costs (since it isn't dependent on zip). get_item_info also has shipping costs when they are fixed (eg 1c shipping) or for local-pickup-only. Both 1c and local pickup have boolean flags in get item info as well.

The object keys of interest are

RagingRoosevelt commented 1 year ago

However, you can still call the shipping estimate endpoint and it will return a value that may differ from what the listing page states.

Should the get_item_shipping_estimate method call the get_item_info method to ensure that a shippingPrice isn't already set? From what I've seen of the API, shippingPrice will show 0.0 if it's a calculated shipping and will show a non-zero value if it's a fixed shipping cost.

I was figuring that any time you would call the get_item_shipping_estimate, you would also end up calling the get_item_info method to get the other costs, at which point you'd have handlingPrice

scottmconway commented 1 year ago

To be honest, I did not recall the whole mess of what get_item_info provides. I will now keep examples on-hand.

With that, I think it's fine to have a single method to return the true shipping cost, and perhaps a convenience method to grab the handling cost of an item, fetch the shipping cost to a zip, and return the sum of the two.

I'd like to keep the API-interfacing methods in the sgw object as close to the raw API responses as possible, but I think adding a rail-guard to check the item's listed shippingPrice here wouldn't hurt. It just strikes me as weird that you can get an inaccurate shipping estimate for an item that has a fixed shipping cost.

RagingRoosevelt commented 1 year ago

What do you think about adding a named parameter zip_code (default None) to get_item_info which, if provided, adds the estimated shipping cost to the get_item_info return object? That'd consolidate the apparent calls at collect that info into a more natural place for it. It's really only a bandaid on such a weird API decision, but 🤷.

Maybe that get_item_shipping_estimate would only be called if get_item_info sees a shippingPrice == 0.0 and pickupOnly == False from its normal call?

scottmconway commented 1 year ago

Seems good. The bit about the shipping estimate API endpoint providing false information is still a bit annoying, but I'd rather keep the current method as-is rather than having it take in all of the item info before deciding to make the API call to get the shipping cost.

I'll keep this open for now, until I address the above recommendation.

ngisvold commented 1 year ago

What about using FedEx api for estimate?

RagingRoosevelt commented 1 year ago

What about using FedEx api for estimate?

IMO that seems like an out-of-scope approach for this library. Certainly a user could go that route, however a lot of the goodwill stores ship with other carriers. Given that an order placed for a specific lot will bill you according to what the SGW api and website says the cost will be, it seems prudent to stick with that value.