Skip to content
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

Incorrect nesting of nutrients and sub-nutrients in /cgi/nutrients.pl API #5997

Closed
stephanegigandet opened this issue Oct 18, 2021 · 6 comments · Fixed by #6031
Closed

Incorrect nesting of nutrients and sub-nutrients in /cgi/nutrients.pl API #5997

stephanegigandet opened this issue Oct 18, 2021 · 6 comments · Fixed by #6031
Labels
API Issues related to the Open Food Facts API. More specific labels exist & should be used (API WRITE…) 🐛 bug This is a bug, not a feature request. Nutrients

Comments

@stephanegigandet
Copy link
Contributor

Describe the bug

https://world.openfoodfacts.org/cgi/nutrients.pl returns a nested structure with nutrients so that apps can know how to display nutrition table, in which order etc. (the result varies based on the country code cc parameter).

The nutrients are not nested correctly though.

e.g.

{
name: "Carbohydrates",
nutrients: [
{
name: "Sugars",
nutrients: [
{
name: "Sucrose",
nutrients: [
{
id: "glucose",
important: false,
display_in_edit_form: false,
name: "Glucose"
},
{
id: "fructose",
important: false,
display_in_edit_form: false,
name: "Fructose"
},
{
name: "Lactose",
display_in_edit_form: false,
important: false,
id: "lactose"
},
{
important: false,
display_in_edit_form: false,
id: "maltose",
name: "Maltose"
},
{
name: "Maltodextrins",
id: "maltodextrins",
important: false,
display_in_edit_form: false
},
{
name: "Starch",
display_in_edit_form: false,
important: false,
id: "starch"
},
{
display_in_edit_form: false,
important: false,
id: "polyols",
name: "Sugar alcohols (Polyols)"
},
{
id: "fiber",
display_in_edit_form: true,
important: false,
name: "Fibers"
},
{
name: "Soluble fiber",
id: "soluble-fiber",
display_in_edit_form: false,
important: false
},
{
id: "insoluble-fiber",
display_in_edit_form: false,
important: false,
name: "Insoluble fiber"
}
],
important: false,
display_in_edit_form: false,
id: "sucrose"
}
],
id: "sugars",
important: false,
display_in_edit_form: true
}
],
important: true,
display_in_edit_form: true,
id: "carbohydrates"
},

To Reproduce

https://world.openfoodfacts.org/cgi/nutrients.pl

Expected behavior

Nutrients should be in the order defined in Food.pm

Screenshots

No response

Additional context

Reported by @monsieurtanuki : openfoodfacts/openfoodfacts-dart#210 (comment)

Type of device

REST-API

Browser version

No response

Number of products impacted

No response

Time per product

No response

@stephanegigandet stephanegigandet added 🐛 bug This is a bug, not a feature request. API Issues related to the Open Food Facts API. More specific labels exist & should be used (API WRITE…) Nutrients labels Oct 18, 2021
@monsieurtanuki
Copy link
Contributor

Not 100% related, but there seems to be a little difference between https://fr.off... and https://world.off/?cc=fr links.
In the first case, the (default) language seems to be deduced from the country.
In the second case, the language is the default (English) language, regardless of the country.
I heard that we were supposed to use rather the "https://world?cc=" syntax, but there's a side effect.

@stephanegigandet
Copy link
Contributor Author

Not 100% related, but there seems to be a little difference between https://fr.off... and https://world.off/?cc=fr links.

It is the expected behaviour. If you use a subdomain like [country code].openfoodfacts.org, it will set a default country cc and a default language lc. For instance ch.openfoodfacts.org will set the country to Switzerland, and the language to German.

If you use ?cc= or ?lc= to override the country or language, it will work, but setting ?cc will only override the country, it will not set a default language.

You can do this: https://world.openfoodfacts.org/cgi/nutrients.pl?cc=fr&lc=fr

One thing about the nutrients.pl API is that you will get different results per country (which nutrients and in which order), and by language (translation of the name of the nutrient).

@monsieurtanuki
Copy link
Contributor

@stephanegigandet Understood.
As we are supposed from now on to use the world. domain in off-dart, I've made the language parameter mandatory in openfoodfacts/openfoodfacts-dart#272.

@monsieurtanuki
Copy link
Contributor

I don't code in perl, but from what I read in https://github.com/openfoodfacts/openfoodfacts-server/blob/main/cgi/nutrients.pl:

  • it would be much easier to store the latest level 0 and level 1 parents
  • if I'm on level 0, I have no parent, and I'm the latest level 0 parent
  • if I'm on level 1, my parent is the latest level 0 parent, and I'm the latest level 1 parent
  • if I'm on level 2, my parent is the latest level 1 parent

@monsieurtanuki
Copy link
Contributor

This is my suggestion (worked on "europe" data).
Sorry I cannot PR but I am almost as clumsy in perl (a once-in-a-lifetime experience for me) as I am in github.

The fix is about:

  • the level, that for some obscure regexp reasons was incorrect in some rare cases.
  • the hierarchy algorithm, that seemed flawed too
my @table = ();
my %parent_level0;
my %parent_level1;
foreach (@{$nutriments_tables{$nutriment_table}}) {
	my $nid = $_;	# Copy instead of alias

	$nid =~/^#/ and next;
	my $important = ($nid =~ /^!/) ? JSON::PP::true : JSON::PP::false;
	$nid =~ s/!//g;
	my $default_edit_form = $nid =~ /-$/ ? JSON::PP::false : JSON::PP::true;
	$nid =~ s/-$//g;

	my $onid = $nid =~ s/^(\-+)//gr;
	my $prefix_length = 0;
	if ($nid =~ s/^--//g) {
		$prefix_length = 2;
	} elsif($nid =~ s/^-//g) {
		$prefix_length = 1;
	}
	my %current = ( id => $onid, important => $important, display_in_edit_form => $default_edit_form );
	my $current_ref = \%current;
	my $name = get_nutrient_label($onid, $lc);
	if (defined $name) {
		$current_ref->{name} = $name;
	}

	if ($prefix_length eq 1) {
		@{$parent_level0->{nutrients}} = () unless defined $parent_level0->{nutrients};
		push @{$parent_level0->{nutrients}}, $current_ref unless not defined $current_ref;
	}
	elsif ($prefix_length eq 2) {
		@{$parent_level1->{nutrients}} = () unless defined $parent_level1->{nutrients};
		push @{$parent_level1->{nutrients}}, $current_ref unless not defined $current_ref;
	}
	elsif ($prefix_length eq 0) {
		push @table, $current_ref unless not defined $current_ref;
	}
	else {
		print "unexpected case";
	}

	if ($prefix_length eq 0) {
		$parent_level0 = $current_ref;
	} elsif ($prefix_length eq 1) {
		$parent_level1 = $current_ref;
	}
}

my %result = ( nutrients => \@table );
my $data = encode_json(\%result);
print header( -type => 'application/json', -content_language => $lc, -charset => 'utf-8', -access_control_allow_origin => '*', -cache_control => 'public, max-age: 86400' ) . $data;

In addition to that, there's a funny hierarchy in Food.pm:

'fiber', # should probably be '-fiber'
'--soluble-fiber-',
'--insoluble-fiber-',

@stephanegigandet
Copy link
Contributor Author

@monsieurtanuki Thank you very much for the fix, I made a corresponding PR here: #6031

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Issues related to the Open Food Facts API. More specific labels exist & should be used (API WRITE…) 🐛 bug This is a bug, not a feature request. Nutrients
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants