-
-
Notifications
You must be signed in to change notification settings - Fork 384
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
build: Add check_quality service for real-time data validation #10129
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10129 +/- ##
==========================================
+ Coverage 49.54% 49.59% +0.05%
==========================================
Files 67 71 +4
Lines 20650 20996 +346
Branches 4980 5036 +56
==========================================
+ Hits 10231 10414 +183
- Misses 9131 9290 +159
- Partials 1288 1292 +4 ☔ View full report in Codecov by Sentry. |
# Check if nutrition data is provided | ||
if (exists $product_ref->{nutrition}) { | ||
ProductOpener::DataQuality::check_quality($product_ref->{nutrition}); | ||
$updated_product_fields_ref->{nutrition_data_quality_tags} = $product_ref->{nutrition}->{data_quality_tags}; | ||
} | ||
|
||
# Check if ingredient data is provided | ||
if (exists $product_ref->{ingredients}) { | ||
ProductOpener::DataQuality::check_quality($product_ref->{ingredients}); | ||
$updated_product_fields_ref->{ingredient_data_quality_tags} = $product_ref->{ingredients}->{data_quality_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.
check_quality needs the product_ref directly not the 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.
I would call check_quality($product_ref) and then remove eventual false positive (warnings that come from missing fields).
Maybe we will have to categorize the quality errors/warning in respective taxonomies to say on which field they are dependent to filter them.
Or maybe we should split the check_quality function with more granularity or even add a parameter to say which fields we want to check quality.
if ($service eq 'check_quality') { | ||
# Create a temporary product reference for quality checks | ||
my $temp_product_ref = {}; | ||
$temp_product_ref->{nutrition} = $request_body_ref->{nutrition} if defined $request_body_ref->{nutrition}; | ||
$temp_product_ref->{ingredients} = $request_body_ref->{ingredients} if defined $request_body_ref->{ingredients}; | ||
|
||
# Call the check_quality service, passing the temporary product ref | ||
&$service_function($temp_product_ref, $request_ref->{updated_product_fields}); | ||
|
||
# Integrate the quality check results back into the main product_ref | ||
$product_ref->{data_quality_tags} = $temp_product_ref->{data_quality_tags} if defined $temp_product_ref->{data_quality_tags}; | ||
} else { |
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.
The idea of this pattern is to avoid having specific code according to the service…
Also we will put the ingredients and nutrition fields under the "product" field so you have product_ref working.
} | ||
} | ||
return $error; | ||
my $response_ref = $request_ref->{api_response}; |
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.
For Perl files, we use tabs, they shouldn't be converted to spaces. (depending on your editor, it might pick it from the .editorconfig file)
@@ -107,69 +107,99 @@ my %service_functions = ( | |||
extend_ingredients => \&ProductOpener::Ingredients::extend_ingredients_service, | |||
estimate_ingredients_percent => \&ProductOpener::Ingredients::estimate_ingredients_percent_service, | |||
analyze_ingredients => \&ProductOpener::Ingredients::analyze_ingredients_service, | |||
check_quality => \&check_quality_service, |
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.
It would be best to put the check_quality_service function in the DataQuality.pm module like the other services
In fact we could change the existing check_quality function so that it behaves like a service (you can look at other service functions: it expects a product_ref as a first argument, and a reference to a hash of updated fields as the second argument) , and rename it to check_quality_service().
Hi @IsaiahLevy , thanks a lot for the PR. |
Quality Gate passedIssues Measures |
What
This update introduces a new
check_quality
service within theAPIProductServices
module to provide real-time quality warnings and errors for nutrition tables and ingredient lists not directly associated with a product.Screenshot
Related issue(s) and discussion