feat: Support for deep links #3995

g123k · 2023-05-22T08:26:32Z

Hi everyone,

This PR allows the app to support deep links (e.g.: clicking on a link in a search result and being redirected to the appropriate screen). We have decided to only support two routes: a product and the homepage.

A product will be opened if:

The url has at least two parts
The barcode contains at least 8 digits

Some examples: product/123456789 or produit/3017620422003/nutella-ferrero

Implementation

We now use the Navigator 2 (or router) in the app, with the help of go_router.
If this is the first time the user opens the app, the link will be ignored and, in general, if the onboarding is not successfully achieved, all links are ignored.

The redirect method of GoRouter is called both for internal routes and external ones (= url), that's why internal routes are prefixed with an underscore.

Product loader

The ProductDetails screen requires it to be in the database. That's why a ProductLoader will load all the required stuff on the server and then show the details. Obviously, products not found, and network errors are handled.

External links

A deep link can be called with an unsupported path (e.g.: /contact). In that case, we will redirect to the browser, through the ExternalPage.

Migration from the Navigator

Instead of calling Navigator.of(context), we now have to use AppNavigator.of(context) which embeds our GoRouter implementation.

Notes

I have not migrated how the onboarding, tabs and preferences handle their navigation, as it's not mandatory.

How to test it?

On Android, you can use this ADB command:

adb shell 'am start -a android.intent.action.VIEW -c android.intent.category.BROWSABLE -d "https://al.openfoodfacts.org/"' org.openfoodfacts.scanner

codecov-commenter · 2023-05-22T08:33:11Z

Codecov Report

Merging #3995 (1431606) into develop (76762b1) will decrease coverage by 0.20%.
The diff coverage is 0.38%.

@@             Coverage Diff             @@
##           develop    #3995      +/-   ##
===========================================
- Coverage    10.96%   10.77%   -0.20%     
===========================================
  Files          265      270       +5     
  Lines        13090    13322     +232     
===========================================
  Hits          1435     1435              
- Misses       11655    11887     +232

Impacted Files	Coverage Δ
...cards/product_cards/smooth_product_card_found.dart	`0.00% <0.00%> (ø)`
...s/product_cards/smooth_product_card_not_found.dart	`0.00% <0.00%> (ø)`
...kages/smooth_app/lib/helpers/analytics_helper.dart	`0.00% <ø> (ø)`
...ooth_app/lib/helpers/extension_on_text_helper.dart	`40.90% <0.00%> (-12.04%)`	⬇️
packages/smooth_app/lib/main.dart	`13.11% <0.00%> (+0.23%)`	⬆️
...s/smooth_app/lib/pages/inherited_data_manager.dart	`0.00% <0.00%> (ø)`
.../smooth_app/lib/pages/navigator/app_navigator.dart	`0.00% <0.00%> (ø)`
...ges/smooth_app/lib/pages/navigator/error_page.dart	`0.00% <0.00%> (ø)`
.../smooth_app/lib/pages/navigator/external_page.dart	`0.00% <0.00%> (ø)`
...p/lib/pages/preferences/user_preferences_page.dart	`93.82% <ø> (ø)`
... and 8 more

... and 4 files with indirect coverage changes

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

monsieurtanuki

Hi @g123k!
For the record I have no previous experience with deep-links.
Please have a look at my comments.
The most important thing is that you probably confused two pages: "add new product" page and "display product" page. I assume that "add new product" page shouldn't be dealt with here.

packages/smooth_app/android/app/src/main/AndroidManifest.xml

packages/smooth_app/ios/Runner/Runner.entitlements

packages/smooth_app/lib/cards/product_cards/smooth_product_card_found.dart

monsieurtanuki · 2023-05-22T08:40:06Z

packages/smooth_app/lib/cards/product_cards/smooth_product_card_not_found.dart

+                AppNavigator.of(context).push(
+                  AppRoutes.PRODUCT_CREATOR(barcode),
                );
-                if (callback != null) {
-                  await callback!();
-                }
+
+                // TODO(g123k): Find another way
+                // if (callback != null) {
+                //   await callback!();
+                // }


I think you're making a mistake here (which explains your TODO): here we open the page where you edit a new product. We don't open the ("new") product page.
Therefore I think you should remove your changes on that file.

PRODUCT_CREATOR actually opens AddNewProductPage.
The only problem we have now is that the push method doesn't return any result.

PRODUCT_CREATOR actually opens AddNewProductPage. The only problem we have now is that the push method doesn't return any result.

Then perhaps the concept of PRODUCT_CREATOR is flawed, or at least we should put it on the shelf.
To me the use-case is a user browsing and clicking on a link that automagically opens the app. A link that opens a known product, I can understand. A link that opens the search, I can understand. A link that opens an edit page for an unknown barcode, given that we scan on the app and we search on the app, I cannot quite picture that.
That said, perhaps the callback (that we actually set only once in that case) is redundant with whatever we do at the end of the AddNewProductPage, to be investigated.

But in general, I think that we assumed a long time ago that the deep-links were rather "read/display" pages than "write/edit" pages, specifically because they don't return values. Maybe it's not relevant, after all.

I understand your point, but the routes in GoRouter, doesn't necessary mean they have to be opened via a deep link. In that case, PRODUCT_CREATOR is just a named route, as we can find in the Navigator v1.

I have handled the case where a product doesn't exist, as it may happen.
Let's say you receive a link by email and the sender has made a typo => we have to do something for that.

I understand your point, but the routes in GoRouter, doesn't necessary mean they have to be opened via a deep link. In that case, PRODUCT_CREATOR is just a named route, as we can find in the Navigator v1.

Ok, but in that case if we want to handle the callback that you commented/TODOed, perhaps we would be better off making this page a full screen dialog instead of a page.

I have handled the case where a product doesn't exist, as it may happen. Let's say you receive a link by email and the sender has made a typo => we have to do something for that.

In that case I would rather land on a "The product doesn't exist" page (with a "do you want to create it?" button).
If not we somehow ask users to create new products with wrong barcodes!

For the first point, OK I will revert what(s done here
Mmm for the second point, I let @teolemon what's the best implementation to use.

@monsieutanuki @g123k Mmh.. I can see some potential abuse potential but also the ability for third party apps to delegate some of their duties to Open Food Facts when a product is missing (some are currently using a web link)
I'd be in favor of giving the product addition approach a try, and kill it if we get abuse

Ok, I let the feature as is.

Are we able as of now to detect from where the Product Creation page was opened?
Or should I add an Analytics event?

I don't believe we are. We can open it from scan, search, and now deep links, right ?

Yes, exactly. Is-it useful to add it, or should I leave it as is?

packages/smooth_app/lib/helpers/extension_on_text_helper.dart

packages/smooth_app/lib/pages/product/product_loader_page.dart

monsieurtanuki · 2023-05-22T09:01:57Z

packages/smooth_app/lib/pages/product/summary_card.dart

-                        ),
-                      ),
+                ? () => AppNavigator.of(context).push(
+                      AppRoutes.PREFERENCES(PreferencePageType.FOOD),


Just checking: is it possible for the user to say, from the browser, "Ok you can send me to the app but this time I prefer to stay on the website"?

Unfortunately, no (both on Android and iOS)

That means we'd better check if we have all the major features on the app, compared to the website on smartphone.
In order to avoid user frustration.

We're at feature parity, but some (hard to implement vs ROI) stuff is missing (like revision history).
Simple solution: on the product page, we could add an optional button to open the classic webpage in a webview.

We're at feature parity, but some (hard to implement vs ROI) stuff is missing (like revision history). Simple solution: on the product page, we could add an optional button to open the classic webpage in a webview.

I guess a button at the end of the product page would do the trick. Assuming we can exit the app using a URL the app is supposed to catch ;)

monsieurtanuki · 2023-05-22T09:04:58Z

packages/smooth_app/lib/pages/navigator/app_navigator.dart

+// TODO(g123k): Improve this with sealed classes (Dart 3 is required)
+// ignore_for_file: non_constant_identifier_names


Looks like in dart the constants should rather be written in camelCase. Is that the same with sealed classes?

Sealed classes are actually like regular classes.
This is just a temporary way to distinguish a simple getter from this.
I know this isn't perfect

monsieurtanuki · 2023-05-22T09:07:23Z

packages/smooth_app/lib/pages/navigator/app_navigator.dart

+  AppRoutes._();
+
+  // Home page (or walkthrough during the onboarding)
+  static const String HOME = _SmoothGoRouter.HOME_PAGE;
+
+  // Product details (a [Product] is mandatory in the extra)
+  static String PRODUCT(String barcode) =>
+      '/${_SmoothGoRouter.PRODUCT_DETAILS_PAGE}/$barcode';
+


Suggested change

AppRoutes._();

// Home page (or walkthrough during the onboarding)

static const String HOME = _SmoothGoRouter.HOME_PAGE;

// Product details (a [Product] is mandatory in the extra)

static String PRODUCT(String barcode) =>

'/${_SmoothGoRouter.PRODUCT_DETAILS_PAGE}/$barcode';

final String path;

// Home page (or walkthrough during the onboarding)

AppRoutes.home() : path = _SmoothGoRouter.HOME_PAGE;

// Product details (a [Product] is mandatory in the extra)

AppRoutes.product(String barcode) : path =

'/${_SmoothGoRouter.PRODUCT_DETAILS_PAGE}/$barcode';

Would look better that way, I think.

That would be better, but it means that each entity contains an attribute for nothing.
And what should we do if you we want no one, but two elements?

That would be better, but it means that each entity contains an attribute for nothing.

The attribute being the String that you return, right? I don't see it as a problem, I see it as OOP.

And what should we do if you we want no one, but two elements?

Then with my OOP solution we can do it, like AppRoutes.product(String barcode, Color color).

In your second example, you would still have only the path and merge the two data in the construct you mean?

packages/smooth_app/lib/pages/navigator/app_navigator.dart

Co-authored-by: monsieurtanuki <fabrice_fontaine@hotmail.com>

