From 3b0eefed2cd12b85a17ad204e2c34f240dc303b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Gigandet?= Date: Thu, 23 Nov 2023 09:54:23 +0100 Subject: [PATCH 1/6] fix issue with emb codes / packager codes facets --- lib/ProductOpener/Display.pm | 20 ++++++++++++------- .../packager-code_fr-85-222-003-ce.json | 15 ++++++++++++++ .../packager-code_fr-85-222-003-ec.json | 15 ++++++++++++++ tests/integration/facets.t | 18 +++++++++++++++++ 4 files changed, 61 insertions(+), 7 deletions(-) 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/lib/ProductOpener/Display.pm b/lib/ProductOpener/Display.pm index da6ef84397c40..f5d714c2a2a37 100644 --- a/lib/ProductOpener/Display.pm +++ b/lib/ProductOpener/Display.pm @@ -3979,7 +3979,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}; } @@ -4149,7 +4149,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; } @@ -4595,6 +4595,9 @@ sub add_params_to_query ($request_ref, $query_ref) { } else { $tagid2 = get_string_id_for_lang("no_language", canonicalize_tag2($tagtype, $tag2)); + if ($tagtype eq 'emb_codes') { + $tagid2 =~ s/-($ec_code_regexp)$/-ec/ie; + } } push @tagids, $tagid2; } @@ -4622,6 +4625,9 @@ sub add_params_to_query ($request_ref, $query_ref) { } else { $tagid = get_string_id_for_lang("no_language", canonicalize_tag2($tagtype, $tag)); + 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}) @@ -7072,8 +7078,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}; @@ -7251,7 +7257,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, @@ -10503,7 +10509,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, @@ -10530,7 +10536,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/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..f0049a99ba667 100644 --- a/tests/integration/facets.t +++ b/tests/integration/facets.t @@ -130,6 +130,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", ), }, { @@ -262,6 +265,21 @@ my $tests_ref = [ expected_status_code => 200, sort_products_by => 'product_name', }, + # 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', + }, ]; # note: we need to execute the tests with bob, because we need authentication From e262c0491664c1e84e044ba132254eec16d2f423 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Gigandet?= Date: Thu, 23 Nov 2023 10:19:45 +0100 Subject: [PATCH 2/6] fix contributor facet --- lib/ProductOpener/Display.pm | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/ProductOpener/Display.pm b/lib/ProductOpener/Display.pm index f5d714c2a2a37..a9833e593956c 100644 --- a/lib/ProductOpener/Display.pm +++ b/lib/ProductOpener/Display.pm @@ -4511,7 +4511,7 @@ my %ignore_params = ( # Parameters that can be query filters # 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) { @@ -4595,6 +4595,7 @@ 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; } @@ -4625,6 +4626,7 @@ 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; } @@ -4646,6 +4648,7 @@ sub add_params_to_query ($request_ref, $query_ref) { ) ) and ($tagtype !~ /^pnns_groups_/) + and ($tagtype ne "creator") ) { if ($not) { From e72db15d4d5d4e6544d0f54bb7311fad97ed4658 Mon Sep 17 00:00:00 2001 From: Pierre Slamich Date: Thu, 23 Nov 2023 10:40:39 +0100 Subject: [PATCH 3/6] Update labeler.yml --- .github/labeler.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/labeler.yml b/.github/labeler.yml index fcb557d39d9fa..d49cebc7bbfd7 100644 --- a/.github/labeler.yml +++ b/.github/labeler.yml @@ -410,6 +410,7 @@ Packager Codes: - 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 📦 Packaging: - lib/ProductOpener/Packaging.pm From 9fa31a8ad1117b0fa982d4dfa6f9e90b05609744 Mon Sep 17 00:00:00 2001 From: Pierre Slamich Date: Thu, 23 Nov 2023 10:42:00 +0100 Subject: [PATCH 4/6] Update labeler.yml --- .github/labeler.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/labeler.yml b/.github/labeler.yml index d49cebc7bbfd7..bedc1fc20101b 100644 --- a/.github/labeler.yml +++ b/.github/labeler.yml @@ -411,6 +411,7 @@ Packager Codes: - 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 From 462b751fd2726ef0f2556e377494c04e708b5295 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Gigandet?= Date: Mon, 27 Nov 2023 19:01:53 +0100 Subject: [PATCH 5/6] more tests and comments --- lib/ProductOpener/Display.pm | 4 +- .../facets/brand_-brand1.json | 7 ++- .../facets/brand_unknown.json | 7 ++- .../facets/category_-apples.json | 7 ++- .../category_-apples_label_organic.json | 7 ++- .../facets/contributor-alice.json | 45 +++++++++++++++++++ .../facets/contributor-bob.json | 12 +++++ tests/integration/facets.t | 42 ++++++++++++++++- 8 files changed, 120 insertions(+), 11 deletions(-) create mode 100644 tests/integration/expected_test_results/facets/contributor-alice.json create mode 100644 tests/integration/expected_test_results/facets/contributor-bob.json diff --git a/lib/ProductOpener/Display.pm b/lib/ProductOpener/Display.pm index af94a08b6c0ce..3bea683b6948c 100644 --- a/lib/ProductOpener/Display.pm +++ b/lib/ProductOpener/Display.pm @@ -4529,7 +4529,9 @@ 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, creator => 1); 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-alice.json b/tests/integration/expected_test_results/facets/contributor-alice.json new file mode 100644 index 0000000000000..5550899dc9fed --- /dev/null +++ b/tests/integration/expected_test_results/facets/contributor-alice.json @@ -0,0 +1,45 @@ +{ + "count" : 12, + "page" : 1, + "page_count" : 12, + "page_size" : 24, + "products" : [ + { + "product_name" : "Apples - Organic - France" + }, + { + "product_name" : "Apples - Organic - France, Belgium, Canada" + }, + { + "product_name" : "Apples - Organic, Fair trade - France" + }, + { + "product_name" : "Bananas - Organic - France - brand2" + }, + { + "product_name" : "Bananas - Organic - Martinique - brand1" + }, + { + "product_name" : "Carrots - Fair trade - Italy - brand1, brand2" + }, + { + "product_name" : "Carrots - Fair trade, Organic - France - brand1" + }, + { + "product_name" : "Carrots - No label - Belgium - brand2" + }, + { + "product_name" : "Carrots - Organic - France - brand1, brand2" + }, + { + "product_name" : "Chocolate - Organic, Fair trade - Martinique" + }, + { + "product_name" : "Oranges - Fair trade - Italy - brand3" + }, + { + "product_name" : "Oranges - Organic - Spain - brand1, brand2, brand3" + } + ], + "skip" : 0 +} 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/facets.t b/tests/integration/facets.t index f0049a99ba667..98f2a4607093b 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 = ( @@ -161,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 = [ @@ -280,6 +303,21 @@ my $tests_ref = [ 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-bob', + method => 'GET', + path => '/contributor/bob.json?fields=product_name', + expected_status_code => 200, + sort_products_by => 'product_name', + }, ]; # note: we need to execute the tests with bob, because we need authentication From 0692fdc157f30abc43025f8f99694860cc853242 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Gigandet?= Date: Tue, 28 Nov 2023 09:46:27 +0100 Subject: [PATCH 6/6] update tests --- .../facets/contributor_tests.json | 45 ------------------- tests/integration/facets.t | 8 ---- 2 files changed, 53 deletions(-) delete mode 100644 tests/integration/expected_test_results/facets/contributor_tests.json diff --git a/tests/integration/expected_test_results/facets/contributor_tests.json b/tests/integration/expected_test_results/facets/contributor_tests.json deleted file mode 100644 index 5550899dc9fed..0000000000000 --- a/tests/integration/expected_test_results/facets/contributor_tests.json +++ /dev/null @@ -1,45 +0,0 @@ -{ - "count" : 12, - "page" : 1, - "page_count" : 12, - "page_size" : 24, - "products" : [ - { - "product_name" : "Apples - Organic - France" - }, - { - "product_name" : "Apples - Organic - France, Belgium, Canada" - }, - { - "product_name" : "Apples - Organic, Fair trade - France" - }, - { - "product_name" : "Bananas - Organic - France - brand2" - }, - { - "product_name" : "Bananas - Organic - Martinique - brand1" - }, - { - "product_name" : "Carrots - Fair trade - Italy - brand1, brand2" - }, - { - "product_name" : "Carrots - Fair trade, Organic - France - brand1" - }, - { - "product_name" : "Carrots - No label - Belgium - brand2" - }, - { - "product_name" : "Carrots - Organic - France - brand1, brand2" - }, - { - "product_name" : "Chocolate - Organic, Fair trade - Martinique" - }, - { - "product_name" : "Oranges - Fair trade - Italy - brand3" - }, - { - "product_name" : "Oranges - Organic - Spain - brand1, brand2, brand3" - } - ], - "skip" : 0 -} diff --git a/tests/integration/facets.t b/tests/integration/facets.t index 98f2a4607093b..350a4a6fae3e1 100644 --- a/tests/integration/facets.t +++ b/tests/integration/facets.t @@ -280,14 +280,6 @@ my $tests_ref = [ expected_status_code => 200, sort_products_by => 'product_name', }, - # Contributor - { - test_case => 'contributor_tests', - method => 'GET', - path => '/contributor/tests.json?fields=product_name', - expected_status_code => 200, - sort_products_by => 'product_name', - }, # EU packager code { test_case => 'packager-code_fr-85-222-003-ce',