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.

Sign up for GitHub

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

fix: #2146 refresh product edition #2201

Merged
merged 10 commits into from
Jun 10, 2022
Merged

fix: #2146 refresh product edition #2201

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
f43720d
fix: #2146 refresh product edition
cli1005 Jun 7, 2022
853ad17
fix: #2146 refresh product edition photo editing
cli1005 Jun 7, 2022
42e3db6
fix: #2146 refactoring
cli1005 Jun 7, 2022
59986c0
fix: fix presubmit test
cli1005 Jun 7, 2022
4865c66
fix: fix presubmit
cli1005 Jun 7, 2022
1af30a0
Merge branch 'develop' into bug/2146-refresh-product-edition
cli1005 Jun 7, 2022
2714bcd
Merge branch 'develop' into bug/2146-refresh-product-edition
cli1005 Jun 9, 2022
e718072
fix: #2146 refresh product edition v2
cli1005 Jun 9, 2022
c2b3471
fix: #2146 fix presubmit check
cli1005 Jun 9, 2022
116bf66
fix: #2146 refactoring v3
cli1005 Jun 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 14 additions & 32 deletions packages/smooth_app/lib/pages/product/add_basic_details_page.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,11 @@ import 'package:flutter_gen/gen_l10n/app_localizations.dart';
import 'package:openfoodfacts/openfoodfacts.dart';
import 'package:provider/provider.dart';
import 'package:smooth_app/cards/product_cards/product_image_carousel.dart';
import 'package:smooth_app/database/dao_product.dart';
import 'package:smooth_app/database/local_database.dart';
import 'package:smooth_app/database/product_query.dart';
import 'package:smooth_app/generic_lib/design_constants.dart';
import 'package:smooth_app/generic_lib/dialogs/smooth_alert_dialog.dart';
import 'package:smooth_app/generic_lib/loading_dialog.dart';
import 'package:smooth_app/generic_lib/widgets/smooth_text_form_field.dart';
import 'package:smooth_app/pages/product/common/product_refresher.dart';

class AddBasicDetailsPage extends StatefulWidget {
const AddBasicDetailsPage(this.product);
Expand Down Expand Up @@ -130,7 +128,7 @@ class _AddBasicDetailsPageState extends State<AddBasicDetailsPage> {
return;
}
final bool savedAndRefreshed =
await _saveData(localDatabase);
await _saveData(localDatabase, widget.product);
if (savedAndRefreshed) {
if (!mounted) {
return;
Expand Down Expand Up @@ -168,39 +166,23 @@ class _AddBasicDetailsPageState extends State<AddBasicDetailsPage> {
),
);

Future<bool> _saveData(LocalDatabase localDatabase) async {
Future<bool> _saveData(
Copy link
Contributor

Choose a reason for hiding this comment

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

You should probably return a Product?.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You should probably return a Product?.

✅

LocalDatabase localDatabase, Product inputProduct) async {
Copy link
Contributor

Choose a reason for hiding this comment

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

First thing first, please add a ',' after inputProduct.

Then, I would not recommend using the inputProduct, because you're changing it. You should start from scratch with a new Product().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your suggestion, I removed the parameter inputProduct

final AppLocalizations appLocalizations = AppLocalizations.of(context);
final Product product = Product(
productName: _productNameController.text,
quantity: _weightController.text,
brands: _brandNameController.text,
barcode: widget.product.barcode,
);
final Status? status = await LoadingDialog.run<Status>(
inputProduct.productName = _productNameController.text;
inputProduct.quantity = _weightController.text;
inputProduct.brands = _brandNameController.text;
inputProduct.barcode = widget.product.barcode;
final Product? savedAndRefreshed = await ProductRefresher().saveAndRefresh(
context: context,
future: OpenFoodAPIClient.saveProduct(
ProductQuery.getUser(),
product,
),
title: appLocalizations.nutrition_page_update_running,
localDatabase: localDatabase,
product: inputProduct,
);
if (status == null || status.error != null) {
if (savedAndRefreshed != null) {
return true;
} else {
_errorMessageAlert(appLocalizations.basic_details_add_error);
Copy link
Contributor

Choose a reason for hiding this comment

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

There's already an error message in saveAndRefresh. If you don't like the current error message that's normal, we have the make it a parameter in saveAndRefresh.

return false;
} else {
final ProductQueryConfiguration configuration = ProductQueryConfiguration(
widget.product.barcode!,
fields: ProductQuery.fields,
language: ProductQuery.getLanguage(),
country: ProductQuery.getCountry(),
);
final ProductResult result = await OpenFoodAPIClient.getProduct(
configuration,
);
if (result.product != null) {
await DaoProduct(localDatabase).put(result.product!);
}
}
return true;
}
}
23 changes: 12 additions & 11 deletions packages/smooth_app/lib/pages/product/common/product_refresher.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,28 +119,29 @@ class ProductRefresher {
return const _MetaProductRefresher.error(null);
}

Future<bool> fetchAndRefresh({
Future<Product?> fetchAndRefresh({
required final BuildContext context,
required final LocalDatabase localDatabase,
required final String barcode,
}) async {
final AppLocalizations appLocalizations = AppLocalizations.of(context);
final bool? savedAndRefreshed = await LoadingDialog.run<bool>(
final _MetaProductRefresher? fetchAndRefreshed =
await LoadingDialog.run<_MetaProductRefresher>(
future: _fetchAndRefresh(localDatabase, barcode),
context: context,
title: appLocalizations.nutrition_page_update_running,
);
if (savedAndRefreshed == null) {
return false;
if (fetchAndRefreshed == null) {
return null;
}
if (!savedAndRefreshed) {
if (fetchAndRefreshed.product == null) {
await LoadingDialog.error(context: context);
return false;
return null;
}
return true;
return fetchAndRefreshed.product;
}

Future<bool> _fetchAndRefresh(
Future<_MetaProductRefresher> _fetchAndRefresh(
Copy link
Contributor

Choose a reason for hiding this comment

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

_saveAndRefresh should call this method instead of having to copy its code there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fetchAndRefresh calls this method instead

final LocalDatabase localDatabase,
final String barcode,
) async {
Expand All @@ -155,10 +156,10 @@ class ProductRefresher {
);
if (result.product != null) {
await DaoProduct(localDatabase).put(result.product!);
return true;
localDatabase.notifyListeners();
return _MetaProductRefresher.product(result.product);
}

return false;
return const _MetaProductRefresher.error(null);
}
}

Expand Down
29 changes: 15 additions & 14 deletions packages/smooth_app/lib/pages/product/edit_product_page.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import 'package:flutter_gen/gen_l10n/app_localizations.dart';
import 'package:openfoodfacts/openfoodfacts.dart';
import 'package:provider/provider.dart';
import 'package:smooth_app/data_models/product_image_data.dart';
import 'package:smooth_app/database/dao_product.dart';
import 'package:smooth_app/database/local_database.dart';
import 'package:smooth_app/helpers/product_cards_helper.dart';
import 'package:smooth_app/pages/product/add_basic_details_page.dart';
Expand Down Expand Up @@ -86,7 +85,6 @@ class _EditProductPageState extends State<EditProductPage> {
);
if (refreshed ?? false) {
_changes++;
await _refreshProduct();
}
},
),
Expand All @@ -112,7 +110,21 @@ class _EditProductPageState extends State<EditProductPage> {
);
if (refreshed ?? false) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I know you didn't write it, but I'm sure refreshed == true is easier to read than refreshed ?? false.

_changes++;
await _refreshProduct();
//Refetch product if needed for new urls
Copy link
Contributor

Choose a reason for hiding this comment

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

Please be more generous in your comment, and explain why you do it there and not in the other places where you code _changes++;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added some more details

if (!mounted) {
return;
}
final LocalDatabase localDatabase =
context.read<LocalDatabase>();
final Product? refreshedProduct =
await ProductRefresher().fetchAndRefresh(
context: context,
localDatabase: localDatabase,
barcode: _product.barcode!,
);
if (refreshedProduct != null) {
_product = refreshedProduct;
}
}
},
),
Expand All @@ -135,7 +147,6 @@ class _EditProductPageState extends State<EditProductPage> {
);
if (refreshed ?? false) {
_changes++;
await _refreshProduct();
}
},
),
Expand Down Expand Up @@ -176,7 +187,6 @@ class _EditProductPageState extends State<EditProductPage> {
);
if (refreshed ?? false) {
_changes++;
await _refreshProduct();
}
},
),
Expand All @@ -186,15 +196,6 @@ class _EditProductPageState extends State<EditProductPage> {
);
}

