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

fix: Exception when product brands are null #4699

Conversation

WildOrangutan
Copy link
Contributor

@WildOrangutan WildOrangutan commented Oct 5, 2023

What

Fixing some null exception, that I randomly found while working on other issues, due to this bang operator: widget.product.brands!

Fixes bug(s)

Fixes #4692, #4693, #4694

@codecov-commenter
Copy link

codecov-commenter commented Oct 5, 2023

Codecov Report

Merging #4699 (3565a4d) into develop (8ac2fa9) will increase coverage by 0.00%.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##           develop   #4699   +/-   ##
=======================================
  Coverage     9.90%   9.90%           
=======================================
  Files          310     310           
  Lines        15815   15808    -7     
=======================================
  Hits          1566    1566           
+ Misses       14249   14242    -7     
Files Coverage Δ
..._app/lib/pages/product/add_other_details_page.dart 0.00% <0.00%> (ø)
...h_app/lib/pages/product/nutrition_page_loaded.dart 0.00% <0.00%> (ø)
.../lib/pages/product/product_image_gallery_view.dart 0.00% <0.00%> (ø)
...mooth_app/lib/pages/product/simple_input_page.dart 0.00% <0.00%> (ø)
..._app/lib/pages/product/add_basic_details_page.dart 0.00% <0.00%> (ø)
...oth_app/lib/pages/product/edit_new_packagings.dart 0.00% <0.00%> (ø)
...s/smooth_app/lib/helpers/product_cards_helper.dart 0.00% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@monsieurtanuki
Copy link
Contributor

@WildOrangutan Please fix and attach issues #4692 #4693 #4694.

@WildOrangutan
Copy link
Contributor Author

Hm, I see some issues are related to my fix. Ok, will refactor and also fix those.

@github-actions github-actions bot added ✏️ Editing - Nutrition input ✏️ Editing - Packaging input Related to the structured input of food packaging. labels Oct 5, 2023
Copy link
Contributor

@monsieurtanuki monsieurtanuki left a comment

Choose a reason for hiding this comment

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

Thank you @WildOrangutan for your PR!
We'll even be better off with a systematic Widget getProductTitle method.
Please have a look at my comments and tell me what you think.

@@ -11,14 +11,22 @@ import 'package:smooth_app/helpers/image_field_extension.dart';
import 'package:smooth_app/helpers/ui_helpers.dart';
import 'package:smooth_app/query/product_query.dart';

String getProductNameAndBrands(
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
String getProductNameAndBrands(
Widget getProductTitle(
final Product product,
final AppLocalizations appLocalizations,
) => Text(
getProductNameAndBrands(upToDateProduct, appLocalizations),
overflow: TextOverflow.ellipsis,
maxLines: 1,
);
String getProductNameAndBrands(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Named it buildProductTitle, so it's hopefully gives a hint, that it returns different type, compared to getProductName below.

@monsieurtanuki monsieurtanuki linked an issue Oct 6, 2023 that may be closed by this pull request
Copy link
Contributor

@monsieurtanuki monsieurtanuki left a comment

Choose a reason for hiding this comment

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

Thank you @WildOrangutan, looks good to me!

@monsieurtanuki monsieurtanuki merged commit 25068e5 into openfoodfacts:develop Oct 6, 2023
6 checks passed
@WildOrangutan WildOrangutan deleted the peter/fix/product-brands-null branch October 6, 2023 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment