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

feat: 4947 - added "download language" to product table #4951

Merged
merged 1 commit into from
Jan 7, 2024

Conversation

monsieurtanuki
Copy link
Contributor

What

  • Now, each time we download one or several products, we store the download language too.
  • "This product was downloaded using an API query with Italian as the language".
  • With this additional metadata, we'll be able to refresh the local database on specific products for localization reasons like "only products downloaded in a different language".
    • either as a silent background task ("just refresh all the products that are not in that language")
    • or as user hint when landing on the product page ("hey, this product was downloaded in Italian, would you like to refresh it in English?")
    • or as user hint when landing on the product list page ("hey, several products on this page were downloaded in Italian, would you like to refresh the page in English?")
    • or a combination of all this

Part of

Files

Deleted file:

  • dao_product_migration.dart

Impacted files:

  • background_task_download_products.dart: added language parameter
  • dao_hive_product.dart: removed dead code
  • dao_product.dart: added table column language lc
  • local_database.dart: upgraded the database version
  • onboarding_data_product.dart: added language parameter
  • product_list_page.dart: added language parameter
  • product_refresher.dart: added language parameter
  • query_product_list_supplier.dart: added language parameter

Deleted file:
* `dao_product_migration.dart`

Impacted files:
* `background_task_download_products.dart`: added language parameter
* `dao_hive_product.dart`: removed dead code
* `dao_product.dart`: added table column language `lc`
* `local_database.dart`: upgraded the database version
* `onboarding_data_product.dart`: added language parameter
* `product_list_page.dart`: added language parameter
* `product_refresher.dart`: added language parameter
* `query_product_list_supplier.dart`: added language parameter
@codecov-commenter
Copy link

Codecov Report

Attention: 23 lines in your changes are missing coverage. Please review.

Comparison is base (72ac084) 9.65% compared to head (37620a8) 9.66%.

Files Patch % Lines
packages/smooth_app/lib/database/dao_product.dart 0.00% 8 Missing ⚠️
...pp/lib/pages/product/common/product_refresher.dart 0.00% 6 Missing ⚠️
...p/lib/data_models/query_product_list_supplier.dart 0.00% 3 Missing ⚠️
...pp/lib/pages/product/common/product_list_page.dart 0.00% 3 Missing ⚠️
.../background/background_task_download_products.dart 0.00% 2 Missing ⚠️
...h_app/lib/data_models/onboarding_data_product.dart 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           develop   #4951      +/-   ##
==========================================
+ Coverage     9.65%   9.66%   +0.01%     
==========================================
  Files          323     322       -1     
  Lines        16207   16175      -32     
==========================================
  Hits          1564    1564              
+ Misses       14643   14611      -32     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@M123-dev M123-dev left a comment

Choose a reason for hiding this comment

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

Thanks a lot @monsieurtanuki

Comment on lines -38 to -81

LazyBox<Product> _getBox() => Hive.lazyBox<Product>(_hiveBoxName);

Future<Product?> get(final String barcode) async => _getBox().get(barcode);

@override
Future<Map<String, Product>> getAll(final List<String> barcodes) async {
final LazyBox<Product> box = _getBox();
final Map<String, Product> result = <String, Product>{};
for (final String barcode in barcodes) {
final Product? product = await box.get(barcode);
if (product != null) {
result[barcode] = product;
}
}
return result;
}

Future<void> put(final Product product) async => putAll(<Product>[product]);

Future<void> putAll(final Iterable<Product> products) async {
final Map<String, Product> upserts = <String, Product>{};
for (final Product product in products) {
upserts[product.barcode!] = product;
}
await _getBox().putAll(upserts);
}

@override
Future<List<String>> getAllKeys() async {
final LazyBox<Product> box = _getBox();
final List<String> result = <String>[];
for (final dynamic key in box.keys) {
result.add(key.toString());
}
return result;
}

// Just for the migration
@override
Future<void> deleteAll(final List<String> barcodes) async {
final LazyBox<Product> box = _getBox();
await box.deleteAll(barcodes);
}
Copy link
Member

Choose a reason for hiding this comment

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

Did you test if this part removal will work or also break the whole app, nontheless we should keep an eye out in the beta

@monsieurtanuki monsieurtanuki merged commit e0b3111 into openfoodfacts:develop Jan 7, 2024
6 checks passed
@monsieurtanuki
Copy link
Contributor Author

monsieurtanuki commented Jan 7, 2024

Thank you @M123-dev for your review!
I understand your concern about dead code removal when it comes to hive. I've removed code that is not "hive native" code, I've managed to upgrade an existing app and to run an app from scratch. Therefore I'm rather confident.
But you're right, we should be cautious.
For the record, it was not (only) for revenge that I removed that code, but because the notion of "product download language" didn't exist back then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants