Support Order Details deep link

[wzieba]

Closes: #7323

Description

Testing instructions

Images/gif

• [ ] I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

[wpmobilebot]

You can test the changes on this Pull Request by downloading an installable build, or scanning this QR code:

[codecov-commenter]

Codecov Report

Base: 44.22% // Head: 44.24% // Increases project coverage by +0.02% :tada:

Coverage data is based on head (ddbb444) compared to base (25d37aa). Patch coverage: 66.66% of modified lines in pull request are covered.

:exclamation: Current head ddbb444 differs from pull request most recent head 4d7f188. Consider uploading reports for the commit 4d7f188 to get more accurate results

Additional details and impacted files

@@             Coverage Diff              @@
##              trunk    #7386      +/-   ##
============================================
+ Coverage     44.22%   44.24%   +0.02%     
- Complexity     3289     3299      +10     
============================================
  Files           584      585       +1     
  Lines         33020    33067      +47     
  Branches       4276     4284       +8     
============================================
+ Hits          14603    14632      +29     
- Misses        17091    17104      +13     
- Partials       1326     1331       +5     

Impacted Files	Coverage Δ
.../woocommerce/android/analytics/AnalyticsTracker.kt	20.76% <ø> (ø)
...ocommerce/android/ui/main/MainActivityViewModel.kt	79.80% <47.36%> (-8.11%)	:arrow_down:
.../com/woocommerce/android/ui/main/ResolveAppLink.kt	75.00% <75.00%> (ø)
...om/woocommerce/android/analytics/AnalyticsEvent.kt	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[nbradbury]

@wzieba I'm seeing some unexpected behavior here. I tried the first test while logged into site B, and it correctly switched to site A and took me to the order. But then I tried to switch back to site B and it took me to that site A order again.

Update: Perhaps this is due to using a debug build? I was unable to build the release variant, so perhaps you can DM me the APK?

[nbradbury]

@wzieba For test #5, I'm consistently taken to an empty order screen.

[peril-woocommerce[bot]]

	Warnings
:warning:	PR is not assigned to a milestone.

	Messages
:book:	This PR contains changes to Tracks-related logic. Please ensure the following are completed: PR Author The PR must be assigned the Tracks label PR Reviewer The tracks events must be validated in the Tracks system. Verify the internal tracks spreadsheet has also been updated.

Generated by :no_entry_sign: dangerJS

[wzieba]

Thank you for the review @nbradbury !

I tried the first test while logged into site B, and it correctly switched to site A and took me to the order. But then I tried to switch back to site B and it took me to that site A order again.

I've tried reproducing it but couldn't (or maybe I'm doing something different?)

https://user-images.githubusercontent.com/5845095/193873745-ae9ca30f-0669-4745-a1bb-a9bc473ec123.mov

For test https://github.com/woocommerce/woocommerce-android/issues/5, I'm consistently taken to an empty order screen.

Sorry, I got too "creative" when writing tests 🤦 . It's an invalid case.

It works as it should - we don't check in the database if an Order with id from URL parameters exists. There's no need - the id is provided from the core. Even if Order not yet fetched but exists in the core, it will be fetched after opening the screen.

[nbradbury]

@wzieba I can still repro this in the release build. Here's a video so you see the steps:

device-2022-10-04-131837.webm

[wzieba]

Gotcha, @nbradbury, I misunderstood your first comment. I can reproduce the issue now, thank you for the video. I'll work on the fix and ping you.

[wzieba]

I've found the issue with tasks + nullified the intent data as you suggested.

Before the change, WooCommerce activity was launched in scope from which deep link was invoked. So in the sample above, in the scope of Keep Notes, next to the "original" instance of just WooCommerce.

After 4d7f188, WooCommerce will always be invoked in its own task. If the task is running, we'll refer to active instance (hence I had to edit onNewIntent - before, every time Android created a new activity so onCreate was called).

But this change led to the discovery of another bug, which I described in a separate issue as it also relates to Notifications: #7483 . This makes 1 and 2 tests not valid anymore, unless every time after opening order details, you'll close them before running a new deep link.

---

Overall, I think that PR could be merged, even with this bug, as it's not critical IMO. I'll work on a fix in a separate PR, and I believe it can land even in another release. I think that the basic "open order details" case could work fine now. What's your opinion on that @nbradbury ?