In https://github.com/openfoodfoundation/openfoodnetwork/pull/3435, it was switched to the system tmp dir because it conventiently provided a method to generate a unique filename. However it didn't handle the case where the system provided a symlink (macOS).
I could have fixed that, but surely it's safer to use the Rails tmp directory.
So I changed back to that, using a tip from https://stackoverflow.com/questions/13787746/creating-a-thread-safe-temporary-file-name to generate a unique name. Perhaps we could use a larger string (eg uuid) or append a timestamp too, but I don't know that it's necessary. Instead, we can just check that the dir didn't exist first (as mentioned in the PR). Let's do that..
The file path was never going to be 'tmp/product_import', so I guess it never deleted the file.
Hmm but shouldn't we clean up on success too? I suppose the tmp dir will be cleaned up eventually, and maybe we want to keep them for debugging purposes.
Due to primary_taxon and supplier having moved to the variant, filtering
by_producer and by_category with a custom order involves some
complicated sql queries. So we moved the sorting logic from
ProductsRenderer to OrderCycles::DistributedProductsService so we can
keep the complicated SQL logic contained in one place
- increments! & decrement! skip validations
- replaced increment! method calls
- one call was for a redefined increment! method
- the other for a regular(ActiveRecord::Persistence)
- removes increments/decrements definition now useless
Creating a variant actually create an extra one via the associated
product, as it will create a "standard variant".
As far as I can see there is no way around it, but it should be fixed
once the Product refactor is finished, and product becomes product
group.
Added a comment on the variant factory to explain the problem.
It's not ideal as it will slow down the test suite a little, but I think
it's better to write the code the way you would expect it, and it will
eventually get fixed.
The original spec check if the supplier and distributor where
updated after deleting product. In reality, the supplier and the
distributor are the same, so no need to test with the supplier
- Apply OR when filtering by both product properties and supplier
properties
- Apply AND when filtering by supplier properties and taxon
Filtering by product properties and taxon is handled by ransack, so
no change there
