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

[On-Hold for Mentions #379005][$500] [LOW] Save the world - The email refers to people by email address + grammar issue #31112

Closed
5 of 6 tasks
izarutskaya opened this issue Nov 9, 2023 · 66 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering Hot Pick Ready for an engineer to pick up and run with Internal Requires API changes or must be handled by Expensify staff Monthly KSv2

Comments

@izarutskaya
Copy link

izarutskaya commented Nov 9, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Version Number: 1.3.97.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:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause-Internal Team
Slack conversation: @

Action Performed:

  1. Navigate to https://staging.new.expensify.com/
  2. Click on the FAB button
  3. Click on the "Save the word" button
  4. Click on the "I know a teacher" button
  5. Fill in all fields
  6. Click on the "Let's do this!" button
  7. Open your email app

Expected Result:

The mail should start with "Hey".
If the name of the sender and teacher is known (filled out before sending) the email should refer them by name.

Actual Result:

The email for the teacher starts with ": Hey".
It refers to both the sender and the teacher by email address.

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

Bug6269778_1699525640410.bandicam_2023-11-08_06-04-08-387.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~019c866a41051c1129
  • Upwork Job ID: 1722577721011531776
  • Last Price Increase: 2023-11-09
Issue OwnerCurrent Issue Owner: @roryabraham
@izarutskaya izarutskaya 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 Nov 9, 2023
@melvin-bot melvin-bot bot changed the title Save the world - The email refers to people by email address + grammar issue [$500] Save the world - The email refers to people by email address + grammar issue Nov 9, 2023
Copy link

melvin-bot bot commented Nov 9, 2023

Job added to Upwork: https://www.upwork.com/jobs/~019c866a41051c1129

Copy link

melvin-bot bot commented Nov 9, 2023

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 9, 2023
Copy link

melvin-bot bot commented Nov 9, 2023

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • This bug is not a duplicate report (check E/App issues and #expensify-bugs)
    • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
  • This bug is reproducible using the reproduction steps in the OP. S/O
    • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
    • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
  • This issue is filled out as thoroughly and clearly as possible
    • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

Copy link

melvin-bot bot commented Nov 9, 2023

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

@erquhart
Copy link
Contributor

erquhart commented Nov 9, 2023

Not seeing email templates in Expensify's OSS repos, guessing this is an internal issue.

@CortneyOfstad
Copy link
Contributor

CortneyOfstad commented Nov 9, 2023

Yeah, thinking this would be internal — going to get engineering eyes on this 👍

Copy link

melvin-bot bot commented Nov 9, 2023

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

@melvin-bot melvin-bot bot added the Overdue label Nov 13, 2023
Copy link

melvin-bot bot commented Nov 13, 2023

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

@erquhart
Copy link
Contributor

This is internal but Help Wanted label is still applied.

Copy link

melvin-bot bot commented Nov 13, 2023

@CortneyOfstad, @sobitneupane, @roryabraham Whoops! This issue is 2 days overdue. Let's get this updated quick!

@CortneyOfstad CortneyOfstad removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 13, 2023
@CortneyOfstad
Copy link
Contributor

@roryabraham any ideas with this one?

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Nov 13, 2023
@CortneyOfstad
Copy link
Contributor

Bump @roryabraham — TIA!

@melvin-bot melvin-bot bot removed the Overdue label Nov 20, 2023
@roryabraham roryabraham added Weekly KSv2 and removed Daily KSv2 labels Nov 21, 2023
@roryabraham
Copy link
Contributor

Ok, so this happens because the HTML in question looks like this:

const string whisperMessageForTeacher = "Hey <mention-user>@" + partnerUserID + "</mention-user>! Your friend <mention-user>@" + inviterEmail + "</mention-user> referred you to Expensify.org's latest campaign which helps teachers and their students. Our Teachers Unite campaign splits teachers' out-of-pocket expenses for essential school supplies. <a href=\"" + AuthCommand::getURLToNewDot() + "teachersunite/i-am-a-teacher\" rel=\"noreferrer noopener\">Click on Save The World</a> in the app's main menu to sign up now and start splitting your expenses with Expensify.org!";

This displays in-product as a mention, which by our current design does not use displayName at all, and instead just uses the full email. There was a ton of discussion about this but I think we went with full emails for simplicity in the V1. cc @puneetlath

So if we wanted to change this it would need to be a broader discussion around mentions in general

@roryabraham
Copy link
Contributor

My main questions here are:

  • Do we care about this issue?
  • Why did we end up always using full emails for mentions? Is that a long-term plan? Whatever happened to this nice mockup?
  • If we don't have any plans to update how mentions are displayed in-product, should we hack something up to inject displayNames for mentions in emails?

@CortneyOfstad CortneyOfstad added Monthly KSv2 and removed Weekly KSv2 labels Mar 18, 2024
@CortneyOfstad
Copy link
Contributor

Doc is still being reviewed so no update 👍

@CortneyOfstad
Copy link
Contributor

Doc still being reviewed!

@CortneyOfstad
Copy link
Contributor

No update for the on-hold issue

@CortneyOfstad
Copy link
Contributor

Still no update on the on-hold issue. David just bumped them here, so will continue to monitor 👍

@CortneyOfstad
Copy link
Contributor

They are chugging along! Only a handful of issues left, so will close this as soon as those are done!

@CortneyOfstad
Copy link
Contributor

Only a small amount still left!

Project link here

@CortneyOfstad
Copy link
Contributor

Only 3 issues left 😍

@CortneyOfstad
Copy link
Contributor

Per this comment, there are two issues that are backlog, which will delay this a bit. However, good progress across the board! 🎉

@melvin-bot melvin-bot bot added the Overdue label Jul 26, 2024
@CortneyOfstad
Copy link
Contributor

Getting close! Most recent comment is here

@melvin-bot melvin-bot bot removed the Overdue label Jul 29, 2024
@CortneyOfstad
Copy link
Contributor

No update on on-hold issue

@trjExpensify trjExpensify changed the title [On-Hold for #379005][$500] [LOW] Save the world - The email refers to people by email address + grammar issue [On-Hold for Mentions #379005][$500] [LOW] Save the world - The email refers to people by email address + grammar issue Aug 9, 2024
@CortneyOfstad
Copy link
Contributor

Latest update is here!

@CortneyOfstad
Copy link
Contributor

Only two bugs left! 🎉

@melvin-bot melvin-bot bot added the Overdue label Sep 19, 2024
@mvtglobally
Copy link

Issue not reproducible during KI retests. (First week)

@CortneyOfstad
Copy link
Contributor

Waiting on this last bug 🎉

@melvin-bot melvin-bot bot removed the Overdue label Sep 23, 2024
@mvtglobally
Copy link

Issue not reproducible during KI retests. (Second week)

@CortneyOfstad
Copy link
Contributor

Still waiting on Expensify/react-native-live-markdown#317

@mvtglobally
Copy link

Issue not reproducible during KI retests. (Third week)

@mvtglobally
Copy link

Issue not reproducible during KI retests. (Fourth week)

@mvtglobally
Copy link

Issue not reproducible during KI retests. (Fifth week). Can we close this?

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. Engineering Hot Pick Ready for an engineer to pick up and run with Internal Requires API changes or must be handled by Expensify staff Monthly KSv2
Projects
No open projects
Development

No branches or pull requests

7 participants