[HELD #41619] [Help Wanted] [$250] Chat – No user name in Concierge welcome message

lanitochka17 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: 1.4.68-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4524515
Email or phone of affected tester (no customers): ponikarchuks+430424@gmail.com
Issue reported by: Applause - Internal Team

Action Performed:

Go to https://staging.new.expensify.com/
Log in as a new user
Enter a name in the onboarding modal and press continue
Concierge chat opens

Expected Result:

We should address the user by the firstName if we have one.

Actual Result:

We mention the user by their email, instead of just referring to them by firstName, which they just entered during onboarding.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/c018b0e1-5f7a-40d2-82cc-47e4be70227e

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01681aa730088019c9
Upwork Job ID: 1786168351516377088
Last Price Increase: 2024-05-02
Automatic offers:

GandalfGwaihir | Contributor | 0

Issue Owner

Current Issue Owner: @danielrvidal

melvin-bot[bot] commented 3 months ago

Triggered auto assignment to @strepanier03 (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.

lanitochka17 commented 3 months ago

@strepanier03 FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

allgandalf commented 3 months ago

Proposal

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

What is the root cause of that problem?

We set the concierge message to `Hey @(username) while building the optimistic comment for onboarding: https://github.com/Expensify/App/blob/573d9893ae2a8e6e4ebe64088cb2fbf62ce81804/src/libs/actions/Report.ts#L3021

That is why we do not see the user name .

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

We need to update this optimistic message, according to the OP we also need to include the user name of the user so the updated mentionComment would be similar to:

const mentionComment = ReportUtils.buildOptimisticAddCommentReportAction(`Hey ${firstName} ${lastName} @${mentionHandle} 👋`, undefined, actorAccountID);

Note: This result is according to the OP description, We can set the message as intended during the PR stage, the basic idea would still remain the same

What alternative solutions did you explore? (Optional)

strepanier03 commented 3 months ago

Raised here.

strepanier03 commented 2 months ago

This is actually working as expected for a public domain user. We could make a change to use their name but this isn't a bug so making that change would be a feature request.

melvin-bot[bot] commented 2 months ago

Job added to Upwork: https://www.upwork.com/jobs/~01681aa730088019c9

melvin-bot[bot] commented 2 months ago

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

abzokhattab commented 2 months ago

i don't understand the issue, can you please explain what is the actual result and what is the desired outcome? as I see that we already show the user handle (email) in the welcome message.

allgandalf commented 2 months ago

bump @rushatgabhane for proposal review

melvin-bot[bot] commented 2 months ago

@strepanier03, @rushatgabhane Eep! 4 days overdue now. Issues have feelings too...

rushatgabhane commented 2 months ago

@lanitochka17 do we have a problem solution statement for this feature request? There are 3 scenarios because name isn't compulsory to input.

First name and last name are blank
First name is blank
Last name is blank

allgandalf commented 2 months ago

because name isn't compulsory to input.

Actually with the new onboarding flow it is , so there are only two scenarios here:

First name is blank
Last name is blank

rushatgabhane commented 2 months ago

ahhh okay, so the name is compulsory now? Then we can work with your proposal @GandalfGwaihir

🎀 👀 🎀 we'll have to take care of the spacing if any name is empty.

melvin-bot[bot] commented 2 months ago

Triggered auto assignment to @neil-marcellini, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

neil-marcellini commented 2 months ago

ahhh okay, so the name is compulsory now? Then we can work with your proposal @GandalfGwaihir

🎀 👀 🎀 we'll have to take care of the spacing if any name is empty.

I agree that proposal will work. I think we also want only their name to show up as the mention, not their name and also an email mention, but that's a small detail we can fix in the PR.

melvin-bot[bot] commented 2 months ago

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

Offer link [Upwork job](https://www.upwork.com/jobs/~01681aa730088019c9

Upwork Job ID: 1786168351516377088

Last Price Increase: 2024-05-02) Please accept the offer 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 📖

melvin-bot[bot] commented 2 months ago

@strepanier03 @neil-marcellini @rushatgabhane @GandalfGwaihir 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!

allgandalf commented 2 months ago

PR ready for review c.c. @rushatgabhane

neil-marcellini commented 2 months ago

@rushatgabhane @GandalfGwaihir I started a discussion about the UX of this internally and our latest decision is to remove the mention entirely. It's not possible to mention a user by their display name only, and we don't really need a mention.

So the message should be "Hey Gandalf" or "Hey Gandalf Gwaihir".

allgandalf commented 2 months ago

Cool, thanks @neil-marcellini

neil-marcellini commented 2 months ago

Heads up, discussions are ongoing for the UX. I'll try to let you know when we reach a more firm decision.

melvin-bot[bot] commented 2 months ago

Current assignee @danielrvidal is eligible for the Waiting for copy assigner, not assigning anyone new.

allgandalf commented 2 months ago

Do we have any update here?

neil-marcellini commented 2 months ago

Sorry, but it got blocked by a desire to update the copy and UX for this flow. It probably won't be ready for a while since @danielrvidal is going to be OOO. He'll let you know when it's ready to be worked on again, but for now it's going to be on HOLD.

danielrvidal commented 2 months ago

I'm going to move to weekly and will get to it next week!

kevinksullivan commented 2 months ago

Updated the OP based on this convo

TLDR:

Let's hold on https://github.com/Expensify/App/issues/41619 so the GBR is working properly and we can assign tasks to create a GBR in the parent chat, instead of relying on the @ mention
If we have firstName, let's just update the copy to use that
Let's not create an @ mention, since we won't need it any longer

rushatgabhane commented 2 months ago

still holding on https://github.com/Expensify/App/issues/41619

danielrvidal commented 2 months ago

Thanks for updating @kevinksullivan, looks like we're still holding.

rushatgabhane commented 2 months ago

still holding

danielrvidal commented 1 month ago

We're still holding, that issue looks to be making solid progress still.

danielrvidal commented 1 month ago

The other issue is still being worked on so we're still holding. Let's make that work and then address this.

allgandalf commented 4 weeks ago

Any update here? , The other issue we held on that PR was merged yesterday

melvin-bot[bot] commented 2 weeks ago

This issue has not been updated in over 15 days. @danielrvidal, @strepanier03, @neil-marcellini, @rushatgabhane, @allgandalf eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

allgandalf commented 2 weeks ago

@danielrvidal , the PR we held on was merged into production, how should we proceed here?

danielrvidal commented 2 weeks ago

I think this has actually be solved already. We do not use the mention in the welcome message at all, it has been moved to the tasks. For gmail accounts, the tasks will remain to be the full email rather than the first name because that is how mentions work. So I think we're good here.

Please reopen if you disagree.

Expensify / App