-
-
Notifications
You must be signed in to change notification settings - Fork 383
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: refactor to allow unlimited facets - WIP #9332
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #9332 +/- ##
==========================================
+ Coverage 48.05% 48.61% +0.56%
==========================================
Files 65 65
Lines 20301 20256 -45
Branches 4921 4894 -27
==========================================
+ Hits 9756 9848 +92
+ Misses 9295 9151 -144
- Partials 1250 1257 +7 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great move !
I propose a small refactoring to have even better readability.
} | ||
my $tagid = $tag_ref->{tagid}; | ||
my $tagtype = $tag_ref->{tagtype}; | ||
my $tag_prefix = $tag_ref->{tag_prefix}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add a comment on what is tag_prefix ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure
lib/ProductOpener/Display.pm
Outdated
my $tag_prefix = $tag_ref->{tag_prefix}; | ||
my $display_tag; | ||
my $canon_tagid; | ||
my $newtagid; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my $newtagid; | |
# tag name in $lc corresponding to $canon_tagid | |
# (if it's not the same as $tagid, we will redirect the request) | |
my $newtagid; |
@@ -408,152 +408,116 @@ sub analyze_request ($request_ref) { | |||
if $log->is_debug(); | |||
} | |||
|
|||
if ( | |||
# Extract tag type / tag value pairs and store them in an array $request_ref->{tags} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we extract this part in a separate sub (making code easier to read) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure.
|
||
=cut | ||
|
||
sub sort_products_for_test_comparison ($array_ref, $sort_field) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea to make it a sub even if it's a one-liner, it really helps understand the code more easily.
# to see data quality panels | ||
execute_api_tests(__FILE__, $tests_ref, $ua); | ||
|
||
done_testing(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😍 some more integration tests
Co-authored-by: Alex Garel <[email protected]>
Co-authored-by: Alex Garel <[email protected]>
Sample mongodb query for http://world.openfoodfacts.localhost/label/organic/category/unknown/labels :
|
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Some initial refactoring to allow to have an unlimited number of / in URLs.