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

[$250] [FT] Onboarding flow: Link is broken to schedule a call with setup specialist (mobile) #43642

Closed
mountiny opened this issue Jun 13, 2024 · 13 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@mountiny
Copy link
Contributor

mountiny commented Jun 13, 2024

Background: I am testing the onboarding flow for a new user who is an admin who clicks "Manage my team's expenses" in the onboarding intent modal.

What's broken: In the "Meet your setup specialist task" there is a prompt to "Chat with your specialist in your #admins room or schedule a call today. The "admins" room deep link appears to be broken, as it currently takes me out of the mobile app and to a web app landing page to sign back in for New Expensify.

Expected behavior: The expected behavior would be to click the hyperlink and be taken to the Admins room directly in the mobile app. If I click the "Open in New Expensify" banner at the top of the mobile web landing page, then it takes me correctly to the Admins room, so it just seems that the deeplink in the onboarding task is incorrect.
(screenshots below show the onboarding task and the mobile web sign-in page that I am re-directed to which includes the banner that has the correct deep link)

App Version: Mobile - v1.4.82-0

IMG_0360 2
IMG_0360 2

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01adeea1e1f4b7a5fb
  • Upwork Job ID: 1801181699006235223
  • Last Price Increase: 2024-06-13
Issue OwnerCurrent Issue Owner: @rayane-djouah
@mountiny mountiny added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jun 13, 2024
Copy link

melvin-bot bot commented Jun 13, 2024

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

@melvin-bot melvin-bot bot changed the title Onboarding flow: Link is broken to schedule a call with setup specialist (mobile) [$250] Onboarding flow: Link is broken to schedule a call with setup specialist (mobile) Jun 13, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 13, 2024
Copy link

melvin-bot bot commented Jun 13, 2024

Current assignee @lschurr is eligible for the Bug assigner, not assigning anyone new.

Copy link

melvin-bot bot commented Jun 13, 2024

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

@truph01
Copy link
Contributor

truph01 commented Jun 13, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue.

Onboarding flow: Link is broken to schedule a call with setup specialist (mobile)

What is the root cause of that problem?

When generating the admins room link in

adminsRoomLink: `${CONFIG.EXPENSIFY.NEW_EXPENSIFY_URL}${ROUTES.REPORT_WITH_ID.getRoute(adminsChatReportID ?? '')}`,
, we use the hardcoded NEW_EXPENSIFY_URL that is the prod URL. It's different from the current environmentURL where the user is accessing the app.

So when opening that link, it has different Expensify origin and will open in a new tab.

What changes do you think we should make in order to solve the problem?

As done in MemberChangeMessage

return `<a href="${environmentURL}/r/${messageElement.roomID}" target="_blank">${messageElement.roomName}</a>`;
, we should use the environmentURL in
adminsRoomLink: `${CONFIG.EXPENSIFY.NEW_EXPENSIFY_URL}${ROUTES.REPORT_WITH_ID.getRoute(adminsChatReportID ?? '')}`,
, not NEW_EXPENSIFY_URL

adminsRoomLink: `${environmentURL}/${ROUTES.REPORT_WITH_ID.getRoute(adminsChatReportID ?? '')}`,

Above that in the same ReportActionsUtils file:

let environmentURL: string;
Environment.getEnvironmentURL().then((url: string) => (environmentURL = url));

What alternative solutions did you explore? (Optional)

Remove && hasSameOrigin in

if (internalNewExpensifyPath && hasSameOrigin) {

so whatever the environment the user is in, as long as it's a New Expensify link, open in the current tab.

Alternative 2:
Before displaying task description in

title={report.description ?? ''}
, replace any Expensify URL with environmentURL (like approach of PolicyChangeLog action)

@truph01
Copy link
Contributor

truph01 commented Jun 13, 2024

Updated with alternative solutions

@mountiny
Copy link
Contributor Author

I think we should find a solution where no matter what environment the link is in, we open the deeplink in the environment the user is in.

@truph01
Copy link
Contributor

truph01 commented Jun 13, 2024

@mountiny Yes, that's included as my first alternative solution

@mountiny
Copy link
Contributor Author

@truph01 I wonder what is the reason for having the && hasSameOrigin in place now, I feel like it will cause some regression if we remove it

@truph01
Copy link
Contributor

truph01 commented Jun 13, 2024

@truph01 I wonder what is the reason for having the && hasSameOrigin in place now, I feel like it will cause some regression if we remove it

@mountiny I believe the && hasSameOrigin was there only because we wanted the internal link to open in the current app only if it has the same Expensify origin/environment (dev vs dev, staging vs staging, ...).

I think we should find a solution where no matter what environment the link is in, we open the deeplink in the environment the user is in.

If we want this cross-environment behavior, we should remove that condition.

The cross-environment vs same-environment is the only difference when && hasSameOrigin is removed. It will cause problems only if there's any specific case that we don't want the different-environment internal link to open in app. I doubt there's such case.

@mountiny
Copy link
Contributor Author

Looking at this PPr which implemented it #24483 its expected

@mountiny
Copy link
Contributor Author

So I think for majority of users they will use production and the deeplink will work in that case, right?

If that is the case, we can close this issue @LLPeckham

@mountiny
Copy link
Contributor Author

I think there is not much else to do here. @truph01 thank you very much for helping figuring this out.

@LLPeckham
Copy link
Contributor

Thank you!!

@LLPeckham LLPeckham changed the title [$250] Onboarding flow: Link is broken to schedule a call with setup specialist (mobile) [$250] [FT] Onboarding flow: Link is broken to schedule a call with setup specialist (mobile) Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
No open projects
Archived in project
Development

No branches or pull requests

5 participants