[HybridApp] App links - Blank state once the report modal is closed

m-natarajan commented 3 months ago

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Version Number: 9.0.27-0 Reproducible in staging?: Y Reproducible in production?: If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: @mountiny Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1725010261607519

Action Performed:

Pre-condition: Have hybrid app switched to NewDot Steps: 1 Clicking a deep link from email to hybrid app for a report that is not newDot compatible yet

This opens the report successfully
Close the modal
Expected Result:

The app should switch back to oldDot as we cannot see the report in NewDot

Actual Result:

Leaves the app in blank state once the modal is closed

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

Which of our officially supported platforms is this issue occurring on?

[ ] Android: Native
[ ] Android: mWeb Chrome
[x] iOS: Native
[ ] iOS: mWeb Safari
[ ] MacOS: Chrome / Safari
[ ] MacOS: Desktop

Screenshots/Videos

https://github.com/user-attachments/assets/997b9004-acad-4ff6-9743-41802b19b84e

https://github.com/user-attachments/assets/8006e7d0-8915-4814-a56f-b02f5ae26b45

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01a34c5e44750fa896
Upwork Job ID: 1831306673608185287
Last Price Increase: 2024-09-04
Automatic offers:

dukenv0307 | Reviewer | 104056620

Issue Owner

Current Issue Owner: @mallenexpensify / @bfitzexpensify

Issue Owner

Current Issue Owner: @mallenexpensify

melvin-bot[bot] commented 3 months ago

Triggered auto assignment to @miljakljajic (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

melvin-bot[bot] commented 2 months ago

Job added to Upwork: https://www.upwork.com/jobs/~01a34c5e44750fa896

melvin-bot[bot] commented 2 months ago

Triggered auto assignment to Contributor-plus team member for initial proposal review - @dukenv0307 (External)

melvin-bot[bot] commented 2 months ago

@miljakljajic, @dukenv0307 Eep! 4 days overdue now. Issues have feelings too...

miljakljajic commented 2 months ago

Waiting for proposals

trjExpensify commented 2 months ago

Thanks for the draftPR, @staszekscp! Who's taking over in your absence?

staszekscp commented 2 months ago

Depending on the amount of other issues it's going to be @war-in or @mateuuszzzzz! We'll talk internally today, and when the time comes, the lucky one will comment under the issue 😄

trjExpensify commented 2 months ago

Cool, cool.

melvin-bot[bot] commented 2 months ago

@miljakljajic, @dukenv0307 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

miljakljajic commented 2 months ago

Not overdue, PR is WIP

melvin-bot[bot] commented 2 months ago

@miljakljajic, @dukenv0307 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

melvin-bot[bot] commented 2 months ago

@miljakljajic @dukenv0307 this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

melvin-bot[bot] commented 2 months ago

@miljakljajic, @dukenv0307 12 days overdue. Walking. Toward. The. Light...

war-in commented 2 months ago

Hi 👋 I started digging into this one and have a question.

Which NewDot links do we want to support on HybridApp now? We have a couple of them defined here (for Android) but I wonder if we want to support all of them 🤔 Please let me know 🙏

cc @trjExpensify

trjExpensify commented 2 months ago

I think we should support all of the deep links we do on the standalone New Expensify app for consistency.

miljakljajic commented 2 months ago

@war-in I'll assign you if you're going to be working on this!

melvin-bot[bot] commented 2 months ago

📣 @dukenv0307 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job

melvin-bot[bot] commented 2 months ago

Triggered auto assignment to @mallenexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

miljakljajic commented 2 months ago

@mallenexpensify I'm heading out on parental leave so I will leave this one with you, thank you. @war-in is still investigating and will share a solution

war-in commented 2 months ago

Hi @trjExpensify 👋

Another question from me 🤓 We have a bunch of deeplinks defined for OD already and I'm not sure if all of them are supported in ND.

/indbox - should we redirect to /home?
/emailReportAction - it takes action as a parameter but it's set to approve or forward. Is there a similar flow in ND? 🤔 Or should we just redirect the user to the report?
/unlimited - I see that in OD it's used to upgrade a policy (correct me if I'm wrong 🙏) but doesn't take params so we don't know which policy
/v - it takes accountID and validateCode and is used to validate the created account. What should the ND flow look like here?

melvin-bot[bot] commented 2 months ago

@mallenexpensify, @war-in, @dukenv0307 Whoops! This issue is 2 days overdue. Let's get this updated quick!

trjExpensify commented 2 months ago

/indbox - should we redirect to /home? /emailReportAction - it takes action as a parameter but it's set to approve or forward. Is there a similar flow in ND? 🤔 Or should we just redirect the user to the report? /unlimited - I see that in OD it's used to upgrade a policy (correct me if I'm wrong 🙏) but doesn't take params so we don't know which policy

If these are all www.expensify.com deep links for OldApp, I think we should go to the OldDot experience in HybridApp. Then any of the new.expensify.com deep links go to the NewDot experience in HybridApp.

/v - it takes accountID and validateCode and is used to validate the created account. What should the ND flow look like here?

For this one, we already have different URLs for new.expensify.com and expensify.com to handle the validation flows depending on the platform it was initiated from, so I'd expect to just leave what we have in place in that regard. So anything new.expensify.com would deep link to the NewDot experience, and anything on expensify.com deep links to the OldDot experience on HybridApp.

Separately, but somewhat related, James in marketing is working on auditing a bunch of our email notifications that include links to have those switched out to point to the right experience depending on what experience is set. There are a bunch which aren't yet (i.e pointing to a setting page to add a billing card as a reminder), but we're progressing that elsewhere.

melvin-bot[bot] commented 2 months ago

@mallenexpensify, @war-in, @dukenv0307 Eep! 4 days overdue now. Issues have feelings too...

melvin-bot[bot] commented 2 months ago

@mallenexpensify, @war-in, @dukenv0307 Still overdue 6 days?! Let's take care of this!

melvin-bot[bot] commented 2 months ago

@mallenexpensify @war-in @dukenv0307 this issue is now 4 weeks old, please consider:

Finding a contributor to fix the bug
Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

melvin-bot[bot] commented 2 months ago

@mallenexpensify, @war-in, @dukenv0307 10 days overdue. Is anyone even seeing these? Hello?

mallenexpensify commented 2 months ago

@war-in can you provide an update when you get a chance? Thx

war-in commented 2 months ago

sure, sorry for the delay 😅 I posted some updates on Slack thread but let me summarize it here

I encountered two major issues:

I'm unable to properly test deeplinks on iOS due to a lack of provisioning profile (fortunately Andrew invited me to the Apple development team, so this one should be fixed now)
the second one is an Android crash happening when the app is in the background and we try to click on the deeplink. I found that the same happens when we try to open Log distance shortcut when the app is in the background so I'm currently looking into it.

Once I manage to solve the Android issue I'll post an update here. Hope to fix it soon 🙏

melvin-bot[bot] commented 1 month ago

@mallenexpensify, @war-in, @dukenv0307 Huh... This is 4 days overdue. Who can take care of this?

war-in commented 1 month ago

Small update here 👋

I managed to fix the android crash (🎉) and now I'm working on opening the correct app depending on the clicked link

war-in commented 1 month ago

I managed to make deeplinks work on Android. See video below ⬇️

https://github.com/user-attachments/assets/ef33eda2-d64c-4fa7-af76-fe6fc765a6f2

Now I have a question regarding the user flow. @trjExpensify said here that OD links should redirect to the old app and ND links should redirect to the new app (I assume regarding tryNewDotNVP value).

If someone has new experience enabled and clicks on OD link, they'll be redirected to the old app. Should we allow them to freely navigate in app? or after clicking the back button they should be taken back to the ND?

Currently, it works like in the linked video -> the user can freely navigate in the app

Please share your thoughts 🙏

trjExpensify commented 1 month ago

If someone has new experience enabled and clicks on OD link, they'll be redirected to the old app. Should we allow them to freely navigate in app? or after clicking the back button they should be taken back to the ND? Currently, it works like in the linked video -> the user can freely navigate in the app

I think that's fine here. I think it's our responsibility elsewhere to make sure that when we're surfacing links to people outside of the product (mainly product emails etc) we send them the right link for the experience they use.

war-in commented 1 month ago

@trjExpensify another question on links 😅

Is mobile.expensify.com a thing in NewDot? I noticed it in OldDot repo, but can't find it in ND. I want to confirm if this is present only in OD app 🙏

trjExpensify commented 1 month ago

Hm, when you say a "thing" whatcha mean? I don't believe we have any mobile.expensify.com links in the app anywhere, if that's the question.

war-in commented 1 month ago

Yup, I mean is NewDot using mobile.expensify.com links anywhere? I can find it in OldDot so would like to check if they're used in ND or not :)

trjExpensify commented 1 month ago

Not to my knowledge. Where on OldDot is it out of curiosity?

war-in commented 1 month ago

Not to my knowledge. Where on OldDot is it out of curiosity?

In Android and iOS configuration files, namely:

Android/AndroidManifest.xml
iOS/Expensify.entitlements

melvin-bot[bot] commented 1 month ago

@mallenexpensify, @war-in, @dukenv0307 Whoops! This issue is 2 days overdue. Let's get this updated quick!

war-in commented 1 month ago

Android PR in draft: https://github.com/Expensify/Mobile-Expensify/pull/13211

I started working on iOS support but I'm not able to test locally. I also found a PR where it's said that iOS deeplinks are tested on staging in NewDot

Julesssss commented 1 month ago

Yup, I mean is NewDot using mobile.expensify.com links anywhere?

From what I can tell, we used to share this link to direct users to the App. It takes you to the Play Store/App Store OR deep links into the app if it is already installed. I see the following in the backend:

public const DOWNLOAD_LINK = 'https://mobile.expensify.com/download';

I see no harm in keeping it, but it can simply open the app, no need to handle the deep link once the app is open.

war-in commented 1 month ago

Hi 👋 the OD PR is in review and the first round of comments was addressed 🎉

melvin-bot[bot] commented 1 month ago

Reviewing label has been removed, please complete the "BugZero Checklist".

melvin-bot[bot] commented 1 month ago

The solution for this issue has been :rocket: deployed to production :rocket: in version 9.0.54-11 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:

https://github.com/Expensify/App/pull/51334

If no regressions arise, payment will be issued on 2024-11-05. :confetti_ball:

For reference, here are some details about the assignees on this issue:

@war-in does not require payment (Contractor)
@dukenv0307 requires payment automatic offer (Reviewer)

war-in commented 1 month ago

FYI, we should not close the issue yet. Two more PRs are being reviewed (ND, OD)

melvin-bot[bot] commented 4 weeks ago

The solution for this issue has been :rocket: deployed to production :rocket: in version 9.0.56-9 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:

https://github.com/Expensify/App/pull/51429

If no regressions arise, payment will be issued on 2024-11-11. :confetti_ball:

For reference, here are some details about the assignees on this issue:

@war-in does not require payment (Contractor)
@dukenv0307 requires payment automatic offer (Reviewer)

melvin-bot[bot] commented 4 weeks ago

@dukenv0307 @mallenexpensify The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

war-in commented 4 weeks ago

There is only one PR left so let's try to merge it 🙏 cc @Julesssss

Julesssss commented 3 weeks ago

I reviewed today, I think the last step is to get Android deep links to auto-verify

melvin-bot[bot] commented 3 weeks ago

Triggered auto assignment to @bfitzexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

mallenexpensify commented 3 weeks ago

@bfitzexpensify I'm off the next week, can you please keep an eye on this issue til I'm back? Thx. Payment likely won't be due til after I'm back, I think.

Expensify / App