Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.36k stars 2.79k forks source link

[$250] Web - Task - Contact number is not set as an assignee followed by @expensify.sms via [] method #40518

Closed kbecciv closed 1 month ago

kbecciv commented 5 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.63-0 Reproducible in staging?: y Reproducible in production?: y Issue found when executing PR: https://github.com/Expensify/App/pull/39475 Issue reported by: Applause - Internal Team

Action Performed:

  1. Navigate to staging.new.expensify.com
  2. Open any chat
  3. Type [] @<contact number followed by @expensify.sms> and send to chat

Expected Result:

Task gets created with the contact number set as the assignee

Actual Result:

No user is filled out as the assignee

Workaround:

n/a

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/93399543/ff9a8042-91bb-436e-86a4-67abb513673f

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0107ae9ef02190251b
  • Upwork Job ID: 1783250182918488064
  • Last Price Increase: 2024-04-24
  • Automatic offers:
    • s77rt | Reviewer | 0
    • nkdengineer | Contributor | 0
melvin-bot[bot] commented 5 months ago

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

kbecciv commented 5 months ago

We think that this bug might be related to #vip-vsb

kbecciv commented 5 months ago

@johncschuster 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.

nkdengineer commented 5 months ago

Proposal

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

No user is filled out as the assignee

What is the root cause of that problem?

For the mentionWithDomain that we don't have the login data of it in personalDetails, assignee value is empty and then the task is created without assignee.

https://github.com/Expensify/App/blob/68e69623a8bca0dce5f384fa351d5cbb2214aeb4/src/pages/home/report/ReportFooter.tsx#L111

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

If assignee value here is empty object, we should create a optimistic personal detail for this and also create optimistic chat report as we do in setAssigneeValue function here.

https://github.com/Expensify/App/blob/68e69623a8bca0dce5f384fa351d5cbb2214aeb4/src/pages/home/report/ReportFooter.tsx#L111

  1. Create a util like buildOptimisticTaskDataForNewAssingee that will merge the optimistic data to Onyx and return an optimistic personal detail and an optimistic assigneeChatReport
function buildOptimisticTaskDataForNewAssingee(assigneeLogin: string) {
    const assigneeAccountID = UserUtils.generateAccountID(assigneeLogin);
    let report: OnyxEntry<Report> | undefined = buildOptimisticChatReport([assigneeAccountID]);
    report.isOptimisticReport = true;

    // When assigning a task to a new user, by default we share the task in their DM
    // However, the DM doesn't exist yet - and will be created optimistically once the task is created
    // We don't want to show the new DM yet, because if you select an assignee and then change the assignee, the previous DM will still be shown
    // So here, we create it optimistically to share it with the assignee, but we have to hide it until the task is created
    if (report) {
        report.isHidden = true;
    }
    Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, report);

    const optimisticPersonalDetailsListAction = {
        accountID: assigneeAccountID,
        avatar: allPersonalDetails?.[assigneeAccountID]?.avatar ?? UserUtils.getDefaultAvatarURL(assigneeAccountID),
        displayName: assigneeLogin,
        login: assigneeLogin,
    };
    Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, {[assigneeAccountID]: optimisticPersonalDetailsListAction});
    return {assignee: optimisticPersonalDetailsListAction, assigneeReport: report}
}
  1. Call the function above here if assignee is empty object to get the optimistic data.
let assignee: OnyxTypes.PersonalDetails | EmptyObject = {};
let assigneeChatReport = undefined;
if (mentionWithDomain) {
    assignee = Object.values(allPersonalDetails).find((value) => value?.login === mentionWithDomain) ?? {};
    if (!Object.keys(assignee).length) {
        const optimisticDataForNewAssignee = ReportUtils.buildOptimisticTaskDataForNewAssingee(mentionWithDomain);
        assignee = optimisticDataForNewAssignee.assignee;
        assigneeChatReport = optimisticDataForNewAssignee.assigneeReport;
    }
}
Task.createTaskAndNavigate(report.reportID, title, '', assignee?.login ?? '', assignee.accountID, assigneeChatReport, report.policyID);

https://github.com/Expensify/App/blob/68e69623a8bca0dce5f384fa351d5cbb2214aeb4/src/pages/home/report/ReportFooter.tsx#L111

What alternative solutions did you explore? (Optional)

NA

melvin-bot[bot] commented 5 months ago

@johncschuster Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

melvin-bot[bot] commented 5 months ago

@johncschuster Huh... This is 4 days overdue. Who can take care of this?

johncschuster commented 5 months ago

Agreed that this fits into #vip-vsb.

melvin-bot[bot] commented 5 months ago

Job added to Upwork: https://www.upwork.com/jobs/~0107ae9ef02190251b

melvin-bot[bot] commented 5 months ago

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

s77rt commented 5 months ago

@nkdengineer Thanks for the proposal. Your RCA is correct. The solution looks good to me.

πŸŽ€ πŸ‘€ πŸŽ€ C+ reviewed Link to proposal

melvin-bot[bot] commented 5 months ago

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

abzokhattab commented 5 months ago

I reported this bug here.. wondering if I am eligible for reporting bounty.

cc @nkuoch @johncschuster

melvin-bot[bot] commented 5 months ago

πŸ“£ @s77rt πŸŽ‰ 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 5 months ago

πŸ“£ @nkdengineer πŸŽ‰ 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 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 πŸ“–

nkdengineer commented 5 months ago

@s77rt The PR is here.

melvin-bot[bot] commented 4 months ago

This issue has not been updated in over 15 days. @nkuoch, @johncschuster, @s77rt, @nkdengineer 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!

s77rt commented 4 months ago

PR was deployed to production 2 weeks ago

nkdengineer commented 3 months ago

@johncschuster This has been on production for a while, could you help with payments here?

Thanks

johncschuster commented 3 months ago

Payment has been issued. Thanks for your patience on this!

johncschuster commented 2 months ago

@s77rt do you feel we need a regression test for this? If not, I think we can close this up!

s77rt commented 2 months ago

@johncschuster I don't think this needs a regression test. It's unlikely for this to resurface again.

melvin-bot[bot] commented 1 month ago

@nkuoch, @johncschuster, @s77rt, @nkdengineer, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.