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

[$500] Scan - __ fake __ is displayed as owner of the system generated message for failed scanning #30134

Closed
6 tasks done
lanitochka17 opened this issue Oct 21, 2023 · 21 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering Help Wanted Apply this label when an issue is open to proposals by contributors Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review Weekly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Oct 21, 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.88-3
Reproducible in staging?: Yes
Reproducible in production?: Yes
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:

Issue found when executing PR #27494

Action Performed:

  1. Go to staging.new.expensify.com
  2. Create a Scan request using a receipt with no merchant
  3. Wait for it to fail
  4. When it fails, open the IOU details page

Expected Result:

The system generated message for 'Receipt scanning failed' should be assigned with an appropriate user

Actual Result:

__ fake __ is displayed as the owner of the system generated message for 'Receipt scanning failed

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

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

Bug6245666_1697907464384!Receipt_with_no_merchant

Bug6245666_1697907464369.bandicam_2023-10-21_22-56-55-613.mp4
MacOS: Desktop

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01fb00b2dc10f94a59
  • Upwork Job ID: 1715843159501512704
  • Last Price Increase: 2023-10-28
@lanitochka17 lanitochka17 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 Oct 21, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 21, 2023

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

@melvin-bot melvin-bot bot changed the title Scan - __ fake __ is displayed as owner of the system generated message for failed scanning [$500] Scan - __ fake __ is displayed as owner of the system generated message for failed scanning Oct 21, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 21, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Oct 21, 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

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

melvin-bot bot commented Oct 21, 2023

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

@wlegolas
Copy link
Contributor

wlegolas commented Oct 21, 2023

Hello!

I was doing my investigation to generate my proposal but in my RCA I understood that the problem is more related to the backend than the frontend, for this reason, I didn't create a proposal, but to help resolve this issue I'll put below the result related to my RCA

Results from my RCA

When creating a Scan request using a receipt with no merchant, and occurs an error, the backend returns the report action with the "0" value in the actorAccountID. And this value is important to extract the actor information in the personalDetailsList.

image

Inside the ReportActionItemSingle component, is extracted the displayName from the personal list using the actorAccountID from the action report (in this case the value is "0"):

function ReportActionItemSingle(props) {
const actorAccountID = props.action.actionName === CONST.REPORT.ACTIONS.TYPE.REPORTPREVIEW && props.iouReport ? props.iouReport.managerID : props.action.actorAccountID;
let {displayName} = props.personalDetailsList[actorAccountID] || {};

There is the variable personArray that will have the array with the person's information, there is a condition to generate the person's information, if the variable displayName has a value, will create an array with the displayName as the text, otherwise, will be used the information person from the action (that is this the issue case)

const personArray = displayName
? [
{
type: 'TEXT',
text: displayName,
},
]
: props.action.person;

In this case, the ReportActionItemSingle component is using the person from the action, and if you see in the image above, the "person" property inside the action has the text "fake".

I hope this helps.

@wlegolas
Copy link
Contributor

Hello @lanitochka17, could you share a receipt with a merchant, please?

@tienifr
Copy link
Contributor

tienifr commented Oct 23, 2023

Proposal

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

Scan - __ fake __ is displayed as owner of the system generated message for failed scanning

What is the root cause of that problem?

After scanning receipt failed, we get the report action below:

Screenshot 2023-10-23 at 16 16 16

As we can see the actorAccountID is 0 and person is fake

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

It's because of this message does not belong to any users so the actorAccountID is 0

Option 1: We should define new actionName for this message, something like Scanning Error, and just show this message as the simple text. (The same way we did with Rename)

Option 2: We can check if the actionName is 0 so we should not show the avatar and displayName

Result

Screenshot 2023-10-23 at 16 23 01

@melvin-bot melvin-bot bot added the Overdue label Oct 24, 2023
@laurenreidexpensify
Copy link
Contributor

@Ollyws what do you think of the proposals above?

@melvin-bot melvin-bot bot removed the Overdue label Oct 24, 2023
@Ollyws
Copy link
Contributor

Ollyws commented Oct 26, 2023

I think some back-end changes will probably be required here so this should possibly be internal.

@tienifr
Copy link
Contributor

tienifr commented Oct 26, 2023

@Ollyws Don't you think we should change on FE side?. IMO we should handle both

@Ollyws
Copy link
Contributor

Ollyws commented Oct 26, 2023

Yes I imagine it would involve a combination, adding a new action name would have to be done on the BE side too.
Either way it would be good to get the input of an internal engineer.

@melvin-bot
Copy link

melvin-bot bot commented Oct 27, 2023

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

@mallenexpensify
Copy link
Contributor

Posted the below in an internal room

Are undefined & and _fake_ correlated-enough that, for the issue above, we want to do something to inc or consider undefined & ? Right now it's not mentioned in the GH, I'm going to ask there too and link to here.

@neil-marcellini I added the engineering label for bonus 👀, in case the two are related and we're able to expedite getting this fixed, check the link above for the discussion, which is more about undefined & than _fake_

@neil-marcellini
Copy link
Contributor

@Gonals may I re-assign this to you? I believe you recently implemented this.

@mallenexpensify
Copy link
Contributor

I co-assigned, only to ensure @Gonals sees this. One of y'all, please unassign the other! Thx

@melvin-bot
Copy link

melvin-bot bot commented Oct 28, 2023

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

@melvin-bot melvin-bot bot added the Overdue label Oct 30, 2023
@laurenreidexpensify
Copy link
Contributor

@neil-marcellini @Gonals can you review and confirm above thanks?

@melvin-bot melvin-bot bot removed the Overdue label Oct 30, 2023
@Gonals Gonals added Internal Requires API changes or must be handled by Expensify staff and removed External Added to denote the issue can be worked on by a contributor labels Oct 30, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 30, 2023

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

@Gonals
Copy link
Contributor

Gonals commented Oct 30, 2023

I am fairly sure this is caused by us using an invalid account. AKA: [email protected] (which should be the account posting this) does not exist or is in a weird state. Checking from supportal, it seems it was originally created in a closed state, so my bet is that Account::getID is not getting it correctly.

In dev, passing a valid account there fixes the problem, so I don't think we need any frontend changes.

Auth PR incoming

@Gonals Gonals added the Reviewing Has a PR in review label Oct 30, 2023
@laurenreidexpensify laurenreidexpensify added Weekly KSv2 and removed Daily KSv2 labels Nov 3, 2023
@mvtglobally
Copy link

Issue not reproducible during KI retests. (First week)

@Gonals
Copy link
Contributor

Gonals commented Nov 10, 2023

This should be fixed already

@Gonals Gonals closed this as completed Nov 10, 2023
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 Help Wanted Apply this label when an issue is open to proposals by contributors Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

9 participants