-
Notifications
You must be signed in to change notification settings - Fork 3k
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 - Green dot is displayed for workspace chat that has failed Scan preview with red dot #31031
Comments
Job added to Upwork: https://www.upwork.com/jobs/~013fd93f0c943296dc |
Triggered auto assignment to @twisterdotcom ( |
Bug0 Triage Checklist (Main S/O)
|
Triggered auto assignment to Contributor-plus team member for initial proposal review - @akinwale ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.Green dot is displayed for workspace chat that has a preview error. What is the root cause of that problem?An error in the scan preview will not be considered as an error for the Chat Reports (1:1 or Workspace chats) as we only check for
Now, since the chat report is considered error free, this line here
requiresAttentionFromCurrentUser
Which will return true due to the report having an outstanding action in the case of pending payment. Line 1445 in c2e4042
Notice that now if we pay the request, the green dot will disappear as the What changes do you think we should make in order to solve the problem?If we want to show the red dot in case of a scan error. We need to perform the same logic used in the For each report action, we need to find the What alternative solutions did you explore? (Optional)N/A |
ProposalPlease re-state the problem that we are trying to solve in this issue.Scan - Green dot is displayed for workspace chat that has failed Scan preview with red dot What is the root cause of that problem?Let's see the logic to get error of LHNOptionRow we only get error of that report, that report actions and parent report action, we don't get error of child report App/src/libs/OptionsListUtils.js Line 339 in c2e4042
What changes do you think we should make in order to solve the problem?We need to add logic to get error of child report like this
It will check all transactions of child report and see that if there is any error. One more thing, I want to suggest that we shouldn't display green dot if that request is scanning and there are no other request (only display green dot if the scan is successfull). To do that, in here Line 167 in c2e4042
we also need to set
and also reset value in FailureData. (also need to fix it in BE) |
After reviewing the proposals, I recommend moving forward with @abdel-h66's proposal as the solution is simple and straightforward. @abdel-h66 The @DylanDylann's proposed solution adds unnecessary complexity, and I imagine having to run a 🎀 👀 🎀 C+ reviewed. |
Triggered auto assignment to @puneetlath, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
@akinwale In my proposal, I mean that App/src/libs/OptionsListUtils.js Lines 342 to 347 in 1928c95
we should update like that const reportActionErrors = _.reduce(
reportActions,
(prevReportActionErrors, action) => {
if (action && !_.isEmpty(action.errors)) {
_.extend(prevReportActionErrors, action.errors)
}
+ if (ReportUtils.hasMissingSmartscanFields(action.childReportID)) { // We can add a condition to only run this logic if + child report type is `expense`
+ _.extend(prevReportActionErrors, {smartscan: ErrorUtils.getMicroSecondOnyxError('report.genericSmartscanFailureMessage')});
+ }
return prevReportActionErrors
},
{},
);
Sorry for my confusion |
@akinwale What do you think about this point? |
Raised for discussion in an internal slack channel here: https://expensify.slack.com/archives/C036QM0SLJK/p1699651533409919 My opinion is that this actually isn't a bug, but we'll discuss and get back to you! |
@puneetlath I can't access to Slack link, could you help to confirm this point in Slack. Thanks |
I don't think we should show the red dot error for all type of actions. The |
@puneetlath I think you landed on this being fine right? From the internal discussion:
|
That's what I think yes. Thought it'd be good to have someone else in that thread agree with me 😄 |
Ahah, I agree. Seems @JmillsExpensify does too. Let's close then? |
Sounds good. Closing! |
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.96-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: Applause - Internal Team
Slack conversation:
Issue found when executing PR #30231
Action Performed:
Expected Result:
A red dot will be displayed for the workspace chat due to the failed scanning
Actual Result:
A green dot is displayed for the workspace chat with failed scanning, while the workspace chat shows Scan preview with a red dot
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Bug6267939_1699388695694.20231108_041924.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: