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

[Payment due April 30th] [$500] Task- Complete domain and @expensify.sms must be included to create task with assignee via [] method #38424

Closed
6 tasks done
lanitochka17 opened this issue Mar 15, 2024 · 26 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Mar 15, 2024

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.53-2
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Action Performed:

  1. Go to staging.new.expensify.com.
  2. Go to any chat.
  3. Create a task with assignee via [] pathway in the following methods:
    a. Assignee from the same domain
  • Type [] @<email selected from suggestion list (dropped domain)> <task_name> and send to chat.
  • Type [] @ <task_name> and send to chat.
    b. Assignee using contact method
  • Type [] @ <task_name> and send to chat.
  • Type [] @<contact number followed by @expensify.sms> <task_name> and send to chat.
    c. Assignee with no prior chat from public domain (Gmail)
  • Type [] @ <task_name> and send to chat.

Expected Result:

Case a - User can create task with assignee using email selected from suggestion list (dropped domain)
Case b - User can create task with assignee using contact number without @expensify.sms
Case c - Task will be created with assignee if the email is a user with no prior chat from public domain (Gmail)

Actual Result:

Case a - Assignee can only be assigned using full email address. User is required to add complete domain after selecting from suggestion list
Case b - User is required to add @expensify.sms to assign user with contact number
Case c - The assignee completely disappears after the task is created

Workaround:

Unknown

Platforms:

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

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6415380_1710533600232.20240316_034701.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~017234f15d3d2395e5
  • Upwork Job ID: 1769796373662351360
  • Last Price Increase: 2024-04-01
  • Automatic offers:
    • abzokhattab | Contributor | 0
@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Mar 15, 2024
Copy link

melvin-bot bot commented Mar 15, 2024

