fix: facets for EU packager codes (EC) and for users (#9380)

- Fixes the normalization of the EC packager codes - Also fixes #9372 --------- Co-authored-by: Pierre Slamich <[email protected]> Co-authored-by: Alex Garel <[email protected]>
openfoodfacts · Nov 28, 2023 · f8584ad · f8584ad
1 parent 1244f90
commit f8584ad
Show file tree

Hide file tree

Showing 11 changed files with 137 additions and 22 deletions.
diff --git a/.github/labeler.yml b/.github/labeler.yml
@@ -413,6 +413,8 @@ Numbers:
 - scripts/test_normalize_packaging_codes.pl
 - scripts/update_emb_codes_in_mongodb.pl
 - packager-codes/**
+- tests/integration/expected_test_results/facets/packager-code_fr-85-222-003-ec.json
+- tests/integration/expected_test_results/facets/packager-code_fr-85-222-003-ce.json
 
 📦 Packaging:
 - lib/ProductOpener/Packaging.pm

diff --git a/lib/ProductOpener/Display.pm b/lib/ProductOpener/Display.pm
@@ -4000,7 +4000,7 @@ HTML
 			if (defined $user_or_org_ref) {
 
 				if ($user_or_org_ref->{name} ne '') {
-					$title = $user_or_org_ref->{name} || $tagid;
+					$title = $user_or_org_ref->{name};
 					$display_tag = $user_or_org_ref->{name};
 				}
 
@@ -4170,7 +4170,7 @@ HTML
 		else {
 			my $field_name = $tag_ref->{tagtype} . "_tags";
 			my $current_value = param($field_name);
-			my $new_value = ($tag_ref->{tag_prefix} || '') . ($tag_ref->{canon_tagid} || $tag_ref->{tagid});
+			my $new_value = ($tag_ref->{tag_prefix} // '') . ($tag_ref->{canon_tagid} // $tag_ref->{tagid});
 			if ($current_value) {
 				$new_value = $current_value . ',' . $new_value;
 			}
@@ -4529,10 +4529,12 @@ my %ignore_params = (
 	no_count => 1,
 );
 
-# Parameters that can be query filters
+# Parameters that can be query filters passed as parameters
+# (GET query parameters, POST JSON body or from url facets),
+# in addition to tags fields.
 # It is safer to use a positive list, instead of just the %ignore_params list
 
-my %valid_params = (code => 1,);
+my %valid_params = (code => 1, creator => 1);
 
 sub add_params_to_query ($request_ref, $query_ref) {
 
@@ -4616,6 +4618,10 @@ sub add_params_to_query ($request_ref, $query_ref) {
 						}
 						else {
 							$tagid2 = get_string_id_for_lang("no_language", canonicalize_tag2($tagtype, $tag2));
+							# EU packager codes are normalized to have -ec at the end
+							if ($tagtype eq 'emb_codes') {
+								$tagid2 =~ s/-($ec_code_regexp)$/-ec/ie;
+							}
 						}
 						push @tagids, $tagid2;
 					}
@@ -4643,6 +4649,10 @@ sub add_params_to_query ($request_ref, $query_ref) {
 					}
 					else {
 						$tagid = get_string_id_for_lang("no_language", canonicalize_tag2($tagtype, $tag));
+						# EU packager codes are normalized to have -ec at the end
+						if ($tagtype eq 'emb_codes') {
+							$tagid =~ s/-($ec_code_regexp)$/-ec/ie;
+						}
 					}
 					$log->debug("add_params_to_query - tags param - single value",
 						{field => $field, lc => $lc, tag_lc => $tag_lc, tag => $tag, tagid => $tagid})
@@ -4661,6 +4671,7 @@ sub add_params_to_query ($request_ref, $query_ref) {
 							)
 						)
 						and ($tagtype !~ /^pnns_groups_/)
+						and ($tagtype ne "creator")
 						)
 					{
 						if ($not) {
@@ -7093,8 +7104,8 @@ sub display_page ($request_ref) {
 	$template_data_ref->{formatted_subdomain} = $formatted_subdomain;
 	$template_data_ref->{css_timestamp} = $file_timestamps{'css/dist/app-' . lang('text_direction') . '.css'};
 	$template_data_ref->{header} = $header;
-	$template_data_ref->{page_type} = $request_ref->{page_type} || "other";
-	$template_data_ref->{page_format} = $request_ref->{page_format} || "normal";
+	$template_data_ref->{page_type} = $request_ref->{page_type} // "other";
+	$template_data_ref->{page_format} = $request_ref->{page_format} // "normal";
 
 	if ($request_ref->{schema_org_itemtype}) {
 		$template_data_ref->{schema_org_itemtype} = $request_ref->{schema_org_itemtype};
@@ -7272,7 +7283,7 @@ sub display_page ($request_ref) {
 	# Replace urls for texts in links like <a href="/ecoscore"> with a localized name
 	$html =~ s/(href=")(\/[^"]+)/$1 . url_for_text($2)/eg;
 
-	my $status_code = $request_ref->{status_code} || 200;
+	my $status_code = $request_ref->{status_code} // 200;
 
 	my $http_headers_ref = {
 		'-status' => $status_code,
@@ -10532,7 +10543,7 @@ sub display_structured_response ($request_ref) {
 			= "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?>\n"
 			. $xs->XMLout($request_ref->{structured_response});  # noattr -> force nested elements instead of attributes
 
-		my $status_code = $request_ref->{status_code} || "200";
+		my $status_code = $request_ref->{status_code} // "200";
 		write_cors_headers();
 		print header(
 			-status => $status_code,
@@ -10559,7 +10570,7 @@ sub display_structured_response ($request_ref) {
 			$jsonp = single_param('callback');
 		}
 
-		my $status_code = $request_ref->{status_code} || 200;
+		my $status_code = $request_ref->{status_code} // 200;
 
 		if (defined $jsonp) {
 			$jsonp =~ s/[^a-zA-Z0-9_]//g;

diff --git a/tests/integration/expected_test_results/facets/brand_-brand1.json b/tests/integration/expected_test_results/facets/brand_-brand1.json
@@ -1,7 +1,7 @@
 {
-   "count" : 7,
+   "count" : 8,
    "page" : 1,
-   "page_count" : 7,
+   "page_count" : 8,
    "page_size" : 24,
    "products" : [
       {
@@ -24,6 +24,9 @@
       },
       {
          "product_name" : "Oranges - Fair trade - Italy - brand3"
+      },
+      {
+         "product_name" : "Yogurt - Organic, Fair trade - Martinique"
       }
    ],
    "skip" : 0

diff --git a/tests/integration/expected_test_results/facets/brand_unknown.json b/tests/integration/expected_test_results/facets/brand_unknown.json
@@ -1,7 +1,7 @@
 {
-   "count" : 4,
+   "count" : 5,
    "page" : 1,
-   "page_count" : 4,
+   "page_count" : 5,
    "page_size" : 24,
    "products" : [
       {
@@ -15,6 +15,9 @@
       },
       {
          "product_name" : "Chocolate - Organic, Fair trade - Martinique"
+      },
+      {
+         "product_name" : "Yogurt - Organic, Fair trade - Martinique"
       }
    ],
    "skip" : 0

diff --git a/tests/integration/expected_test_results/facets/category_-apples.json b/tests/integration/expected_test_results/facets/category_-apples.json
@@ -1,7 +1,7 @@
 {
-   "count" : 9,
+   "count" : 10,
    "page" : 1,
-   "page_count" : 9,
+   "page_count" : 10,
    "page_size" : 24,
    "products" : [
       {
@@ -30,6 +30,9 @@
       },
       {
          "product_name" : "Oranges - Organic - Spain - brand1, brand2, brand3"
+      },
+      {
+         "product_name" : "Yogurt - Organic, Fair trade - Martinique"
       }
    ],
    "skip" : 0

diff --git a/tests/integration/expected_test_results/facets/category_-apples_label_organic.json b/tests/integration/expected_test_results/facets/category_-apples_label_organic.json
@@ -1,7 +1,7 @@
 {
-   "count" : 6,
+   "count" : 7,
    "page" : 1,
-   "page_count" : 6,
+   "page_count" : 7,
    "page_size" : 24,
    "products" : [
       {
@@ -21,6 +21,9 @@
       },
       {
          "product_name" : "Oranges - Organic - Spain - brand1, brand2, brand3"
+      },
+      {
+         "product_name" : "Yogurt - Organic, Fair trade - Martinique"
       }
    ],
    "skip" : 0

diff --git a/...est_results/facets/contributor_tests.json → ...est_results/facets/contributor-alice.json b/...est_results/facets/contributor_tests.json → ...est_results/facets/contributor-alice.json
diff --git a/tests/integration/expected_test_results/facets/contributor-bob.json b/tests/integration/expected_test_results/facets/contributor-bob.json
@@ -0,0 +1,12 @@
+{
+   "count" : 1,
+   "page" : 1,
+   "page_count" : 1,
+   "page_size" : 24,
+   "products" : [
+      {
+         "product_name" : "Yogurt - Organic, Fair trade - Martinique"
+      }
+   ],
+   "skip" : 0
+}
diff --git a/tests/integration/expected_test_results/facets/packager-code_fr-85-222-003-ce.json b/tests/integration/expected_test_results/facets/packager-code_fr-85-222-003-ce.json
@@ -0,0 +1,15 @@
+{
+   "count" : 1,
+   "page" : 1,
+   "page_count" : 1,
+   "page_size" : 24,
+   "products" : [
+      {
+         "emb_codes_tags" : [
+            "fr-85-222-003-ec"
+         ],
+         "product_name" : "Apples - Organic, Fair trade - France"
+      }
+   ],
+   "skip" : 0
+}
diff --git a/tests/integration/expected_test_results/facets/packager-code_fr-85-222-003-ec.json b/tests/integration/expected_test_results/facets/packager-code_fr-85-222-003-ec.json
@@ -0,0 +1,15 @@
+{
+   "count" : 1,
+   "page" : 1,
+   "page_count" : 1,
+   "page_size" : 24,
+   "products" : [
+      {
+         "emb_codes_tags" : [
+            "fr-85-222-003-ec"
+         ],
+         "product_name" : "Apples - Organic, Fair trade - France"
+      }
+   ],
+   "skip" : 0
+}
diff --git a/tests/integration/facets.t b/tests/integration/facets.t
@@ -17,11 +17,18 @@ remove_all_products();
 
 wait_application_ready();
 
-my $ua = new_client();
+# First user
 
-my %create_user_args = (%default_user_form, (email => '[email protected]'));
+my $ua = new_client();
+my %create_user_args = (%default_user_form, (email => '[email protected]', userid => "alice"));
 create_user($ua, \%create_user_args);
 
+# Second user
+
+my $ua2 = new_client();
+my %create_user_args2 = (%default_user_form, (email => '[email protected]', userid => "bob"));
+create_user($ua2, \%create_user_args2);
+
 # Create some products to test facets
 
 my @products = (
@@ -130,6 +137,9 @@ my @products = (
 			categories => "en:apples",
 			labels => "en:organic,en:fair-trade",
 			origins => "en:france",
+			emb_codes =>
+				"FR 85.222.003 CE", # EU traceability code to check we normalize and query the code correctly (CE -> EC)
+			countries => "France",
 		),
 	},
 	{
@@ -158,6 +168,22 @@ foreach my $product_ref (@products) {
 	edit_product($ua, $product_ref);
 }
 
+# Create another product with a different contributor
+
+edit_product(
+	$ua2,
+	{
+		%{dclone(\%empty_product_form)},
+		(
+			code => '200000000013',
+			product_name => "Yogurt - Organic, Fair trade - Martinique",
+			categories => "en:yogurt",
+			labels => "en:organic,en:fair-trade",
+			origins => "en:france",
+		),
+	}
+);
+
 # Note: expected results are stored in json files, see execute_api_tests
 # We use the API with .json to test facets, in order to easily get the products that are returned
 my $tests_ref = [
@@ -254,11 +280,33 @@ my $tests_ref = [
 		expected_status_code => 200,
 		sort_products_by => 'product_name',
 	},
-	# Contributor
+	# EU packager code
+	{
+		test_case => 'packager-code_fr-85-222-003-ce',
+		method => 'GET',
+		path => '/packager-code/fr-85-222-003-ce.json?fields=product_name,emb_codes_tags',
+		expected_status_code => 200,
+		sort_products_by => 'product_name',
+	},
+	{
+		test_case => 'packager-code_fr-85-222-003-ec',    # not normalized code (ec instead of ce)
+		method => 'GET',
+		path => '/packager-code/fr-85-222-003-ce.json?fields=product_name,emb_codes_tags',
+		expected_status_code => 200,
+		sort_products_by => 'product_name',
+	},
+	# contributor facet
+	{
+		test_case => 'contributor-alice',
+		method => 'GET',
+		path => '/contributor/alice.json?fields=product_name',
+		expected_status_code => 200,
+		sort_products_by => 'product_name',
+	},
 	{
-		test_case => 'contributor_tests',
+		test_case => 'contributor-bob',
 		method => 'GET',
-		path => '/contributor/tests.json?fields=product_name',
+		path => '/contributor/bob.json?fields=product_name',
 		expected_status_code => 200,
 		sort_products_by => 'product_name',
 	},