refactor: merge the products and product images directories for OFF, OBF, OPF and OPFF #10959
Conversation
github-actions
bot
added
✏️ Editing
API WRITE
|
@stephanegigandet small test conflict |
stephanegigandet
changed the title
|
| if ( (defined $request_body_ref->{product}{code}) | ||
| and ($product_ref->{code} ne $request_body_ref->{product}{code})) |
There was a problem hiding this comment.
I'm a bit disappointed that we have to do those checks here and in product_jqm_multilingual… could'nt we factor this out ?
| autoload("ProductOpener::Config_$flavor"); | ||
|
|
||
| # | ||
| # Add values common to all flavors |
There was a problem hiding this comment.
Thanks for simplifying this !
| @@ -10599,6 +10614,24 @@ sub display_product_api ($request_ref) { | |||
| $response{jqm} .= $html; | |||
| } | |||
| } | |||
| elsif ((defined $product_ref->{product_type}) and ($product_ref->{product_type} ne $options{product_type})) { | |||
There was a problem hiding this comment.
there again we have a complete duplication for product_multilingual.pl, that's a bit frustrating…
| &change_product_code | ||
| &change_product_type |
There was a problem hiding this comment.
That's great we de-entangled both actions !
| $product_ref->{old_code} = $code; | ||
| $code = $new_code; | ||
| $product_ref->{code} = $code; |
There was a problem hiding this comment.
| $product_ref->{old_code} = $code; | |
| $code = $new_code; | |
| $product_ref->{code} = $code; | |
| # we only register the code change, | |
| # file modification will happen as we save the product sto | |
| $product_ref->{old_code} = $code; | |
| $code = $new_code; | |
| $product_ref->{code} = $code; |
| # Update the product_type from the server | ||
| if (defined $options{flavors_product_types}{$new_server}) { | ||
| my $errors_ref = {}; | ||
| change_product_type($product_ref, $options{flavors_product_types}{$new_server}, $errors_ref); |
There was a problem hiding this comment.
I think scripts should call store_product with a try / catch !
Or maybe we could add a parameter to say we wan't a fail-safe call.
| @@ -1303,15 +1362,15 @@ sub store_product ($user_id, $product_ref, $comment) { | |||
| "moving product images", | |||
| { | |||
| source => "$BASE_DIRS{PRODUCTS_IMAGES}/$old_path", | |||
| destination => "$new_www_root/images/products/$path" | |||
| destination => "$www_root/images/products/$path" | |||
There was a problem hiding this comment.
In this whole sections you should not use $www_root and $data_root any more…
Please use %BASE_DIRS values …
|
|
||
| sub move_product_dir_to_off ($dir, $dir2, $dir3, $dir4) { | ||
| # move .sto files | ||
| print STDERR "moving $dir/$dir2/$dir3/$dir4\n"; |
There was a problem hiding this comment.
For me, it's incredible that in this whole function you never put $dir/$dir2/$dir3/$dir4 in a variable to avoid copying it everywhere around !
There was a problem hiding this comment.
It's just a one off script that will never be used again, so I mainly wanted to get this thing done as quickly as possible.
| die; | ||
| } | ||
| } | ||
| die; |
There was a problem hiding this comment.
I launched the script product by product for the first products to verify everything was working as expected as I couldn't replicate everything in the dev environment. I removed the die() in opf etc. after that to do all the products.
In the last few weeks, we normalized product barcodes and deduplicated products that existed in multiple flavors (Open Food Facts, Open Beauty Facts, Open Product Facts and Open Pet Food Facts).
We are now going to merge the directories that contain product and product revision data (the .sto files) and the product images. We will keep separate MongoDB databases for each flavor.
This will:
Deployment plan:
In this PR:
Will solve: