diff --git a/cgi/product_jqm_multilingual.pl b/cgi/product_jqm_multilingual.pl index dab593dcb7748..7ce034b668e3a 100755 --- a/cgi/product_jqm_multilingual.pl +++ b/cgi/product_jqm_multilingual.pl @@ -57,7 +57,8 @@ =head1 DESCRIPTION use ProductOpener::ForestFootprint qw/:all/; use ProductOpener::Text qw/remove_tags_and_quote/; use ProductOpener::API qw/get_initialized_response check_user_permission/; -use ProductOpener::APIProductWrite qw/skip_protected_field/; +use ProductOpener::APIProductWrite + qw/process_change_product_type_request_if_we_have_one process_change_product_code_request_if_we_have_one skip_protected_field/; use Apache2::RequestRec (); use Apache2::Const (); @@ -230,30 +231,14 @@ =head1 DESCRIPTION # Change code or product type - if (defined single_param('new_code')) { + push @errors, + process_change_product_code_request_if_we_have_one($request_ref, $response_ref, $product_ref, + single_param("new_code")); + $code = $product_ref->{code}; - if (check_user_permission($request_ref, $response_ref, "product_change_code")) { - - push @errors, change_product_code($product_ref, single_param('new_code')); - $code = $product_ref->{code}; - } - else { - push @errors, "No permission: product_change_code"; - } - } - - if ( (defined single_param("product_type")) - and ($product_ref->{product_type} ne single_param("product_type"))) - { - - if (check_user_permission($request_ref, $response_ref, "product_change_product_type")) { - - push @errors, change_product_type($product_ref, single_param("product_type")); - } - else { - push @errors, "No permission: product_change_product_type"; - } - } + push @errors, + process_change_product_type_request_if_we_have_one($request_ref, $response_ref, $product_ref, + single_param("product_type")); # Display an error message and exit if we have a fatal error (no permission to change barcode or product type, or invalid barcode or product type) if ($#errors >= 0) { diff --git a/cgi/product_multilingual.pl b/cgi/product_multilingual.pl index 0c6bd41604ef8..d53dcbbebf5e1 100755 --- a/cgi/product_multilingual.pl +++ b/cgi/product_multilingual.pl @@ -56,6 +56,8 @@ use ProductOpener::APIProductWrite qw/skip_protected_field/; use ProductOpener::ProductsFeatures qw/feature_enabled/; use ProductOpener::Orgs qw/update_import_date update_last_import_type/; +use ProductOpener::APIProductWrite + qw/process_change_product_type_request_if_we_have_one process_change_product_code_request_if_we_have_one/; use Apache2::RequestRec (); use Apache2::Const (); @@ -408,18 +410,14 @@ ($product_ref) exists $product_ref->{new_server} and delete $product_ref->{new_server}; - if ($request_ref->{admin} or $User{moderator}) { - if ((defined single_param("new_code")) and (single_param("new_code") ne "")) { - change_product_code($product_ref, single_param("new_code")); - $code = $product_ref->{code}; - } - if ( (defined single_param("product_type")) - and (single_param("product_type") ne "") - and ($product_ref->{product_type} ne single_param("product_type"))) - { - change_product_type($product_ref, single_param("product_type")); - } - } + # Check if the request is for changing the product code or the product type, if so process it + + process_change_product_code_request_if_we_have_one($request_ref, $response_ref, $product_ref, + single_param("new_code")); + $code = $product_ref->{code}; + + process_change_product_type_request_if_we_have_one($request_ref, $response_ref, $product_ref, + single_param("product_type")); my @param_fields = (); diff --git a/lib/ProductOpener/APIProductWrite.pm b/lib/ProductOpener/APIProductWrite.pm index d4a3cc996f458..431e4e16bcd94 100644 --- a/lib/ProductOpener/APIProductWrite.pm +++ b/lib/ProductOpener/APIProductWrite.pm @@ -37,6 +37,8 @@ BEGIN { use vars qw(@ISA @EXPORT_OK %EXPORT_TAGS); @EXPORT_OK = qw( &write_product_api + &process_change_product_code_request_if_we_have_one + &process_change_product_type_request_if_we_have_one &skip_protected_field ); # symbols to export on request %EXPORT_TAGS = (all => [@EXPORT_OK]); @@ -361,6 +363,113 @@ sub update_product_fields ($request_ref, $product_ref, $response_ref) { return; } +=head2 process_change_product_code_request_if_we_have_one($request_ref, $response_ref, $product_ref, $new_code) + +Process a change of code request if we have one. + +=head3 Parameters + +=head4 $request_ref (input) + +Reference to the request object. + +=head4 $response_ref (input) + +Reference to the response object. + +=head4 $product_ref (input) + +Reference to the product object. + +=head4 $new_code (input) + +New code. + +=head3 Return value + +undef if we don't have a change product code request, or if it was processed correctly. +an error id if there was an error (e.g. no_permisssion or invalid_product_type). + +=cut + +sub process_change_product_code_request_if_we_have_one($request_ref, $response_ref, $product_ref, $new_code) { + + my $error; + # Change of code + if ( (defined $new_code) + and ($new_code ne $product_ref->{code})) + { + + if (check_user_permission($request_ref, $response_ref, "product_change_code")) { + + $error = change_product_code($product_ref, $new_code); + if ($error) { + add_error( + $response_ref, + { + message => {id => $error}, + field => {id => "new_code"}, + impact => {id => "failure"}, + } + ); + } + } + else { + $error = "no_permission: product_change_code"; + } + } + # If we have an error, we return it, otherwise we just use "return;" + # so that the function can be used in list context: push @errors, process_change_product_code_request_if_we_have_one(...) + if ($error) { + return $error; + } + return; +} + +sub process_change_product_type_request_if_we_have_one($request_ref, $response_ref, $product_ref, $new_product_type) { + + my $error; + + # Change of product type + if ( + (defined $new_product_type) + and ($new_product_type ne "") + and ($new_product_type ne + "null") # 2024/11/21: OFF app sends "null" as a string, ignore it as it is not a valid product type + and ($new_product_type ne $product_ref->{product_type}) + ) + { + + if (check_user_permission($request_ref, $response_ref, "product_change_product_type")) { + + $error = change_product_type($product_ref, $new_product_type); + if ($error) { + add_error( + $response_ref, + { + message => {id => $error}, + field => {id => "product_type"}, + impact => {id => "failure"}, + } + ); + } + else { + $request_ref->{updated_product_fields}{product_type} = 1; + } + } + else { + $error = "no_permission: product_change_product_type"; + } + } + + # If we have an error, we return it, otherwise we just use "return;" + # so that the function can be used in list context: push @errors, process_change_product_code_request_if_we_have_one(...) + if ($error) { + return $error; + } + return; +} + =head2 write_product_api($request_ref) Process API v3 WRITE product requests. @@ -483,62 +592,22 @@ sub write_product_api ($request_ref) { } else { - # Change of code - if ( (defined $request_body_ref->{product}{code}) - and ($product_ref->{code} ne $request_body_ref->{product}{code})) + if ( + process_change_product_code_request_if_we_have_one( + $request_ref, $response_ref, $product_ref, $request_body_ref->{product}{code} + ) + ) { - - if (check_user_permission($request_ref, $response_ref, "product_change_code")) { - - my $change_product_code_error - = change_product_code($product_ref, $request_body_ref->{product}{code}); - if ($change_product_code_error) { - add_error( - $response_ref, - { - message => {id => $error}, - field => {id => "new_code"}, - impact => {id => "failure"}, - } - ); - $error = 1; - } - else { - $code = $product_ref->{code}; - } - } - else { - $error = 1; - } + $error = 1; } - # Change of product type - if ( (defined $request_body_ref->{product}{product_type}) - and ($product_ref->{product_type} ne $request_body_ref->{product}{product_type})) + if ( + process_change_product_type_request_if_we_have_one( + $request_ref, $response_ref, $product_ref, $request_body_ref->{product}{product_type} + ) + ) { - - if (check_user_permission($request_ref, $response_ref, "product_change_product_type")) { - - my $change_product_type_error - = change_product_type($product_ref, $request_body_ref->{product}{product_type}); - if ($change_product_type_error) { - add_error( - $response_ref, - { - message => {id => $error}, - field => {id => "product_type"}, - impact => {id => "failure"}, - } - ); - $error = 1; - } - else { - $request_ref->{updated_product_fields}{product_type} = 1; - } - } - else { - $error = 1; - } + $error = 1; } if (not $error) { diff --git a/tests/integration/change_product_code_and_product_type.t b/tests/integration/change_product_code_and_product_type.t index 32ba310c46380..92e460ec3fd23 100644 --- a/tests/integration/change_product_code_and_product_type.t +++ b/tests/integration/change_product_code_and_product_type.t @@ -220,6 +220,29 @@ my $tests_ref = [ }, ua => $moderator_ua, }, + # Send null product type + # 2024/11/21: the OFF app is sending product_type=null for new products + { + test_case => 'change-product-type-to-null-api-v2-moderator', + method => 'POST', + path => '/cgi/product_jqm_multilingual.pl', + form => { + code => "1234567890102", + product_type => "null", + }, + ua => $moderator_ua, + }, + # Send empty string product type + { + test_case => 'change-product-type-to-empty-string-api-v2-moderator', + method => 'POST', + path => '/cgi/product_jqm_multilingual.pl', + form => { + code => "1234567890102", + product_type => "", + }, + ua => $moderator_ua, + }, # Send product_type=beauty to move product to Open Beauty Facts { test_case => 'change-product-type-to-beauty-api-v2-moderator', @@ -390,7 +413,7 @@ my $tests_ref = [ }', ua => $moderator_ua, }, - # Change the product_type field to petfood, with API v3 + # Change the product_type field to invalid product type, with API v3 { test_case => 'change-product-type-to-opff-api-v3-invalid-product-type', method => 'PATCH', diff --git a/tests/integration/expected_test_results/change_product_code_and_product_type/change-product-code-api-v2-normal-account.json b/tests/integration/expected_test_results/change_product_code_and_product_type/change-product-code-api-v2-normal-account.json index e4c8b4b86ab32..f29b47ef0d11b 100644 --- a/tests/integration/expected_test_results/change_product_code_and_product_type/change-product-code-api-v2-normal-account.json +++ b/tests/integration/expected_test_results/change_product_code_and_product_type/change-product-code-api-v2-normal-account.json @@ -1,4 +1,4 @@ { "status" : 0, - "status_verbose" : "No permission: product_change_code" + "status_verbose" : "no_permission: product_change_code" } diff --git a/tests/integration/expected_test_results/change_product_code_and_product_type/change-product-code-api-v2-not-logged-in.json b/tests/integration/expected_test_results/change_product_code_and_product_type/change-product-code-api-v2-not-logged-in.json index e4c8b4b86ab32..f29b47ef0d11b 100644 --- a/tests/integration/expected_test_results/change_product_code_and_product_type/change-product-code-api-v2-not-logged-in.json +++ b/tests/integration/expected_test_results/change_product_code_and_product_type/change-product-code-api-v2-not-logged-in.json @@ -1,4 +1,4 @@ { "status" : 0, - "status_verbose" : "No permission: product_change_code" + "status_verbose" : "no_permission: product_change_code" } diff --git a/tests/integration/expected_test_results/change_product_code_and_product_type/change-product-code-api-v3-moderator-existing-code.json b/tests/integration/expected_test_results/change_product_code_and_product_type/change-product-code-api-v3-moderator-existing-code.json index e57132fc05a5e..7b8573a4753a4 100644 --- a/tests/integration/expected_test_results/change_product_code_and_product_type/change-product-code-api-v3-moderator-existing-code.json +++ b/tests/integration/expected_test_results/change_product_code_and_product_type/change-product-code-api-v3-moderator-existing-code.json @@ -11,7 +11,7 @@ "name" : "Failure" }, "message" : { - "id" : 0, + "id" : "error_new_code_already_exists", "lc_name" : "", "name" : "" } diff --git a/tests/integration/expected_test_results/change_product_code_and_product_type/change-product-code-api-v3-moderator-invalid-code.json b/tests/integration/expected_test_results/change_product_code_and_product_type/change-product-code-api-v3-moderator-invalid-code.json index e57132fc05a5e..f7574ef3d6563 100644 --- a/tests/integration/expected_test_results/change_product_code_and_product_type/change-product-code-api-v3-moderator-invalid-code.json +++ b/tests/integration/expected_test_results/change_product_code_and_product_type/change-product-code-api-v3-moderator-invalid-code.json @@ -11,9 +11,9 @@ "name" : "Failure" }, "message" : { - "id" : 0, - "lc_name" : "", - "name" : "" + "id" : "invalid_code", + "lc_name" : "Invalid code", + "name" : "Invalid code" } } ], diff --git a/tests/integration/expected_test_results/change_product_code_and_product_type/change-product-type-to-beauty-api-v2-normal-account.json b/tests/integration/expected_test_results/change_product_code_and_product_type/change-product-type-to-beauty-api-v2-normal-account.json index ae0b2b080b3ec..b9086608fe023 100644 --- a/tests/integration/expected_test_results/change_product_code_and_product_type/change-product-type-to-beauty-api-v2-normal-account.json +++ b/tests/integration/expected_test_results/change_product_code_and_product_type/change-product-type-to-beauty-api-v2-normal-account.json @@ -1,4 +1,4 @@ { "status" : 0, - "status_verbose" : "No permission: product_change_product_type" + "status_verbose" : "no_permission: product_change_product_type" } diff --git a/tests/integration/expected_test_results/change_product_code_and_product_type/change-product-type-to-empty-string-api-v2-moderator.json b/tests/integration/expected_test_results/change_product_code_and_product_type/change-product-type-to-empty-string-api-v2-moderator.json new file mode 100644 index 0000000000000..39d4261a9d3f0 --- /dev/null +++ b/tests/integration/expected_test_results/change_product_code_and_product_type/change-product-type-to-empty-string-api-v2-moderator.json @@ -0,0 +1,4 @@ +{ + "status" : 1, + "status_verbose" : "fields saved" +} diff --git a/tests/integration/expected_test_results/change_product_code_and_product_type/change-product-type-to-null-api-v2-moderator.json b/tests/integration/expected_test_results/change_product_code_and_product_type/change-product-type-to-null-api-v2-moderator.json new file mode 100644 index 0000000000000..39d4261a9d3f0 --- /dev/null +++ b/tests/integration/expected_test_results/change_product_code_and_product_type/change-product-type-to-null-api-v2-moderator.json @@ -0,0 +1,4 @@ +{ + "status" : 1, + "status_verbose" : "fields saved" +} diff --git a/tests/integration/expected_test_results/change_product_code_and_product_type/change-product-type-to-opff-api-v3-invalid-product-type.json b/tests/integration/expected_test_results/change_product_code_and_product_type/change-product-type-to-opff-api-v3-invalid-product-type.json index 5739720c6a25c..223ca6a8194a0 100644 --- a/tests/integration/expected_test_results/change_product_code_and_product_type/change-product-type-to-opff-api-v3-invalid-product-type.json +++ b/tests/integration/expected_test_results/change_product_code_and_product_type/change-product-type-to-opff-api-v3-invalid-product-type.json @@ -11,9 +11,9 @@ "name" : "Failure" }, "message" : { - "id" : 0, - "lc_name" : "", - "name" : "" + "id" : "invalid_product_type", + "lc_name" : "Invalid product type", + "name" : "Invalid product type" } } ],