I was hoping to reduce the query count but it stayed the same. In fact,
I'm expecting the query count to be higher with this version. The
DfcCatalogImporter queries all variants and links at once while the OC
job is not pre-loading the variants at the moment. We can optimise the
job though.
If we kept the old version and there were multiple catalogs per variant
then we would call the importer with the reset multiple times. The job
is iterating through each link only once though. So depending on the
ratio of catalogs to variants, I'm not sure which version would be more
efficient.
No version is properly dealing with the edge-case of multiple catalogs
per variant anyway. Imagine there are two catalogs with different stock
levels. Which one do we choose? Or do we add up?
And if the variant disappears from one catalog we still want to sell it
through the other. But depending on the order of processing, we may
reset the variant if it's missing in the last catalog. But let's worry
about that when it actually happens. Maybe it will be better to restrict
variants to one catalog.
When retail variants are mapped to wholesale variants, we usually have a
some leftover stock at the end of an order cycle. For example, we
backordered a slab of 12 cans of tomatoes but our customers bought only
9 of those. Then we have 3 left for the next order cycle.
Even when the product is not available for backorder with the supplier,
we still want to sell off our leftover stock, the three cans of tomatoes
in our example.
And it might be that the product will come back in the future.
The previous version wasn't testing anything as there was no fees set
up.
Now we check that fees are applied as expected, and also that supplier
fees are applied only to the expected product.
This name better reflects what it's doing.
As this job is scheduled automatically by Sidekiq, I think there shouldn't be any jobs with the old name in redis. So I didn't bother keeping a placholder for the old name.
And Clean up unused include
After 10 minutes, I'd consider that it failed to open the order cycle. Who would want their products to sync, or get a notification at a random time during the order cycle?
Best viewed with whitespace ignored.
Being based on the DB value should be more robust.
This prevents an order cycle from being "opened" when it's already open. But note that the order cycle can become "unopened" (see OrderCycle#reset_opened_at).
Nice to see the database query count drop, but I must confess I don't know why!
There might be a delay before it gets sent, so it's better to record the time the event occurred at.
It would have been simpler to just add it to the data hash, but I felt it was an important detail for an event and should be at the top level along with event name.
In the case of order cycle opening, this is the same as opened_at. I've included this in the payload for clarity too.
It was probably better to be explicit at each test, but this one is always repeated and approaches the line length, so I wanted to just define it once at the top.
