From f8584ad348a8052616e1493e13acc5072141de8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Gigandet?= Date: Tue, 28 Nov 2023 17:17:21 +0100 Subject: [PATCH] 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 Co-authored-by: Alex Garel --- .github/labeler.yml | 2 + lib/ProductOpener/Display.pm | 29 +++++++--- .../facets/brand_-brand1.json | 7 ++- .../facets/brand_unknown.json | 7 ++- .../facets/category_-apples.json | 7 ++- .../category_-apples_label_organic.json | 7 ++- ...utor_tests.json => contributor-alice.json} | 0 .../facets/contributor-bob.json | 12 ++++ .../packager-code_fr-85-222-003-ce.json | 15 +++++ .../packager-code_fr-85-222-003-ec.json | 15 +++++ tests/integration/facets.t | 58 +++++++++++++++++-- 11 files changed, 137 insertions(+), 22 deletions(-) rename tests/integration/expected_test_results/facets/{contributor_tests.json => contributor-alice.json} (100%) create mode 100644 tests/integration/expected_test_results/facets/contributor-bob.json create mode 100644 tests/integration/expected_test_results/facets/packager-code_fr-85-222-003-ce.json create mode 100644 tests/integration/expected_test_results/facets/packager-code_fr-85-222-003-ec.json diff --git a/.github/labeler.yml b/.github/labeler.yml index 6c6ab7cdcf0a6..1c52e9cf20623 100644 --- 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 index f43eff3990c6d..ff467a3bb46cc 100644 --- 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 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) { = "\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 index ec6fc8d3e5a67..bd6716c5a7c6a 100644 --- 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 index 07471b66408df..7c7ce41ca6924 100644 --- 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 index 2a872feffcf86..3d1fc27a2fafb 100644 --- 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 index b082e074d23b2..3dcaa4d1b27a5 100644 --- 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/tests/integration/expected_test_results/facets/contributor_tests.json b/tests/integration/expected_test_results/facets/contributor-alice.json similarity index 100% rename from tests/integration/expected_test_results/facets/contributor_tests.json rename to tests/integration/expected_test_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 new file mode 100644 index 0000000000000..71683311099b5 --- /dev/null +++ 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 new file mode 100644 index 0000000000000..7888c8f8ff590 --- /dev/null +++ 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 new file mode 100644 index 0000000000000..7888c8f8ff590 --- /dev/null +++ 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 index 46c28c434ceb7..350a4a6fae3e1 100644 --- 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 => 'bob@gmail.com')); +my $ua = new_client(); +my %create_user_args = (%default_user_form, (email => 'alice@gmail.com', userid => "alice")); create_user($ua, \%create_user_args); +# Second user + +my $ua2 = new_client(); +my %create_user_args2 = (%default_user_form, (email => 'bob@gmail.com', 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', },