You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[$500] Scan - Green dot is displayed for workspace chat that has failed Scan preview with red dot #31031
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:
changed the title [-]Scan - Green dot is displayed for workspace chat that has failed Scan preview with red dot[/-][+][$500] Scan - Green dot is displayed for workspace chat that has failed Scan preview with red dot[/+]on Nov 7, 2023
Please 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 report.errors and report.errorFields in the getAllReportErrors function that sets the brickRoadIndicator value for the LHN items.
getAllReportErrors also perform some other checks when to check if the report isTransactionThread ,isIOUReport or isExpenseReport and none of these will work for the chat reports.
Now, since the chat report is considered error free, this line here
Notice that now if we pay the request, the green dot will disappear as the hasOutstandingChildRequest is no longer present.
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 ReportPreview.js
For each report action, we need to find the iouReportID from the preview. Using ReportActionUtils.getIOUReportIDFromReportActionPreview and then check if that iouReport has a missing scan field using const hasMissingField = ReportUtils.hasMissingSmartscanFields(iouReportID)
What alternative solutions did you explore? (Optional)
Please 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
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
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
},
{},
);
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
After reviewing the proposals, I recommend moving forward with @abdel-h66's proposal as the solution is simple and straightforward. @abdel-h66 The optionItem in OptionRowLHN already has an iouReportID field, so there's no need to call ReportActionUtils.getIOUReportIDFromReportActionPreview to obtain the value.
@DylanDylann's proposed solution adds unnecessary complexity, and I imagine having to run a _.reduce on every option item in the LHN would cause some performance issues especially with accounts that have a large number of reports open.
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
},
{},
);
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)
I don't think we should show the red dot error for all type of actions. The smartscan error should only appear for Preview actions. Otherwise, it would be confusing if the user does not know where the error is coming from when opening the report with a red dot error.
Activity
[-]Scan - Green dot is displayed for workspace chat that has failed Scan preview with red dot[/-][+][$500] Scan - Green dot is displayed for workspace chat that has failed Scan preview with red dot[/+]melvin-bot commentedon Nov 7, 2023
Job added to Upwork: https://www.upwork.com/jobs/~013fd93f0c943296dc
melvin-bot commentedon Nov 7, 2023
Triggered auto assignment to @twisterdotcom (
Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.melvin-bot commentedon Nov 7, 2023
Bug0 Triage Checklist (Main S/O)
Platformsin OP are ✅)melvin-bot commentedon Nov 7, 2023
Triggered auto assignment to Contributor-plus team member for initial proposal review - @akinwale (
External)abdel-h66 commentedon Nov 7, 2023
Proposal
Please 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
report.errorsandreport.errorFieldsin the getAllReportErrors function that sets thebrickRoadIndicatorvalue for the LHN items.getAllReportErrorsalso perform some other checks when to check if the reportisTransactionThread,isIOUReportorisExpenseReportand none of these will work for the chat reports.Now, since the chat report is considered error free, this line here
App/src/components/LHNOptionsList/OptionRowLHN.js
Line 119 in c2e4042
requiresAttentionFromCurrentUserWhich will return true due to the report having an outstanding action in the case of pending payment.
hasOutstandingChildRequestApp/src/libs/ReportUtils.js
Line 1445 in c2e4042
Notice that now if we pay the request, the green dot will disappear as the
hasOutstandingChildRequestis no longer present.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
ReportPreview.jsFor each report action, we need to find the
iouReportIDfrom the preview. UsingReportActionUtils.getIOUReportIDFromReportActionPreviewand then check if thatiouReporthas a missing scan field usingconst hasMissingField = ReportUtils.hasMissingSmartscanFields(iouReportID)What alternative solutions did you explore? (Optional)
N/A
DylanDylann commentedon Nov 8, 2023
Proposal
Please 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
App/src/libs/actions/IOU.js
Line 167 in c2e4042
we also need to set
and also reset value in FailureData. (also need to fix it in BE)
akinwale commentedon Nov 10, 2023
After reviewing the proposals, I recommend moving forward with @abdel-h66's proposal as the solution is simple and straightforward. @abdel-h66 The
optionIteminOptionRowLHNalready has aniouReportIDfield, so there's no need to callReportActionUtils.getIOUReportIDFromReportActionPreviewto obtain the value.@DylanDylann's proposed solution adds unnecessary complexity, and I imagine having to run a
_.reduceon every option item in the LHN would cause some performance issues especially with accounts that have a large number of reports open.🎀 👀 🎀 C+ reviewed.
melvin-bot commentedon Nov 10, 2023
Triggered auto assignment to @puneetlath, see https://stackoverflow.com/c/expensify/questions/7972 for more details.
DylanDylann commentedon Nov 10, 2023
@akinwale In my proposal, I mean that
in here
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
DylanDylann commentedon Nov 10, 2023
@akinwale What do you think about this point?
cc @puneetlath
puneetlath commentedon Nov 10, 2023
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!
DylanDylann commentedon Nov 11, 2023
@puneetlath I can't access to Slack link, could you help to confirm this point in Slack. Thanks
abdel-h66 commentedon Nov 11, 2023
I don't think we should show the red dot error for all type of actions. The
smartscanerror should only appear for Preview actions. Otherwise, it would be confusing if the user does not know where the error is coming from when opening the report with a red dot error.twisterdotcom commentedon Nov 13, 2023
@puneetlath I think you landed on this being fine right? From the internal discussion:
puneetlath commentedon Nov 13, 2023
That's what I think yes. Thought it'd be good to have someone else in that thread agree with me 😄
twisterdotcom commentedon Nov 13, 2023
Ahah, I agree. Seems @JmillsExpensify does too. Let's close then?
puneetlath commentedon Nov 13, 2023
Sounds good. Closing!