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

Concierge doesn't have an avatar in the unread summary email of expense reports #52851

Closed
1 of 8 tasks
m-natarajan opened this issue Nov 20, 2024 · 28 comments
Closed
1 of 8 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review

Comments

@m-natarajan
Copy link

m-natarajan commented Nov 20, 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:
Reproducible in staging?: Needs Reproduction
Reproducible in production?: Needs Reproduction
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?:
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: @blimpich
Slack conversation (hyperlinked to channel name): ts_external_expensify_bugs

Action Performed:

  1. Setup a control workspace with instant submit and auto-reimbursement
  2. Have an employee of that workspace create/submit an expense
  3. As the approver, comment on the expense report
  4. Then approve the report, confirm it’s auto-reimbursed and the sys message from Concierge is added to the expense report.
  5. As the submitter, wait 10mins for the unread summary email to be received for the comment added in step 3.

Expected Result:

The unread summary email sent to the expense submitter should include an avatar for the Concierge comment.

Actual Result:

The avatar is missing from the Concierge comment in the email.

Workaround:

Unknown

Platforms:

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

  • Android: Standalone
  • Android: HybridApp
  • Android: mWeb Chrome
  • iOS: Standalone
  • iOS: HybridApp
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Screenshot 2024-11-20 at 10 44 23 AM

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021859743425112993650
  • Upwork Job ID: 1859743425112993650
  • Last Price Increase: 2024-11-21
Issue OwnerCurrent Issue Owner: @MarioExpensify
@m-natarajan m-natarajan added Daily KSv2 Needs Reproduction Reproducible steps needed Bug Something is broken. Auto assigns a BugZero manager. labels Nov 20, 2024
Copy link

melvin-bot bot commented Nov 20, 2024

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

@MelvinBot
Copy link

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

@trjExpensify
Copy link
Contributor

Chatting here about the steps.

@trjExpensify trjExpensify changed the title Concierge doesn't have an avatar in the the email notifications we send out for paid expenses Concierge doesn't have an avatar in the unread summary email of expense reports Nov 21, 2024
@trjExpensify trjExpensify added External Added to denote the issue can be worked on by a contributor and removed Needs Reproduction Reproducible steps needed labels Nov 21, 2024
@melvin-bot melvin-bot bot changed the title Concierge doesn't have an avatar in the unread summary email of expense reports [$250] Concierge doesn't have an avatar in the unread summary email of expense reports Nov 21, 2024
@trjExpensify
Copy link
Contributor

Applause repro'd in thread!

Copy link

melvin-bot bot commented Nov 21, 2024

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

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

melvin-bot bot commented Nov 21, 2024

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

@trjExpensify
Copy link
Contributor

@allgandalf I have a feeling this might be internal, but not 100% sure.

@allgandalf
Copy link
Contributor

@allgandalf I have a feeling this might be internal, but not 100% sure.

Definitely internal, has to do something with our ESP or the way we configure things

@trjExpensify
Copy link
Contributor

Kewl, making it internal.

@trjExpensify trjExpensify added Internal Requires API changes or must be handled by Expensify staff Hot Pick Ready for an engineer to pick up and run with and removed External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors labels Nov 22, 2024
@trjExpensify trjExpensify moved this to Hot Picks in [#whatsnext] #retain Nov 22, 2024
@melvin-bot melvin-bot bot added the Overdue label Nov 25, 2024
@trjExpensify
Copy link
Contributor

Hopefully this will get called out in today's #retain update! CC: @maddylewis

@melvin-bot melvin-bot bot removed the Overdue label Nov 25, 2024
@trjExpensify
Copy link
Contributor

Called out!

@trjExpensify
Copy link
Contributor

@blimpich as this is confirmed backend and we have the steps to repro easily, maybe you fancy it?

@melvin-bot melvin-bot bot removed the Overdue label Dec 2, 2024
@blimpich
Copy link
Contributor

blimpich commented Dec 2, 2024

My plate is full at the moment but will put this on my radar 👍

@MarioExpensify
Copy link
Contributor

Hey @trjExpensify, I can take a look at this one!

@MarioExpensify MarioExpensify self-assigned this Dec 2, 2024
@MarioExpensify MarioExpensify changed the title [$250] Concierge doesn't have an avatar in the unread summary email of expense reports Concierge doesn't have an avatar in the unread summary email of expense reports Dec 2, 2024
@trjExpensify
Copy link
Contributor

Perfect, thanks!

@trjExpensify trjExpensify removed the Hot Pick Ready for an engineer to pick up and run with label Dec 3, 2024
@MarioExpensify
Copy link
Contributor

Update

Just a quick update, had to deal with a production bug today so could not focus 100% on this one, I already found where and how the email is sent, not 100% sure why Concierge icon is not appearing, will probably have an update (and possibly PR) tomorrow.

Copy link

melvin-bot bot commented Dec 4, 2024

@trjExpensify @MarioExpensify this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@MarioExpensify
Copy link
Contributor

MarioExpensify commented Dec 5, 2024

Update

Another quick update, I'm still working on this one, I've got a small question regarding the flow in ND (before the summary email hits the user) that I've left in this thread. Now I'm taking a look at how the Concierge avatar is added to the message. I'll keep you posted.

Edit. It seems the message sent to ND comes from the workspace admin, while the email receives the system message from the Concierge, I believe that may be the source of the issue, looking how to fix it:

ND:
image

Email:
image

@trjExpensify now, which of these is actually wrong? Is it the ND message that should come from Concierge or the Email notification that should have the system message from the workspace admin?

@trjExpensify
Copy link
Contributor

Chatted it out here!

@trjExpensify trjExpensify moved this from Hot Picks to Product (CRITICAL) in [#whatsnext] #retain Dec 5, 2024
@melvin-bot melvin-bot bot added the Overdue label Dec 9, 2024
Copy link

melvin-bot bot commented Dec 9, 2024

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

@MarioExpensify
Copy link
Contributor

Update

Hey, no updates during the weekend, sorry 😅 . I already started working on how to change the email to stop using Concierge account as "sender" for the sys message. Expecting to have a PR ready for review Today if this is the only flow affected.

@melvin-bot melvin-bot bot removed the Overdue label Dec 9, 2024
@trjExpensify
Copy link
Contributor

Love it, appreciate the update!

@melvin-bot melvin-bot bot added the Overdue label Dec 12, 2024
@MarioExpensify
Copy link
Contributor

Update

Still working on this one, I thought I had the fix already but after the test the problem persists. The testing of this issue locally takes some time as sometimes the email takes more than 10 minutes to arrive so I'm multitasking a little. Will have this wrapped up soon.

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Dec 12, 2024
Copy link

melvin-bot bot commented Dec 16, 2024

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

@MarioExpensify
Copy link
Contributor

Update

PR on its way (finally!) - Testing locally and updating unit/integration tests before wrapping up.

@melvin-bot melvin-bot bot removed the Overdue label Dec 16, 2024
@MarioExpensify
Copy link
Contributor

PR is ready for review!

@MarioExpensify MarioExpensify added the Reviewing Has a PR in review label Dec 17, 2024
@MarioExpensify
Copy link
Contributor

Deployed to production. Waiting for any regressions then we can close thise.

@github-project-automation github-project-automation bot moved this from Product (CRITICAL) to Done in [#whatsnext] #retain Dec 27, 2024
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. Daily KSv2 Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review
Projects
Status: DONE
Development

No branches or pull requests

6 participants