[$500] Scan - notifications@expensify.com only changes to Expensify after opening the message in thread

[lanitochka17]

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.8-1 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. Log in with a new account
2. Go to any chat
3. Create a Scan request using an image that will fail the scanning
4. After the scanning fails, click on the IOU preview twice to open IOU details page
5. Right click on the "scanning failed" system message > Reply in thread

Expected Result:

In Step 4, the owner of "scanning failed" system message is displayed as Expensify

Actual Result:

In Step 4, the owner of "scanning failed" system message is notifications@expensify.com. It only changes to Expensify after opening the system message in thread

Workaround:

Unknown

Platforms:

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

• [x] Android: Native
• [x] Android: mWeb Chrome
• [x] iOS: Native
• [x] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [x] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/67410c2b-baee-47b4-9f79-16ed8c8acaa6

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01e866fa63ee2203e1
• Upwork Job ID: 1732481608650776576
• Last Price Increase: 2024-01-03

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[FitseTLT]

Proposal

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

notifications@expensify.com only changes to Expensify after opening the message in thread

What is the root cause of that problem?

When the personal detail for an account id doesn't exist in ONYXKEYS.PERSONAL_DETAILS_LIST ReportUtils.getDisplayNameForParticipant will return the login/email https://github.com/Expensify/App/blob/5a0b7fdb08979ea765f8a6c9e3b03e78ce61eb4a/src/libs/ReportUtils.ts#L1403-L1409 so the email is displayed but after opening in thread the personal detail for notifications@expensify.com will be inserted in ONYXKEYS.PERSONAL_DETAILS_LIST and ReportUtils.getDisplayNameForParticipant will return the proper display name Expensify and hence Expensify is displayed.

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

Similar to what we did for concierge we can return custom made personal detail for CONST.ACCOUNT_ID.NOTIFICATIONS below this code here https://github.com/Expensify/App/blob/5a0b7fdb08979ea765f8a6c9e3b03e78ce61eb4a/src/libs/ReportUtils.ts#L1366-L1373

if (Number(accountID) === CONST.ACCOUNT_ID.NOTIFICATIONS) {
        return {
            accountID,
            displayName: 'Expensify',
            login: CONST.EMAIL.NOTIFICATIONS,
            avatar: UserUtils.getDefaultAvatar(accountID),
        };
    }

(we can also set avatar to be expensify icon) but it is better to return this custom made personal detail only if expensify notifications personal detail is not found in onyx personal details list so that it will return the expensify notifications personal detail from onyx record whenever it exists as the avatar for expensify notification is not the same as user default avatar. change this https://github.com/Expensify/App/blob/5a0b7fdb08979ea765f8a6c9e3b03e78ce61eb4a/src/libs/ReportUtils.ts#L1374-L1378 to

return (
        allPersonalDetails?.[accountID] ??
        (Number(accountID) === CONST.ACCOUNT_ID.NOTIFICATIONS
            ? {
                  accountID,
                  displayName: 'Expensify',
                  login: CONST.EMAIL.NOTIFICATIONS,
                  avatar: UserUtils.getDefaultAvatar(accountID),
              }
            : {
                  avatar: UserUtils.getDefaultAvatar(accountID),
                  isOptimisticPersonalDetail: true,
              })
    );

or in ReportUtils.getDisplayNameForParticipant we return Expensify when accountID is CONST.ACCOUNT_ID.NOTIFICATIONS instead of the email/login when the personal detail doesn't exist in onyx

What alternative solutions did you explore? (Optional)

[Pujan92]

How about adding this required entry in the personalDetailsList itself within the backend 'OpenApp' api response, which can avoid more custom checks in front end

[melvin-bot[bot]]

@eVoloshchak, @anmurali Eep! 4 days overdue now. Issues have feelings too...

[anmurali]

When I scan a receipt to a workspace and it fails, this is what I see

This seems pretty clear.

[melvin-bot[bot]]

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

[FitseTLT]

1. Log in with a new account

@anmurali This issue occurs when logging in a new account where the notifications@expensify.com doesn't exist in personal details list on Onyx.

[melvin-bot[bot]]

@eVoloshchak, @anmurali Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot[bot]]

@eVoloshchak, @anmurali Huh... This is 4 days overdue. Who can take care of this?

[anmurali]

This issue occurs when logging in a new account where the notifications@expensify.com doesn't exist in personal details list on Onyx.

@FitseTLT what would cause this to be the case? I am not sure if this is actually possible or is an edge case.

[melvin-bot[bot]]

@eVoloshchak @anmurali 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!

[FitseTLT]

@anmurali here it is. It occurred when I signed in with a new account and created a request with receipt.

[Victor-Nyagudi]

1. Log in with a new account

I can reproduce this on my over 6-month old account with significant amount of activity.

Screenshot of reproduction


I believe the one thing my account and a new account have in common is they both don't have the Expensify account ID in personalDetailsList, which is why I most likely was able to reproduce the issue.

@anmurali Based on your earlier comment, you probably already had the Expensify accountID in your personalDetailsList from an earlier interaction.

Here's how I found out the accountID was missing.

When inspecting the props passed to the ReportActionItemThread preview below the scan request, I noticed the Expensify avatar isn't shown, and this is because its accountID is needed to retrieve the avatar picture from personalDetailsList.

Screenshot of props


Screenshot of missing accountID


It's only after I started a reply thread with the Expensify account did the accountID get added to personalDetailsList, and the avatar and display name got updated.

Screenshot of updated personalDetailsList