Triggered auto assignment to @zanyrenney (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@lanitochka17
Copy link
Author

@zanyrenney 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

@lanitochka17
Copy link
Author

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

@abzokhattab
Copy link
Contributor

abzokhattab commented Mar 16, 2024

Proposal

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

short mentioned are not parsed when creating a task via chat commands

What is the root cause of that problem?

we don't consider the short mentions in the current regex which create tasks as following:

/**
* Matching task rule by group
* Group 1: Start task rule with []
* Group 2: Optional email group between \s+....\s* start rule with @+valid email
* Group 3: Title is remaining characters
*/
const taskRegex = /^\[\]\s+(?:@([^\s@]+@[\w.-]+\.[a-zA-Z]{2,}))?\s*([\s\S]*)/;

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

we need to expand the second group in the regex to consider also short mentions:

            const taskRegex = /^\[\]\s+(?:@([^\s@]+(?:@\w+\.\w+)?))?\s*([\s\S]*)/;

also, we need to modify the email assignment so that If the parsed text is a short mention, it will attach the current user domain to it:

first we need to add the following variables here in order to use them in the handleCreateTask function

    const allPersonalDetailLogins = Object.values(allPersonalDetails).map((personalDetail) => personalDetail?.login ?? '');
    const currentUserEmail = session?.email;
    const currentUserPrivateDomain = isEmailPublicDomain(currentUserEmail ?? '') ? '' : Str.extractEmailDomain(currentUserEmail ?? '');

then inside the function we need to modify the mail to:

      let email = match[1] ? match[1].trim() : undefined;
            if (!Str.isValidEmail(email ?? '') && currentUserPrivateDomain) {
                const mentionWithEmailDomain = `${email}@${currentUserPrivateDomain}`;
                if (allPersonalDetailLogins.includes(mentionWithEmailDomain)) {
                    email = mentionWithEmailDomain;
                }
            }

            if (Str.isValidPhone(email ?? '')) {
                const mentionWithSmsDomain = addSMSDomainIfPhoneNumber(email ?? '');

                if (allPersonalDetailLogins.includes(mentionWithSmsDomain)) {
                    email = mentionWithSmsDomain;
                }
            }

POC:

Screen.Recording.2024-03-16.at.9.07.22.AM.mov

@melvin-bot melvin-bot bot added the Overdue label Mar 18, 2024
@zanyrenney
Copy link
Contributor

Agree this looks external and adding to VIPVSB.

@melvin-bot melvin-bot bot removed the Overdue label Mar 18, 2024
@zanyrenney zanyrenney added the External Added to denote the issue can be worked on by a contributor label Mar 18, 2024
@melvin-bot melvin-bot bot changed the title Task- Complete domain and @expensify.sms must be included to create task with assignee via [] method [$500] Task- Complete domain and @expensify.sms must be included to create task with assignee via [] method Mar 18, 2024
Copy link

melvin-bot bot commented Mar 18, 2024

Job added to Upwork: https://www.upwork.com/jobs/~017234f15d3d2395e5

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Mar 18, 2024
Copy link

melvin-bot bot commented Mar 18, 2024

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

@zanyrenney zanyrenney removed the Bug Something is broken. Auto assigns a BugZero manager. label Mar 21, 2024
@melvin-bot melvin-bot bot added the Overdue label Mar 21, 2024
@zanyrenney zanyrenney added Bug Something is broken. Auto assigns a BugZero manager. and removed Overdue labels Mar 21, 2024
@melvin-bot melvin-bot bot added the Overdue label Mar 21, 2024
Copy link

melvin-bot bot commented Mar 21, 2024

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

@zanyrenney zanyrenney removed their assignment Mar 21, 2024
@melvin-bot melvin-bot bot removed the Overdue label Mar 21, 2024
@zanyrenney zanyrenney added Overdue Bug Something is broken. Auto assigns a BugZero manager. and removed Bug Something is broken. Auto assigns a BugZero manager. labels Mar 21, 2024
Copy link

melvin-bot bot commented Mar 21, 2024

Triggered auto assignment to @Christinadobrzyn (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@melvin-bot melvin-bot bot removed the Overdue label Mar 21, 2024
@zanyrenney
Copy link
Contributor

Just a note to say, I am OOO now until April 2nd, reassigning BZ team member to help while I am OOO.

@zanyrenney zanyrenney self-assigned this Mar 21, 2024
@Christinadobrzyn
Copy link
Contributor

Christinadobrzyn commented Mar 25, 2024

looks like we're accepting and reviewing proposals cc @eVoloshchak

Copy link

melvin-bot bot commented Mar 25, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@Christinadobrzyn
Copy link
Contributor

@eVoloshchak can you check out this proposal to see if it would work? #38424 (comment)

@Christinadobrzyn
Copy link
Contributor

Just a heads up that I'm going to be ooo on Friday and Monday for Easter. I'll check in on this on Tuesday when I'm back.

@eVoloshchak
Copy link
Contributor

@abzokhattab's proposal does look good to me, let's spin up the PR!

🎀👀🎀 C+ reviewed!

Copy link

melvin-bot bot commented Mar 29, 2024

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

@melvin-bot melvin-bot bot added the Overdue label Apr 1, 2024
@MonilBhavsar
Copy link
Contributor

Looks good. Thanks for attaching the POC. It doesn't display the case for user with contact number. I believe it should work.

@melvin-bot melvin-bot bot removed the Overdue label Apr 1, 2024
Copy link

melvin-bot bot commented Apr 1, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@abzokhattab
Copy link
Contributor

abzokhattab commented Apr 1, 2024

Good catch @MonilBhavsar, i didnt consider the phone numbers in my proposal, i have just modified it to consider phones as well.

so basically we are mimicking these conditions in the getParsedComment function.. so i would suggest to move it in a shared function that would be used in both locations instead of duplicating the code.

Here is the result:

Screen.Recording.2024-04-01.at.7.49.53.PM.mov

@MonilBhavsar
Copy link
Contributor

so i would suggest to move it in a shared function that would be used in both locations instead of duplicating the code.

Sounds good 👍
Let's do it

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 2, 2024
Copy link

melvin-bot bot commented Apr 2, 2024

📣 @abzokhattab 🎉 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 📖

@abzokhattab
Copy link
Contributor

The PR is ready. please have a look and let me know if you have any comments.

@Christinadobrzyn
Copy link
Contributor

looks like PR is in production - #39475

@Christinadobrzyn Christinadobrzyn changed the title [$500] Task- Complete domain and @expensify.sms must be included to create task with assignee via [] method [Payment due April 30th] [$500] Task- Complete domain and @expensify.sms must be included to create task with assignee via [] method Apr 23, 2024
@zanyrenney
Copy link
Contributor

payment summary:

paid $500 to @abzokhattab via upwork.
@eVoloshchak has not applied to the job on Upwork, i have DM'd them to apply.

@zanyrenney
Copy link
Contributor

payment summary:

paid $500 to @abzokhattab via upwork.
@eVoloshchak owed $500 to be paid via ND requests!

@JmillsExpensify
Copy link

$500 approved for @eVoloshchak

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. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
No open projects
Archived in project
Development

No branches or pull requests

7 participants