Future<void> _refreshProduct() async {
final LocalDatabase localDatabase = context.read<LocalDatabase>();
final Product? refreshedProduct =
await DaoProduct(localDatabase).get(_product.barcode ?? '');
if (refreshedProduct != null) {
_product = refreshedProduct;
}
}

Widget _getSimpleListTileItem(final AbstractSimpleInputPageHelper helper) =>
_ListTitleItem(
title: helper.getTitle(),
Expand Down
6 changes: 6 additions & 0 deletions packages/smooth_app/lib/pages/product/nutrition_container.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,12 @@ class NutritionContainer {
servingSize: _servingSize,
);

void updateProduct(Product product) {
product.barcode = _barcode;
product.nutriments = _getNutriments();
product.servingSize = _servingSize;
}

/// Converts all the data to a [Nutriments].
Nutriments _getNutriments() {
/// Converts a (weight) value to grams (before sending a value to the BE)
Expand Down
4 changes: 2 additions & 2 deletions packages/smooth_app/lib/pages/product/nutrition_page_loaded.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,12 +442,12 @@ class _NutritionPageLoadedState extends State<NutritionPageLoaded> {
_nutritionContainer.setControllerText(key, controller.text);
}
// minimal product: we only want to save the nutrients
final Product inputProduct = _nutritionContainer.getProduct();
_nutritionContainer.updateProduct(widget.product);

final Product? savedAndRefreshed = await ProductRefresher().saveAndRefresh(
context: context,
localDatabase: localDatabase,
product: inputProduct,
product: widget.product,
);
if (savedAndRefreshed != null) {
if (!mounted) {
Expand Down
21 changes: 2 additions & 19 deletions packages/smooth_app/lib/pages/product/product_image_gallery_view.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import 'dart:io';
import 'package:flutter/material.dart';
import 'package:flutter_gen/gen_l10n/app_localizations.dart';
import 'package:http/http.dart' as http;
import 'package:openfoodfacts/model/Product.dart';
import 'package:path_provider/path_provider.dart';
import 'package:photo_view/photo_view.dart';
import 'package:photo_view/photo_view_gallery.dart';
Expand Down Expand Up @@ -286,16 +287,6 @@ class _ProductImageGalleryViewState extends State<ProductImageGalleryView> {
if (!mounted) {
return;
}
final LocalDatabase localDatabase =
context.read<LocalDatabase>();
await ProductRefresher().fetchAndRefresh(
context: context,
localDatabase: localDatabase,
barcode: widget.barcode!,
);
if (!mounted) {
return;
}
final AppLocalizations appLocalizations =
AppLocalizations.of(context);
final String message = getImageUploadedMessage(
Expand Down Expand Up @@ -356,15 +347,7 @@ class _ProductImageGalleryViewState extends State<ProductImageGalleryView> {
if (!mounted) {
return;
}
final LocalDatabase localDatabase = context.read<LocalDatabase>();
await ProductRefresher().fetchAndRefresh(
context: context,
localDatabase: localDatabase,
barcode: widget.barcode!,
);
if (!mounted) {
return;
}

setState(() {
allProductImageProviders[currentIndex] = FileImage(photoUploaded);
});
Expand Down