That being said, I think it would be a good idea to have the Expensify accountID added to personalDetailsList right after an account is opened given, like Concierge, it also sends automated responses, unless maybe there's some other reason why it's not included there in the first place.

[melvin-bot[bot]]

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

[melvin-bot[bot]]

@eVoloshchak, @anmurali Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot[bot]]

@eVoloshchak @anmurali this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

[melvin-bot[bot]]

@eVoloshchak, @anmurali Still overdue 6 days?! Let's take care of this!

[melvin-bot[bot]]

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

[melvin-bot[bot]]

@eVoloshchak, @anmurali Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

[melvin-bot[bot]]

@eVoloshchak, @anmurali 12 days overdue. Walking. Toward. The. Light...

[anmurali]

@eVoloshchak thoughts on this bug and @FitseTLT's proposal?

[melvin-bot[bot]]

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

[melvin-bot[bot]]

@eVoloshchak @anmurali this issue is now 4 weeks old and preventing us from maintaining WAQ, can you:

• Decide whether any proposals currently meet our guidelines and can be approved as-is today
• If no proposals meet that standard, please take this issue internal and treat it as one of your highest priorities
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

[melvin-bot[bot]]

Current assignee @eVoloshchak is eligible for the Internal assigner, not assigning anyone new.

[eVoloshchak]

I agree with @Pujan92's comment https://github.com/Expensify/App/issues/32581#issuecomment-1845008701

How about adding this required entry in the personalDetailsList itself within the backend 'OpenApp' api response, which can avoid more custom checks in front end

While we can fix this on front end, I don't see a reason why notifications@expensify.com contact shouldn't be added to personalDetailsList by a back end. This will also ensure we won't have to do the same fix on FE in case other service accounts are added, they can just be updated on the BE.

@anmurali, could you assign an internal engineer to this please?

[FitseTLT]

We are doing this frontend fix for concierge here https://github.com/Expensify/App/blob/5a0b7fdb08979ea765f8a6c9e3b03e78ce61eb4a/src/libs/ReportUtils.ts#L1366-L1373 If we chose BE fix we should apply it to concierge too.

[melvin-bot[bot]]

@eVoloshchak, @anmurali Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot[bot]]

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

[blimpich]

I meant to get around to looking at this today but didn't get the chance. Will try to take a look at it early next week.

[melvin-bot[bot]]

@eVoloshchak, @anmurali, @blimpich Whoops! This issue is 2 days overdue. Let's get this updated quick!

[anmurali]

@blimpich - any updates here?

[blimpich]

@anmurali not yet, I was OOO monday and then had to take a sick day yesterday. Playing catch up today.

[blimpich]

@eVoloshchak can you clarify for me, we're trying to check for / add these personal details whenever the user opens the app? Or just when a new user is created?

[anmurali]

@blimpich @eVoloshchak - any updates here?

[blimpich]

@eVoloshchak can you answer the question in this comment? I started looking into this on the backend and realized that even after reading through this issues comments it wasn't clear to me whether this issue is isolated to only when we create new accounts or can happen in other contexts too. Knowing this will help me more efficiently find a fix for this. If you're not sure that is also fine, just looking for your take on this.

[eVoloshchak]

@blimpich, apologies, missed the original message

can you clarify for me, we're trying to check for / add these personal details whenever the user opens the app? Or just when a new user is created?

It should be added to personal details whenever an account is created (I assume it would also be there every time user opens the app, unless we clear personal details?)

[blimpich]

Sounds good. I don't think we clear personal details, but I also wouldn't be surprised if there is some edge case. Knowing that the main issue is when creating a new user is helpful. Thank you 🙂

[blimpich]

Looked into this a little bit today, its slightly difficult to reproduce on the typical Expensify engineers machine since getting smart scan setup locally is quite a pain. Will keep looking into this.

[blimpich]

Banged my head against this today. @Beamanator it looks like you added this chunk of code so that concierge would be handled when we're looking for personal details. Do you remember if you looked into whether this should be done on the backend? All good if you don't remember, 8 months ago is a long time to remember a single block of code and why you put it where it was.

Right now I'm having a hard time figuring out where to put this in the backend. When we request personal details from the backend we do a complicated query to basically say "who does this person know" to see who's information we need, but its based off who the user is interacting with and doesn't really use any constants for Expensify service accounts, so its not clear where exactly this fix should be in the backend. I'll keep looking into this but it is a bit tricky.

[Beamanator]

Sorry for the delay @blimpich !

Hmm IIRC (of course i have a perfect memory of previous alex decisions), I added this as a safety net, because we didn't "always" have concierge details available in Onyx - but I like the plan to move this to the backend so we can remove this dumb looking check from the frontend.

Hmm and it sounds like you're looking into some more forward-thinking solution where we can hard-code future expensify account "personal details", right?

My thought would be: Maybe you could add some code near here which would manually add some "expensify bot accounts" (or whatever we wanna call them) & their personal details to the Onyx response when opening the app?

[melvin-bot[bot]]

@eVoloshchak, @anmurali, @blimpich Whoops! This issue is 2 days overdue. Let's get this updated quick!

[blimpich]

Update: put out a PR on the backend for fixing this. Waiting on feedback.

[blimpich]

Update: PR for backend finally got merged this morning. Waiting for it to deploy. Once its deployed we can check to see if this is still an issue in production.

[anmurali]

Adding in BZ backup when I am OOO

[melvin-bot[bot]]

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

[blimpich]

Still an issue in production. The personal details list now has notifications@expensify.com in it but it doesn't have a display name so it looks the same as before. Just created a PR to give it a display name, then this should be good.

[slafortune]

Thanks for the update @blimpich 🤞