-
-
Notifications
You must be signed in to change notification settings - Fork 286
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: #2339 - added language and country to query product list keys #2465
feat: #2339 - added language and country to query product list keys #2465
Conversation
…t list keys What: * added language and country to query product list keys * that means that if I run a query for "pizza", not only the results will be different depending on the language and country (that was already the case), but the results will be cached accordingly - if I change language or country, I will see the cached results for those new language/country, not the results of the same query but with different language/country * we needed that improvement in order to display correctly the "world" results; that was the first step * also moved all the "query" classes to a specific folder * fun fact: there are more "pizza"s in France than in Italy Impacted files: * `abstract_onboarding_data.dart`: minor refactoring * `barcode_product_query.dart`: moved; minor refactoring * `category_product_query.dart`: moved; added language and country to product list key * `continuous_scan_model.dart`: minor refactoring * `country_selector.dart`: minor refactoring * `database_product_list_supplier.dart`: minor refactoring * `keywords_product_query.dart`: moved; added language and country to product list key * `main.dart`: minor refactoring * `new_product_page.dart`: minor refactoring * `nutrition_page_loaded.dart`: minor refactoring * `ocr_helper.dart`: minor refactoring * `onboarding_data_product.dart`: minor refactoring * `ordered_nutrients_cache.dart`: minor refactoring * `paged_product_query.dart`: moved; added language and country to product list key; refactored around `getProducts` * `paged_search_product_query.dart`: moved; refactored * `paged_to_be_completed_product_query.dart`: moved; added language and country to product list key; refactored * `paged_user_product_query.dart`: moved; added language to product list key; refactored * `picture_capture_helper.dart`: minor refactoring * `product_dialog_helper.dart`: minor refactoring * `product_list.dart`: added optional language and country to constructor parameters * `product_list_import_export.dart`: minor refactoring * `product_list_page.dart`: minor refactoring * `product_list_supplier.dart`: minor refactoring * `product_query.dart`: moved; not an interface anymore * `product_query_model.dart`: minor refactoring * `product_query_page_helper.dart`: minor refactoring * `product_refresher.dart`: minor refactoring * `query_product_list_supplier.dart`: minor refactoring * `robotoff_insight_helper.dart`: minor refactoring * `robotoff_questions_query.dart`: moved; minor refactoring * `search_page.dart`: minor refactoring * `simple_input_page_helpers.dart`: minor refactoring * `summary_card.dart`: minor refactoring * `user_preferences_account.dart`: minor refactoring * `user_preferences_dev_mode.dart`: minor refactoring
Codecov Report
@@ Coverage Diff @@
## develop #2465 +/- ##
==========================================
- Coverage 8.86% 7.49% -1.38%
==========================================
Files 161 205 +44
Lines 6623 9825 +3202
==========================================
+ Hits 587 736 +149
- Misses 6036 9089 +3053
Continue to review full report at Codecov.
|
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.
Looks great, thanks @monsiertarnuki
CC: @AshAman999 probably interesting for you
Thank you @M123-dev for your review! |
@AshAman999 is the right one to ask but as the ultimate goal is to be able to allow multiple different datasets for countries that's probably a good idea + a a own field for language to be able to explicitly get data for a single data set. "German" Similar to Google Maps but don't take my word for granted let's wait for @AshAman999 s answer |
Though I am not an expert here dealing with db's, but i am thinking of making multiple tables according to the desired country and language. Thoughts @monsieurtanuki @M123-dev , |
1. It's not a good idea. The correct way to do that is by adding a language and a country column. Another way would be to create "partitions", but I don't think SQLite supports them.
2. I don't think you can even do that with dynamic languages and countries in sqflite, given that each table change means a new database version. If you knew from the start that we need tables product_en_us, product_fr_fr and product_it_it, that would be thinkable. But that's not the case.
3. Maintenance would be hard because you would have to do all your table changes to all product tables.
4. I don't believe we need that for the moment.
5. Given that SQL is not something you're very comfortable with, I would suggest to go step by step and first develop something that works. And then make it evolve with the needs. If you try to make everything at the same time you add confusion in your code - not good for you, not good for the reviewers.
6. That was definitely a good idea to ask ;)
…________________________________
De : Aman Raj ***@***.***>
Envoyé : lundi 4 juillet 2022 08:08
À : openfoodfacts/smooth-app ***@***.***>
Cc : monsieurtanuki ***@***.***>; Mention ***@***.***>
Objet : Re: [openfoodfacts/smooth-app] feat: #2339 - added language and country to query product list keys (PR #2465)
Though I am not an expert here dealing with db's, but i am thinking of making multiple tables according to the desired country and language.
So the current table name is String TABLE_PRODUCT = 'product';
What i am thinking here is to do something like this for a table name String TABLE_PRODUCT = 'product_$language_$country';
In this process i could seperate the products for different coutries and language according to the need here.
Thoughts @monsieurtanuki<https://github.com/monsieurtanuki> @M123-dev<https://github.com/M123-dev> ,
Would be great to hear yours answer on this as well
—
Reply to this email directly, view it on GitHub<#2465 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ACYKI34TXFXHMVGQNYKMA7TVSJ5V5ANCNFSM52MSB7XQ>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
What
Impacted files:
abstract_onboarding_data.dart
: minor refactoringbarcode_product_query.dart
: moved; minor refactoringcategory_product_query.dart
: moved; added language and country to product list keycontinuous_scan_model.dart
: minor refactoringcountry_selector.dart
: minor refactoringdatabase_product_list_supplier.dart
: minor refactoringkeywords_product_query.dart
: moved; added language and country to product list keymain.dart
: minor refactoringnew_product_page.dart
: minor refactoringnutrition_page_loaded.dart
: minor refactoringocr_helper.dart
: minor refactoringonboarding_data_product.dart
: minor refactoringordered_nutrients_cache.dart
: minor refactoringpaged_product_query.dart
: moved; added language and country to product list key; refactored aroundgetProducts
paged_search_product_query.dart
: moved; refactoredpaged_to_be_completed_product_query.dart
: moved; added language and country to product list key; refactoredpaged_user_product_query.dart
: moved; added language to product list key; refactoredpicture_capture_helper.dart
: minor refactoringproduct_dialog_helper.dart
: minor refactoringproduct_list.dart
: added optional language and country to constructor parametersproduct_list_import_export.dart
: minor refactoringproduct_list_page.dart
: minor refactoringproduct_list_supplier.dart
: minor refactoringproduct_query.dart
: moved; not an interface anymoreproduct_query_model.dart
: minor refactoringproduct_query_page_helper.dart
: minor refactoringproduct_refresher.dart
: minor refactoringquery_product_list_supplier.dart
: minor refactoringrobotoff_insight_helper.dart
: minor refactoringrobotoff_questions_query.dart
: moved; minor refactoringsearch_page.dart
: minor refactoringsimple_input_page_helpers.dart
: minor refactoringsummary_card.dart
: minor refactoringuser_preferences_account.dart
: minor refactoringuser_preferences_dev_mode.dart
: minor refactoringPart of