[$500] Expensify comment to enter the details for the deleted expense request

[m-natarajan]

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.4.36-5
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: @danieldoglas
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1707168112041529

Action Performed:

1. smartscan invalid receipts and wait for to fail
2. Delete th requests

Expected Result:

No message saying that Receipt scanning failed. Enter the details manually.

Actual Result:

hey now had a comment from Expensify saying that Receipt scanning failed. Enter the details manually. . Because of those comments, the expense policy chat is now always showing as a GBR, when it shouldn't show anything since I can't act on it.

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

Recording.2693.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~018bc064bf2f244ac3
• Upwork Job ID: 1754669805080993792
• Last Price Increase: 2024-02-27

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[dukenv0307]

Proposal

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

hey now had a comment from Expensify saying that Receipt scanning failed. Enter the details manually. . Because of those comments, the expense policy chat is now always showing as a GBR, when it shouldn't show anything since I can't act on it.

What is the root cause of that problem?

Currently we only delete the money request if there's no visible report action here. This doesn't work in the case where in the money request there're only actions from Expensify Notification account because those are considered as visible report action.

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

Extend the check whether delete the money request to check if the report actions are all from Expensify Notification, we can do this by, for example, counting the report actions from Expensify Notification (those will have actorAccountID of CONST.ACCOUNT_ID.NOTIFICATIONS) and check if it's equal to the childVisibleActionCount. In this case, we'll still delete the money request thread.

We might want to fix the back-end to follow the same logic too.

What alternative solutions did you explore? (Optional)

An alternative is to looping through the list of report actions to see if there's any non-Expensify-Notification visible report actions (like comment, attachment, ...). If yes, delete the money request, if not, keep the money request

[CortneyOfstad]

@abdulrahuman5196 thoughts on the proposal above?

[abdulrahuman5196]

Hi, Will review the proposal in my morning

[abdulrahuman5196]

Reviewing now

[abdulrahuman5196]

@CortneyOfstad Could you kindly confirm the expectation here?

Should we not show the failure message or not show the whole request itself? I am not sure what we should expect here.

[CortneyOfstad]

@danieldoglas since you reported the issue, I would love your input here. I'm leaning towards not showing the whole request, but wondered if you had a preference. Thanks!

[danieldoglas]

We should dismiss the GBR or RBR if we cancel the request. There's nothing else to do there, so it doesn't make sense to keep it alerting.

[CortneyOfstad]

Sounds good @danieldoglas! Does that make sense for you @abdulrahuman5196?

[CortneyOfstad]

Bump @abdulrahuman5196 ^^^

[melvin-bot]

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

[CortneyOfstad]

Bump @abdulrahuman5196 on the comment here — thanks!

[abdulrahuman5196]

Hi, @CortneyOfstad sorry for the delay. Will check on the review in my morning.

[abdulrahuman5196]

Reviewing now

[abdulrahuman5196]

This is the expected from @danieldoglas comment

We should dismiss the GBR or RBR if we cancel the request. There's nothing else to do there, so it doesn't make sense to keep it alerting.

@dukenv0307 proposal targets to delete the request in case of it only having expensify notifications. But the expectation here is to not show the green indicator alone. So the proposal is invalid at this point.

@danieldoglas Do correct me if misunderstood the requirements in any way.

[dukenv0307]

@danieldoglas The steps in the OP currently do not result in a GBR/RBR showing, neither is the video attached in the OP. Can you double check to see if it's correct?

CortneyOfstad: I'm leaning towards not showing the whole request, but wondered if you had a preference. Thanks!

@danieldoglas What do you think about this? IMO we should delete the money request completely if there's only "Expensify Notification" in it, there's no point keeping the money request in that case, especially when the user accidentally uploaded a wrong receipt and tried to delete that incorrect request.

We already do the same (remove the request completely) if there's only description changed/amount changed/other money request update message.

[CortneyOfstad]

What do you think @danieldoglas ?
I think we might also need backend changes. If possible can we confirm on backend changes.
Or we can proceed to make frontend changes and have a follow-up on backend changes?

@danieldoglas do you agree with this strictly being BE or do you think they can make the FE changes for now to keep this rolling? This issue is low-priority, but I am in the camp of keeping this moving wherever possible.

My only concern with doing the FE pieces now is the potential of creating more work on the BE side later. Do you see that happening, or is that not the case? Thanks!

[CortneyOfstad]

@danieldoglas bump on the above ^^^ TIA!

[CortneyOfstad]

Bump @danieldoglas ^^^ Thanks!

[danieldoglas]

my bad, started a conversation here: https://expensify.slack.com/archives/C03TQ48KC/p1711377098976279

[CortneyOfstad]

@danieldoglas any update? From what I could gather on the chat thread, it appears that we want to remove the violation as a message, but wanted to hear your thoughts on the best plan forward 👍

[danieldoglas]

working on the PRs to work on this internally!

[dukenv0307]

From what I could gather on the chat thread

@CortneyOfstad @danieldoglas Could you brief on the outcome of the discussion in the chat thread? Since we don't have access to it, thank you!

[CortneyOfstad]

Sorry @dukenv0307 — there is a PR request (here) to remove the message 👍

[danieldoglas]

@dukenv0307 - Since we now have violations, the comment for smartscan failed won't be added anymore. So if you delete the request, since it won't have any comments, this will work automatically.

[melvin-bot]

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

[melvin-bot]

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

[danieldoglas]

I think this is done! Adding retest label.

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[danieldoglas]

Great, closing this!