verbb / postie

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

A hook to replace box packing class? - Box Packer is timing out #123

Open joshuapease opened 3 months ago

joshuapease commented 3 months ago

What are you trying to do?

Hi there! Thanks for making Postie awesome.

We've got a client who sells super small, lightweight products. It's not unheard of for an order to contain quantities of 50,000 - 100,000 (usually just a handful of line items, but each having really high quantities)

We've found that when line item quantities get large, the box packing algorithm ends up timing out.

They don't use dimensions for their products (just weight), so the box packing seems a little fruitless in this case.

Are there any hooks we could use to bypass the default box packing & instead provide our own box packing logic (in most cases, this would just be a single box with the total weight of the products).

What's your proposed solution?

I could see it being useful to have some hooks or config options for customizing how items are added to the box packer.

It's a bit yucky opening up these internal features, but we've got a very unique Craft Commerce client (with 662,650 products & counting).

Feels like perhaps the cleanest path would be to let folks override the InfalliblePacker class with their own version of the Packer class. I'd love to hear your thoughts on these solutions.

I'm more than happy to make a PR for this if you feel it's worth pursuing.

Additional context

No response

engram-design commented 3 months ago

Interesting, I've not tested things on that scale to fine-tune performance (I'll do that), but you're right that it might be easier to override box packing in some fashion. There are before/after packing events but they're used for modification of things, rather than replacing the packing mechanism.

You could certainly use EVENT_BEFORE_PACK_ORDER to replace the box packer with your own class. That would be my preference, although it's a little more work for developers to handle (creating some packer classes), as opposed to just event callbacks. But I think that's going to be the more rebust approach.

I think being able to modify how things are added to the packer is also an option.

Let me know if using the EVENT_BEFORE_PACK_ORDER is an option?

joshuapease commented 3 months ago

Thanks for the quick reply! I think replacing the box packer will work great in my use case.

EVENT_BEFORE_PACK_ORDER almost does the trick, but I had to make a small modification after the event is handled.

if ($this->hasEventHandlers(self::EVENT_BEFORE_PACK_ORDER)) {
    $this->trigger(self::EVENT_BEFORE_PACK_ORDER, $packOrderEvent);

    // Allow event hander to override $packer
    $packer = $packOrderEvent->packer;
}

I've created a PR with that additional line.

engram-design commented 3 months ago

Oh, great point, thanks!