Skip to content
This repository has been archived by the owner on Sep 5, 2023. It is now read-only.

Taxonomy model for Label is not correct #720

Merged
merged 2 commits into from
Oct 6, 2020
Merged

Taxonomy model for Label is not correct #720

merged 2 commits into from
Oct 6, 2020

Conversation

jncosideout
Copy link
Contributor

@jncosideout jncosideout commented Aug 23, 2020 •
edited
Loading

PR Description

While discussing how to create a mock-up for Issue #707 @teolemon realized that Labels have not been implemented yet. I have taken the first steps in adding the necessary methods to TaxonomyService, TaxonomyParser PersistanceManager by mimicking the other Taxonomy methods in the protocol/API.

I am stuck on how to implement the Label model. I copied the Category model because from my limited knowledge, they seem to have similar data models, and I noticed that the model for Mineral may be identical to Category. For clarity, I am talking about the models under Sources/Models/API/Taxonomies/

Type of Changes

Checklist

Make sure you've done all the following (Put an x in the boxes that apply.)

  • If you have multiple commits please combine them into one commit by squashing them.
  • Code is well documented
  • Included unit tests for new functionality
  • All user-visible strings are made translatable
  • Code passes Travis builds in your branch

@aleene aleene marked this pull request as draft August 26, 2020 06:59
@jncosideout jncosideout force-pushed the Add-Labels-to-TaxonomiesService branch from e896c8e to 210e186 Compare August 31, 2020 17:51
@jncosideout
Copy link
Contributor Author

By adding labelsTags to the Product API model and mapping it to the OFFJson, Labels are now displayed on scan. However, they aren't translated correctly yet, despite having fixed the downloading of Taxonomies on launch of the app.

As you can see below in the before and after pictures, the Labels are now included in the tableview of the ProductDetail Rows but they are not being translated into English, which is the default language on my testing device.

The labels 'Bio, Bio européen, Agriculture UE/Non-UE' are the French translations of those Labels, and in English they should say 'en:organic, en:eu-organic, en:ab-agriculture-biologique, en:eu-non-eu-agriculture.'

Before

before_labels

After

after_labels

@aleene
Copy link
Collaborator

aleene commented Sep 14, 2020

Well, it looks good. Is now the translation the only remaining issue?

@jncosideout
Copy link
Contributor Author

@aleene
Yes, still need to figure out translations. I don't know how the downloaded taxonomies are supposed to be used to to translate. Like I said above, I believe that the downloading of taxonomies was not working before I made my changes. So there must be some way to look up a Label in the downloaded taxonomy database, e.g. after scanning a product which has the 'Organic' Label to get its translations.

@jncosideout jncosideout force-pushed the Add-Labels-to-TaxonomiesService branch from d3f737e to ea88f67 Compare September 26, 2020 21:52
@jncosideout
Copy link
Contributor Author

jncosideout commented Sep 26, 2020 •
edited
Loading

As you can see below in the before and after pictures, the Labels are now included in the tableview of the ProductDetail Rows but they are not being translated into English, which is the default language on my testing device.

@teolemon @aleene
I have fixed the translation of the Labels. I started by checking how the Categories are translated, but I found that the code to do so was not working correctly. They were always being displayed in English, even when I changed the language setting to Français (see below):

Before

Language : French
Categories : English
Labels : English (displaying Labels_Tags values for testing)
OrangeAmere-in_French-Labels_tags

After

Language : French
Categories : French
Labels : French
OrangeAmere-in_French-Categories-translated

Language : English
Categories : English
Labels : English
OrangeAmere-in_English-Labels_translated

[EDIT: Below are my findings from before I figured out how to use taxonomies to translate Labels and Categories]

The product I tested with was Orange Amère - Barcode: 3760036843626(EAN / EAN-13)
The reason the Labels appeared in French and not English (which is the default language on my testing device) was because this particular product has different values for the ‘Labels’ on the dev server than on the production server.

On the dev server, this product’s
‘label’ values are in French,
‘label_tags’ values are in English.
label_lc: fr

On the production server, this product’s
‘label’ values are in English,
‘label_tags’ values are in English.
label_lc: en