packages/smooth_app/lib/pages/navigator/app_navigator.dart

teolemon · 2023-05-23T04:31:30Z

@monsieurtanuki @g123k

I'd like this merged and deployed quickly, at least on Android.
There's no path dependancy (we can iterate on the feature in subsequent releases without regrets on corner cutting done now)
There's a monthly 2.7 million visitors conversion opportunity from the website

Co-authored-by: Pierre Slamich <pierre.slamich@gmail.com>

…ound card

monsieurtanuki · 2023-05-23T12:32:02Z

I'd like this merged and deployed quickly, at least on Android.

@teolemon If you mean for a prod release, I guess we should list the other issues we would like to be fixed beforehand.
For a beta release, I have no strong objection against the current PR, and I've already listed my remarks.
In both cases, for the record I'll have limited time for Smoothie in the next 10 days. And after it's Roland Garros ;)

teolemon · 2023-05-23T13:04:32Z

ok, @monsieurtanuki @g123k let's merge as is ?

g123k added 8 commits May 19, 2023 22:28

Support for deep links on Android

ef08bd1

Deep links: Better support for network errors / unknown product

0b6c011

iOS configuration for deep links

7f59d6e

Improve documentation

d1c41ba

Improve doc

d3240ff

Support other domains

4f458ce

Add an error page

c3a0e96

Upgrade to go_router 7.0.2

87d445a

g123k added 🥫 Product page 🔗 Deeplinking Ensure people browsing Open Food Facts on the mobile web have a bridge to the mobile apps labels May 22, 2023

g123k requested a review from a team as a code owner May 22, 2023 08:26

g123k self-assigned this May 22, 2023

Add some Sentry events

ec186cc

github-actions bot added the 📈 Analytics We use Sentry and Matomo, with an opt-in system label May 22, 2023

g123k linked an issue May 22, 2023 that may be closed by this pull request

Handle openfoodfacts.org links (Add deeplinking towards the product page) #920

Closed

17 tasks

monsieurtanuki reviewed May 22, 2023

View reviewed changes

g123k and others added 5 commits May 22, 2023 11:23

Update packages/smooth_app/lib/helpers/extension_on_text_helper.dart

7aa416b

Co-authored-by: monsieurtanuki <fabrice_fontaine@hotmail.com>

SizedBox -> EMPTY_WIDGET

e611c3b

PreferencePageType: from name to tag

ab7c1c9

Remove an unused async keyword

716a025

Move internal app routes to an enum

1431606

Add a clarification about ProductLoaderPage

19eaf62

teolemon reviewed May 23, 2023

View reviewed changes

packages/smooth_app/lib/pages/navigator/app_navigator.dart Outdated Show resolved Hide resolved

g123k and others added 2 commits May 23, 2023 08:55

Update packages/smooth_app/lib/pages/navigator/app_navigator.dart

632fea9

Co-authored-by: Pierre Slamich <pierre.slamich@gmail.com>

Revert how we open the product addition/creation on the product not f…

c380cae

…ound card

teolemon merged commit 4c44b06 into openfoodfacts:develop May 23, 2023

github-actions bot mentioned this pull request May 23, 2023

chore(develop): release 4.8.0 #3937

Merged

g123k deleted the deep_links branch May 23, 2023 14:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Support for deep links #3995

feat: Support for deep links #3995

g123k commented May 22, 2023 •

edited

Loading

codecov-commenter commented May 22, 2023 •

edited

Loading

monsieurtanuki left a comment

monsieurtanuki May 22, 2023

g123k May 22, 2023

monsieurtanuki May 22, 2023

g123k May 22, 2023

monsieurtanuki May 22, 2023

g123k May 23, 2023

teolemon May 23, 2023 •

edited

Loading

g123k May 23, 2023

teolemon May 23, 2023

g123k May 23, 2023

monsieurtanuki May 22, 2023

g123k May 22, 2023

monsieurtanuki May 22, 2023

teolemon May 22, 2023

monsieurtanuki May 23, 2023

monsieurtanuki May 22, 2023

g123k May 22, 2023

monsieurtanuki May 22, 2023

g123k May 22, 2023

monsieurtanuki May 22, 2023

g123k May 23, 2023

teolemon commented May 23, 2023

monsieurtanuki commented May 23, 2023

teolemon commented May 23, 2023

		// TODO(g123k): Improve this with sealed classes (Dart 3 is required)
		// ignore_for_file: non_constant_identifier_names

feat: Support for deep links #3995

feat: Support for deep links #3995

Conversation

g123k commented May 22, 2023 • edited Loading

Implementation

Product loader

External links

Migration from the Navigator

Notes

How to test it?

codecov-commenter commented May 22, 2023 • edited Loading

Codecov Report

monsieurtanuki left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

teolemon May 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

teolemon commented May 23, 2023

monsieurtanuki commented May 23, 2023

teolemon commented May 23, 2023

g123k commented May 22, 2023 •

edited

Loading

codecov-commenter commented May 22, 2023 •

edited

Loading

teolemon May 23, 2023 •

edited

Loading