[HOLD for payment 2022-06-01] [$2000] Request a call is redirecting to the link - reported by @adeel0202

[mvtglobally]

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

---

Action Performed:

1. Open Concierge chat
2. click on Request a call link in the message

Expected Result:

Request a call should redirect to a screen to request a call

Actual Result:

Request a call screen is redirecting to the link

Workaround:

unknown

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number: 1.1.40-0 Reproducible in staging?: Y Reproducible in production?: Y Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Notes/Photos/Videos: Any additional supporting documentation

https://user-images.githubusercontent.com/43995119/155663968-9d652b62-3730-4844-bee6-b108786a892e.mp4

Expensify/Expensify Issue URL: Issue reported by: @adeel0202 Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1644425391389849

View all open jobs on GitHub

[MelvinBot]

Triggered auto assignment to @conorpendergrast (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[MelvinBot]

@conorpendergrast Whoops! This issue is 2 days overdue. Let's get this updated quick!

[conorpendergrast]

Reproduced; the expected effect is the same as clicking the phone icon ("Start a call") in the top-right

[MelvinBot]

Triggered auto assignment to @alex-mechler (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[alex-mechler]

Ah interesting, nice catch. I think this can be handled externally.

[MelvinBot]

Triggered auto assignment to @mallenexpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[mallenexpensify]

Posted! https://www.upwork.com/jobs/~017f47eb7070f5ad02

[MelvinBot]

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

[MelvinBot]

Triggered auto assignment to @iwiznia (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[iwiznia]

I doubt it is this, but it's possible this was broken by https://github.com/Expensify/App/pull/7662/

[rushatgabhane]

@iwiznia hmm looks like we merged 7662 three days ago. And this issue was created days one week ago. So that shouldn't be possible

[iwiznia]

Yeah sorry, it is not that issue. The problem is that it is a regular link in the text of message. The app is opening it as is, but we need to open it differently calling openOldDotLink so that it ensures the link opens logged in in oldDot.

[rushatgabhane]

Ahh makes sense

[rushatgabhane]

@mallenexpensify please consider doubling this issue for more 👀. No proposals since 12 days

[mallenexpensify]

Price doubled to $500 https://www.upwork.com/jobs/~017f47eb7070f5ad02 Thanks @rushatgabhane

[mateusbra]

I couldn't reproduce this one.

https://user-images.githubusercontent.com/56851391/158896366-6a693d20-9676-4bfb-9fb8-df0cf8d2fcbd.mp4

[rushatgabhane]

@mateusbra this issue is reproducible on clicking request a call link from Concierge's message (see the video attached in the issue)

[iwiznia]

@mallenexpensify shouldn't we double the price here?

[mallenexpensify]

Yup, thanks for the ping Ioni Price doubled to $1000 https://www.upwork.com/jobs/~017f47eb7070f5ad02

[mallenexpensify]

Doubled price to $2000 https://www.upwork.com/jobs/~017f47eb7070f5ad02

[mateusbra]

PROPOSAL

If we want to change the behavior to navigate to concierge's request a call screen we need to change how concierge's backend send messages:

We need to change the old links to new ones instead: e.g.: At the moment we are sending this message as "request a call message": You can now track, approve, and pay bills directly in Expensify! Just add your company email address to your account so suppliers can send bills to example.com@expensify.cash and SmartScan will take care of the rest. Learn more here. For questions, just reply to this message or request a call. Changing the sent link from: https://www.expensify.com/inbox?policyID=5647CB9453B86A84&taskID=ForwardToExpensifyCash to: https://new.expensify.com/request-call/NewExpensifyConciergeDM will give us the expected behavior. It will trigger AnchorRenderer.js to use navigation instead of defafult link behavior: https://github.com/Expensify/App/blob/43910fe5c20584b05fe56013005e8e59ba93e8a6/src/components/HTMLEngineProvider/HTMLRenderers/AnchorRenderer.js#L23-L37

SCREEN RECORDING

https://user-images.githubusercontent.com/56851391/162600088-1113f985-c870-41cc-9524-8541e817b6f7.mp4

[iwiznia]

hmmm I think that same message needs to work in expensify.com (@mallenexpensify can you confirm?), so we can't really change the URL.

[mallenexpensify]

I'm not able to reproduce on Desktop Version 1.1.54-0 nor iphone v1.1.52-0. For both, when I clicked the "Call me" I got a notification in-app that I had requested a call. I wasn't redirected to a link/page. Am I doing something wrong?

[rushatgabhane]

when I clicked the "Call me"

@mallenexpensify try clicking "Request a call"?

[mallenexpensify]

Thanks @rushatgabhane , I was doing something wrong. I am now able to reproduce the issue.

Is there a way to make the link contextual? if it's clicked in-app (NewDot) it mirrors the current functionality of clicking the 📞 and it's clicked anywhere else (ie. email or OldDot) it opens the link? @AndrewGable what's that service we use which helps us open links in-app? Would it be helpful/useful for this instance?

[mateusbra]

@mallenexpensify my proposal on https://github.com/Expensify/App/issues/7909#issuecomment-1094160040 explains how we could use new links to mirror the current functionality of clicking the 📞 start a call button. But as I said, it needs a change in the BE.

[rushatgabhane]

so we can't really change the URL

@mateusbra I'm guessing that we can't change the URL. https://github.com/Expensify/App/issues/7909#issuecomment-1095018770

[mallenexpensify]

https://www.expensify.com/inbox?policyID=5647CB9453B86A84&taskID=ForwardToExpensifyCash to: https://new.expensify.com/request-call/NewExpensifyConciergeDM

Couple questions....

1. Do we need/want the policyID to be in the link? If we're always asking a user to sign in, or be signed in, does it matter).
2. Should it even be necessary to log in or create an account to get a call?
3. Does anyone know what the link we use for the 📞 icon?

[mateusbra]

Sorry for the mess guys, I think I missunderstood the problem. I have an updated proposal:

PROPOSAL

We have to open it with openOldDotLink() function when we have an old dot link: we should create:

    const oldDotPath = (attrHref.startsWith("https://www.expensify.com/") && attrHref.replace("https://www.expensify.com/",''));

like what we do for internal path: https://github.com/Expensify/App/blob/43910fe5c20584b05fe56013005e8e59ba93e8a6/src/components/HTMLEngineProvider/HTMLRenderers/AnchorRenderer.js#L22-L37 then when we have an old link we could use:

    if (oldDotPath) {
        return (
            <Text
                style={styles.link}
                onPress={() => openOldDotLink(oldDotPath)}
            >
                <TNodeChildrenRenderer tnode={props.tnode} />
            </Text>
        );
    }

We also should create the old dot path at CONSTS.js instead of comparing directly, but I did this way to stay explicit what we should do.

RESULT

https://user-images.githubusercontent.com/56851391/164037285-c648fe0a-c623-4b39-b4aa-7a9c4bf35d2b.mp4

[rushatgabhane]

@mateusbra fantastic! I'm sure we can DRY it up a bit in the PR.

🎀 👀 🎀 C+ reviewed @iwiznia I like @mateusbra's proposal.

[iwiznia]

then when we have an old link we could use:

Where exactly would this code be?

We also should create the old dot path at CONSTS.js

AFAIK this should already exist... seems we need this, no?

[mateusbra]

Where exactly would this code be?

Inside AnchorRenderer.js, it will have a similar logic from what we do for internalLinks. Maybe we could even DRY the component as @rushatgabhane suggested to something like:

    if (oldDotPath || internalExpensifyPath) {
        return (
            <Text
                style={styles.link}
                onPress={() => oldDotPath ? openOldDotLink(oldDotPath) : Navigation.navigate(internalExpensifyPath)}
            >
                <TNodeChildrenRenderer tnode={props.tnode} />
            </Text>
        );
    }

AFAIK this should already exist... seems we need this, no?

I think you're right, we could use this!

[iwiznia]

Nice, I like the proposal. @mallenexpensify please hire @mateusbra

[melvin-bot[bot]]

📣 @mateusbra You have been assigned to this job by @mallenexpensify! Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

[mallenexpensify]

Hired @mateusbra as the contributor/contractor. @rushatgabhane for C+ and @adeel0202 for reporter! https://www.upwork.com/jobs/~017f47eb7070f5ad02

[mallenexpensify]

Movin' along

[mallenexpensify]

Hit staging two days ago 🎉 https://github.com/Expensify/App/pull/8769#issuecomment-1121521922

[thienlnam]

The PR caused this regression: https://github.com/Expensify/App/issues/8943

All of our attachments are from expensify.com and therefore the checks for internalExpensifyPath ended up returning it early and causing attachment items to render as links

[rushatgabhane]

@thienlnam Should we revert #8769? Because it breaks attachments.

[rushatgabhane]

@mateusbra please let us know when you can post an updated solution, thanks!

[thienlnam]

Yeah let's revert, it's a regression - I will get a PR up

[mateusbra]

Hi guys, sorry for this regression, I'll try to give a look on this tomorrow and post an alternative solution.

[mateusbra]

Hi guys, I think I achieved a way to deal with attachmens: I think we could use the same logic we did before, but this time also checking if we are rendering an attachment:

    // If we are handling an old dot Expensify link we need to open it with openOldDotLink() so we can navigate to it with the user already logged in.
    if (internalExpensifyPath && !isAttachment) {
        return (
            <Text
                style={styles.link}
                onPress={() => Link.openOldDotLink(internalExpensifyPath)}
            >
                <TNodeChildrenRenderer tnode={props.tnode} />
            </Text>
        );
    }

[rushatgabhane]

@mateusbra one small problem here is that there could be other links (now or in the future) that use expensify.com Can you please find out what else uses expensify.com

Is there a way we can make it loosely coupled?

[mallenexpensify]

@rushatgabhane can you please add details on the regression to the RCA sheet? Then, follow the steps in the C+ doc for documenting why/how the regression happened? It's not that time sensitive, it just needs to get done (in case you're catching up after being out for a bit)

[mateusbra]

@rushatgabhane I couldn't think in any other uses of expensify.com out of attachments could you think on a hint of what you want me to do? To be honest I don't think we gonna have some case we are going to use expensify.com on chat out of attachments.

[mateusbra]

@mallenexpensify I have write access to this RCA sheet , I think I shouldn't have this permission, just pinging you so you can be aware and change it :).

[mallenexpensify]

Thanks @mateusbra , it is open/write-able to anyone with the link, this is so that C+ can update it (without us having to individually invite them). In the spirit of open source, and since the related GH issues and PRs are public, there shouldn't be any sensitive details in there

[rushatgabhane]

@iwiznia as per step # 13 in C+ doc, please send an all@ post mortem email regarding this RCA doc.

The summary is posted on #expensify-open-source

• Problem solution 1
• Problem solution 2

Thanks!

[iwiznia]

Done! Thanks a lot!