App
App copied to clipboard
Expensify
[$500] Android - IOU-Error message remains on IOU report even same error message is dismissed from 1:1
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.23-0 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:
- Log in with separate accounts in the main testing device and a secondary device
- In the main testing device create an IOU with a user
- Disable the internet connection in the main testing device
- In the main testing device (while offline) cancel the IOU
- In the secondary device (online) mark the IOU as paid with the option "Settled up elsewhere"
- Enable the internet connection in the main testing device
- Verify a red dot appears on the chat in LHN
- Tap the X from the conversation
- Go to IOU report
Expected Result:
Since User dismiss the X error should not appear into IOU report
Actual Result:
Error message remains on IOU report even same error message is dismissed from 1:1
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
- [x] Android: Native
- [ ] Android: mWeb Chrome
- [ ] iOS: Native
- [ ] iOS: mWeb Safari
- [ ] MacOS: Chrome / Safari
- [ ] MacOS: Desktop
Screenshots/Videos
Add any screenshot/video evidence
https://github.com/Expensify/App/assets/78819774/36572323-44bc-4adb-bf1f-35470abbdf78
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~019c035b355c60f48e
- Upwork Job ID: 1754884327412707328
- Last Price Increase: 2024-02-06
Job added to Upwork: https://www.upwork.com/jobs/~015c3e14be277849ac
![melvin-bot[bot] avatar](https://avatars.githubusercontent.com/in/179547?v=4 "Published by a pub.dev verified publisher"){kind=small}
Triggered auto assignment to @alexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.
Triggered auto assignment to Contributor-plus team member for initial proposal review - @Santhosh-Sellavel (External)
Proposal
Please re-state the problem that we are trying to solve in this issue.
Error message remains on IOU report even same error message is dismissed from 1:1
What is the root cause of that problem?
When we failed to delete the money request, there'll be 2 different errors created on the IOU report and the chat report here and here
What changes do you think we should make in order to solve the problem?
When we generate the errors on IOU report and chat report, generate them with the same microseconds since they are basically 1 single error.
When clearing the error from either the IOU report or the chat report, optionally check if the error content is iou.error.genericDeleteFailureMessage, if true, we'll find a report action from the other report (if IOU report then we find in chat report and vice versa) that has an error with the same microseconds (and error content), and clear that error as well.
Detail implementation
- Define the errorKey
const errorKey = DateUtils.getMicroseconds();
- Store the error for the action itself Update this line here to
errors: {
[errorKey]: 'iou.error.genericDeleteFailureMessage'
}
and here to
errors: {
[errorKey]: 'iou.error.genericDeleteFailureMessage'
},
-
Create new function named
clearAllRelatedReportActionErrorsinlibs/action/ReportActions.tsto clear the error from action itself and children/parent actions -
Update onClose to use clearAllRelatedReportActionErrors
onClose={() => ReportActions.clearAllRelatedReportActionErrors(props.report.reportID, props.action)}
Here's the test branch: https://github.com/tienifr/App/tree/fix/34189
What alternative solutions did you explore? (Optional)
The same approach listed above can be applied to many other scenarios where there's an error that occurs in multiple places but actually are the same error.
Another approach is to make microsecond errors an error object instead of just a text/translation key. In that object we can include metadata on where else this same error is added and use that to clear the error. Or we can even create a dedicated errors_${microseconds} key in Onyx to store the metadata, and when deleting we cross check that object to see if there's anywhere else this error needs to be deleted.
Result
https://github.com/Expensify/App/assets/113963320/3ae0d590-a6a4-475e-bca7-44f0422132bf
Agree with @tienifr's diagnosis. 👍
Not sure about adding logic to sync error deletion across reports though, adds a fair bit of complexity and potential for more bugs.
Ideally, if we want the error to behave as it belongs to a single entity (say IOU report) and displayed in multiple places, we should reflect that in the data and component structure. Data normalisation, as opposed to duplication. Otherwise, we may find this type of issue props up again.
Thoughts?
I agree with @tienifr and @codebycarlos. I checked his proposal. The proposal advises that the data structure be normalized. This would eliminate duplicate errors and speed up the resolution process. The solution aims for simplicity and effectiveness by providing consistent error generation with shared microseconds and updating dismissal logic to eliminate errors from both reports at the same time.
@alexpensify, @Santhosh-Sellavel Eep! 4 days overdue now. Issues have feelings too...
@Santhosh-Sellavel - could you please share feedback here on the proposals and other comments? Thanks!
@Santhosh-Sellavel - please keep me posted if you are unable to review this one. Thanks!
When we generate the errors on the IOU report and chat report, generate them with the same microseconds since they are basically 1 single error.
@tienifr How you are planning to do that, can you elaborate on that?
@tienifr Always better to add a follow-up instead of always editing the first comment
@Santhosh-Sellavel - do we need more proposals here or wait for an updated proposal from @tienifr? Thanks for clarifying where we are at for this issue. Thanks!
Sorry, I forgot to post a review comment. Yeah, let's wait for other proposals.
I'm unavailable next week, Please assign a new C+ Issue here if required while I am away, thanks!
cc: @alexpensify
Alright, @DylanDylann you are the new C+ here. Please keep us posted if one of these proposals will fix this issue. Thanks!
Also, @Santhosh-Sellavel - thank you for giving me this notice! We will keep moving forward here.
@tienifr Small note
The same approach listed above can be applied to many other scenarios where there's an error that occurs in multiple places but actually are the same error.
This point should be a bonus point in the section What changes do you think we should make in order to solve the problem?
It is not a alternative solution
Why does this bug only happen on android?
This bug happens on all platforms.
@tienifr I can't reproduce on web. Correct me if I miss any step
https://github.com/Expensify/App/assets/141406735/172f3748-8bce-43d9-b58a-485949d40d31
@alexpensify @DylanDylann 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!
@tienifr can you reproduce or do you agree with @DylanDylann? If yes, we will close. Thanks!
I still can reproduce. @alexpensify @DylanDylann
https://github.com/Expensify/App/assets/113963320/71a3f67d-03fc-4954-a659-2c9964430117