@teolemon
Copy link
Member

teolemon commented Sep 27, 2020 •
edited
Loading

Let's focus on what production does. label_tags are always in English, and labels will be in the main language of the product. 80-90% of the values will be in label_tags in english, and the non-taxonomized values will also be there, with a language prefix.
Let's display what's already taxonomized, if possible in the user language for the phone, or in the main language of the product, or in English at the last resort.

@jncosideout
Copy link
Contributor Author

jncosideout commented Sep 27, 2020 •
edited
Loading

label_tags are always in English, and labels will be in the main language of the product.

@teolemon
Sorry for the confusion. Yes at first I used Labels_tags as a work-around to translate into English , then I realized they are always in English but they have the "en:" prefix and therefore that was not the correct way to do it.

If you look at my 2nd and 3rd screenshots in my last post you will see that the Labels are displaying their translated values, and if you look at the changes I made to ProductDetailViewController you will see I am using the downloaded taxonomies to fetch the translated names.

@jncosideout jncosideout force-pushed the Add-Labels-to-TaxonomiesService branch from ea88f67 to 0578263 Compare September 28, 2020 18:38
@jncosideout jncosideout marked this pull request as ready for review September 28, 2020 18:41
@jncosideout
Add Labels to TaxonomiesService and Product view
34bccfe 
Labels were partially implemented but still needed to be incorporated
into the API Models and Network Services. In doing so, it was discovered
that the Taxonomies weren't being downloaded to Realm on first launch of
the app as intended. After fixing that and adding the Label taxonomy to
Models, they were successfully retrieved and stored. Then, to allow
Labels to be displayed by ProductDetailViewController, they were
included in the OFFJson mapping. The downloaded taxonomies were then
used to translate Labels and code was added to fix translation of
Categories. Lastly, the functionality to link link Labels to their OFF
urls was added.

Closes openfoodfacts#719
@jncosideout jncosideout force-pushed the Add-Labels-to-TaxonomiesService branch from 0578263 to 34bccfe Compare September 29, 2020 20:14
jncosideout
jncosideout commented Sep 30, 2020
View reviewed changes
Sources/Helpers/OFFUrlsHelper.swift Outdated Show resolved Hide resolved
Sources/ViewControllers/Products/Detail/ProductDetailViewController.swift Outdated Show resolved Hide resolved
Sources/Helpers/OFFUrlsHelper.swift
static func url(forLabel label: Label) -> URL {
return URL(string: "\(baseUrl())/label/\(label.code)")!
}

// Not sure if there is a taxonomy for this
/*
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merged with the latest changes from this PR Open weblinks to OFF urls in the language chosen in app settings, dynamically #747

aleene
aleene previously approved these changes Oct 1, 2020
View reviewed changes
Copy link
Collaborator

@aleene aleene left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many changes, so if it works for you, we can do some testing on develop.

@jncosideout jncosideout mentioned this pull request Oct 5, 2020
Closed
1 task
@teolemon teolemon changed the base branch from develop to release/3.4 October 6, 2020 20:19
@teolemon teolemon changed the base branch from release/3.4 to develop October 6, 2020 20:19
@teolemon teolemon dismissed aleene’s stale review October 6, 2020 20:19

The base branch was changed.

@teolemon teolemon merged commit 685fc79 into openfoodfacts:develop Oct 6, 2020
@aleene aleene added this to the Version 3.4 milestone Oct 8, 2020
@aleene aleene changed the title WIP: Taxonomy model for Label is not correct Taxonomy model for Label is not correct Oct 8, 2020
@jncosideout jncosideout deleted the Add-Labels-to-TaxonomiesService branch October 14, 2020 19:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@aleene aleene aleene left review comments

@philippeauriach philippeauriach Awaiting requested review from philippeauriach

@teolemon teolemon Awaiting requested review from teolemon

@mohammedsafwat mohammedsafwat Awaiting requested review from mohammedsafwat

Assignees

@jncosideout jncosideout

Labels
P1 product-browsing
Projects
None yet
Milestone
Version 3.4
Development

Successfully merging this pull request may close these issues.
4 participants
@jncosideout @aleene @teolemon @philippeauriach