Fix - Workspace chat - Both error messages appear when submitting expense fails#93698
Conversation
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
| pendingAction={report?.pendingFields?.addWorkspaceRoom ?? report?.pendingFields?.createChat} | ||
| errors={report?.errorFields?.addWorkspaceRoom ?? report?.errorFields?.createChat} | ||
| errorRowStyles={[styles.ml10, styles.mr2]} | ||
| errorRowStyles={[styles.ml10, styles.mr2, styles.mb2]} |
There was a problem hiding this comment.
I think we shouldn't add this style.
There was a problem hiding this comment.
It looks bad for workspace room error that's why I added but I know we didn't introduce it should I revert?
There was a problem hiding this comment.
Yes. It's out of scope for this PR, so I think we should leave it as is.
There was a problem hiding this comment.
I knew that but that is also a better UI. We give 8px spacing for error row in ReportScreen. Applied for all cases.
@FitseTLT you have a duplicate "Toggle Simulate failing network requests" step, which results in turning Simulate failing network requests off. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid.hybrid.mp4Android: mWeb Chromeandroid.mweb.mp4iOS: HybridAppios.hybrid.mp4iOS: mWeb Safariios.safari.mp4MacOS: Chrome / Safarimac.safari.mp4 |
It was kind of intentional because they are two different steps btw. |
@FitseTLT I know that. However, I'm concerned that QA might be confused by it. Alternatively, you could split it into two separate tests for better clarity. |
|
BTW it is already split separate numbered list in side edit view but when saved it combines it 😄 Very odd. NOw I had to add a title between them. |
|
Tests failed |
|
asking @dmkt9 a question |
|
Updated my checklist to use the latest template to fix the "PR Reviewwer Checklist" failure |
|
🚧 @Valforte has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/Valforte in version: 9.4.15-0 🚀
|
|
🤖 Help site review: no changes required I reviewed the changes in this PR against the help site files under This PR is an internal bug fix that prevents duplicate error messages from appearing when an expense submission fails (e.g. while offline / simulating failing network requests). The changes are limited to error-deduplication logic in four React components:
There is no change to any user-facing feature, workflow, setting, tab, or button label that the help site documents — it only affects how an already-existing error state is rendered (one message instead of two). The help site does not document transient error-message rendering behavior, so no article needs updating and no draft docs PR was created. @FitseTLT, if you believe a help site article is nonetheless affected, let me know which one and I'll open a draft PR. |
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.4.15-3 🚀
|



Explanation of Change
Fixed Issues
$ #92279
PROPOSAL: #92279 (comment)
Tests
Test 2
Offline tests
same as tests
QA Steps
same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari