Skip to content

Commit

Permalink
fix!: 742 - robotoff - countries instead of country (openfoodfacts#792)
Browse files Browse the repository at this point in the history 
Impacted files:
* `api_get_robotoff_test.dart`: minor refactoring
* `open_food_api_client.dart`: removed a deprecated method
* `robot_off_api_client.dart`: new robotoff syntax - replaced country with countries
  • Loading branch information
@monsieurtanuki
monsieurtanuki authored Sep 2, 2023
1 parent 527373a commit 8582519
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 103 deletions.
17 changes: 0 additions & 17 deletions lib/src/open_food_api_client.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -788,23 +788,6 @@ class OpenFoodAPIClient {
}
}
//TODO: deprecated from 2023-04-05; remove when old enough
@Deprecated('Use [RobotOffAPIClient.getRandomInsight] Instead')
static Future<InsightsResult> getRandomInsight(
User user, {
InsightType? type,
String? country,
String? valueTag,
String? serverDomain,
QueryType? queryType,
}) =>
RobotoffAPIClient.getRandomInsights(
type: type,
country: OpenFoodFactsCountry.fromOffTag(country),
valueTag: valueTag,
serverDomain: serverDomain,
queryType: queryType);
//TODO: deprecated from 2023-04-05; remove when old enough
@Deprecated('Use [RobotOffAPIClient.getProductInsights] Instead')
static Future<InsightsResult> getProductInsights(
Expand Down
86 changes: 17 additions & 69 deletions lib/src/robot_off_api_client.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class RobotoffAPIClient {

static Future<InsightsResult> getRandomInsights({
InsightType? type,
OpenFoodFactsCountry? country,
Iterable<OpenFoodFactsCountry>? countries,
String? valueTag,
// TODO: deprecated from 2023-06-13; remove when old enough
@Deprecated('Not used anymore') String? serverDomain,
Expand All @@ -29,7 +29,8 @@ class RobotoffAPIClient {
}) async {
final Map<String, String> parameters = {
if (type != null) 'type': type.offTag,
if (country != null) 'country': _getLousyCountryTag(country),
if (countries?.isNotEmpty == true)
'countries': _getCountryList(countries!),
if (valueTag != null) 'value_tag': valueTag,
if (count != null) 'count': count.toString(),
if (serverType != null) 'server_type': serverType.offTag,
Expand Down Expand Up @@ -167,7 +168,7 @@ class RobotoffAPIClient {
int? count,
int? page,
List<InsightType>? insightTypes,
OpenFoodFactsCountry? country,
Iterable<OpenFoodFactsCountry>? countries,
List<String>? brands,
RobotoffQuestionOrder? questionOrder,
ServerType? serverType,
Expand All @@ -189,7 +190,8 @@ class RobotoffAPIClient {
if (insightValues.isNotEmpty) 'insight_types': insightValues.join(','),
if (brands?.isNotEmpty == true) 'brands': brands!.join(','),
if (questionOrder != null) 'order_by': questionOrder.offTag,
if (country != null) 'country': _getLousyCountryTag(country),
if (countries?.isNotEmpty == true)
'countries': _getCountryList(countries!),
if (valueTag != null) 'value_tag': valueTag,
};

Expand All @@ -209,71 +211,6 @@ class RobotoffAPIClient {
);
}

/// Lousy conversion from a country to a country tag.
///
/// In a near future, we should be able to get rid of this method, and pass
/// directly the country (offTag).
/// There are some countries for which I don't know if the country tag is
/// misspelled or just if there are no data for this country.
static String _getLousyCountryTag(final OpenFoodFactsCountry country) {
switch (country) {
case OpenFoodFactsCountry.BRUNEI_DARUSSALAM:
return 'en:brunei';
case OpenFoodFactsCountry.CZECHIA:
return 'en:czech-republic';
case OpenFoodFactsCountry.USA:
return 'en:united-states';
case OpenFoodFactsCountry.VIET_NAM:
return 'en:vietnam';
// from there I cannot say if the spelling is correct...
case OpenFoodFactsCountry.ANTARCTICA:
case OpenFoodFactsCountry.SAINT_BARTHELEMY:
case OpenFoodFactsCountry.BAHAMAS:
case OpenFoodFactsCountry.BHUTAN:
case OpenFoodFactsCountry.BOUVET_ISLAND:
case OpenFoodFactsCountry.COCOS_ISLANDS:
case OpenFoodFactsCountry.CONGO:
case OpenFoodFactsCountry.CABO_VERDE:
case OpenFoodFactsCountry.CHRISTMAS_ISLAND:
case OpenFoodFactsCountry.WESTERN_SAHARA:
case OpenFoodFactsCountry.FALKLAND_ISLANDS:
case OpenFoodFactsCountry.MICRONESIA:
case OpenFoodFactsCountry.SOUTH_GEORGIA:
case OpenFoodFactsCountry.GUINEA_BISSAU:
case OpenFoodFactsCountry.HEARD_ISLAND:
case OpenFoodFactsCountry.BRITISH_INDIAN_OCEAN_TERRITORY:
case OpenFoodFactsCountry.KIRIBATI:
case OpenFoodFactsCountry.NORTH_KOREA:
case OpenFoodFactsCountry.LAOS:
case OpenFoodFactsCountry.LESOTHO:
case OpenFoodFactsCountry.MACAO:
case OpenFoodFactsCountry.NORFOLK_ISLAND:
case OpenFoodFactsCountry.NAURU:
case OpenFoodFactsCountry.NIUE:
case OpenFoodFactsCountry.PITCAIRN:
case OpenFoodFactsCountry.PALESTINE:
case OpenFoodFactsCountry.SUDAN:
case OpenFoodFactsCountry.SAINT_HELENA:
case OpenFoodFactsCountry.SVALBARD_AND_JAN_MAYEN:
case OpenFoodFactsCountry.SAO_TOME_AND_PRINCIPE:
case OpenFoodFactsCountry.ESWATINI:
case OpenFoodFactsCountry.FRENCH_SOUTHERN_TERRITORIES:
case OpenFoodFactsCountry.TAJIKISTAN:
case OpenFoodFactsCountry.TOKELAU:
case OpenFoodFactsCountry.TIMOR_LESTE:
case OpenFoodFactsCountry.TURKMENISTAN:
case OpenFoodFactsCountry.TONGA:
case OpenFoodFactsCountry.TUVALU:
case OpenFoodFactsCountry.UNITED_STATES_MINOR_OUTLYING_ISLANDS:
case OpenFoodFactsCountry.HOLY_SEE:
case OpenFoodFactsCountry.US_VIRGIN_ISLANDS:
case OpenFoodFactsCountry.SAMOA:
default:
// works in most cases
return 'en:${country.name.toLowerCase().replaceAll('_', '-')}';
}
}

static Future<Status> postInsightAnnotation(
String? insightId,
InsightAnnotation annotation, {
Expand Down Expand Up @@ -308,4 +245,15 @@ class RobotoffAPIClient {
);
return Status.fromApiResponse(response.body);
}

/// Returns a list of country as comma-separated 2-letter codes
static String _getCountryList(
final Iterable<OpenFoodFactsCountry> countries,
) {
final List<String> result = <String>[];
for (final OpenFoodFactsCountry country in countries) {
result.add(country.offTag);
}
return result.join(',');
}
}
69 changes: 52 additions & 17 deletions test/api_get_robotoff_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,25 +103,60 @@ void main() {
});

test('get popular questions with types', () async {
const InsightType type = InsightType.CATEGORY;
const int numQuestions = 10;
final RobotoffQuestionResult result =
await RobotoffAPIClient.getQuestions(
OpenFoodFactsLanguage.GERMAN,
user: TestConstants.PROD_USER,
insightTypes: [type],
count: numQuestions,
country: OpenFoodFactsCountry.GERMANY,
questionOrder: RobotoffQuestionOrder.popularity,
Future<List<RobotoffQuestion>> getTopPopularQuestions(
final OpenFoodFactsCountry country,
) async {
const InsightType type = InsightType.CATEGORY;
const int numQuestions = 10;
final RobotoffQuestionResult result =
await RobotoffAPIClient.getQuestions(
OpenFoodFactsLanguage.GERMAN,
user: TestConstants.PROD_USER,
insightTypes: [type],
count: numQuestions,
countries: <OpenFoodFactsCountry>[country],
questionOrder: RobotoffQuestionOrder.popularity,
);

expect(result.status, isNotNull);
expect(result.status, 'found');
expect(result.questions, isNotNull);
expect(result.questions!.length, numQuestions);
for (final RobotoffQuestion question in result.questions!) {
expect(question.insightType, type);
}
return result.questions!;
}

List<RobotoffQuestion> questions;

questions = await getTopPopularQuestions(
OpenFoodFactsCountry.GERMANY,
);
final List<String> germanBarcodes1 = <String>[];
for (final RobotoffQuestion question in questions) {
germanBarcodes1.add(question.barcode!);
}

expect(result.status, isNotNull);
expect(result.status, 'found');
expect(result.questions, isNotNull);
expect(result.questions!.length, numQuestions);
for (final RobotoffQuestion question in result.questions!) {
expect(question.insightType, type);
questions = await getTopPopularQuestions(
OpenFoodFactsCountry.GERMANY,
);
final List<String> germanBarcodes2 = <String>[];
for (final RobotoffQuestion question in questions) {
germanBarcodes2.add(question.barcode!);
}
// highly probable
expect(germanBarcodes2, germanBarcodes1);

questions = await getTopPopularQuestions(
OpenFoodFactsCountry.FRANCE,
);
final List<String> frenchBarcodes1 = <String>[];
for (final RobotoffQuestion question in questions) {
frenchBarcodes1.add(question.barcode!);
}
// highly probable
expect(germanBarcodes2, isNot(frenchBarcodes1));
});

test('get 2 random questions with no specific type', () async {
Expand All @@ -143,7 +178,7 @@ void main() {
test('get random insight', () async {
final InsightsResult result = await RobotoffAPIClient.getRandomInsights(
type: InsightType.CATEGORY,
country: OpenFoodFactsCountry.FRANCE,
countries: <OpenFoodFactsCountry>[OpenFoodFactsCountry.FRANCE],
);

expect(result.status, isNotNull);
Expand Down

0 comments on commit 8582519

Please sign in to comment.