-
Notifications
You must be signed in to change notification settings - Fork 3.5k
fix: Invoice - Workspace chat has RBR in LHN after sending invoice without category. #61709
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
fix: Invoice - Workspace chat has RBR in LHN after sending invoice without category. #61709
Conversation
…thout category. Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@brunovjk, the required category violation is removed when the api is success. Is this a bug? Monosnap.screencast.2025-05-09.14-46-48.mp4 |
Yes, I had noticed this before in staging. I haven't found any related issue yet, I'll report it on slack later. |
|
Very good @Krishna2323, our initial issuer is no longer reproducible: However, looking at the "Expected Result" of this issue, I wonder if "The RBR should appear in the invoice room" in LHN or just in the preview. I will ask for help to confirm the expected result. Thanks |
|
@Krishna2323, it turns out that this is not a bug, in fact davidcardoza confirmed that the "category" field is not mandatory when we are going to create an invoice. cristipaval said that we can do everything in this PR, do you think you can remove the violation flags when we do not select a category when creating an invoice?: Screen.Recording.2025-05-15.at.10.19.40.movWe have to track all the components that interact with this flow, if the work is greater than initially planned in the issue we can ask for a bounty increase later, what do you think? Thank you very much. |
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@brunovjk I’ve updated the code: the category is no longer required for invoice transactions, and the violation is no longer added to the optimistic data. I’ve also removed the required label from the confirmation list for invoice-type requests, but I’m not sure if that change is necessary—@davidcardoza, could you please confirm? Monosnap.screencast.2025-05-17.22-32-11.mp4 |
|
@brunovjk, friendly bump ^ |
|
Apologies for the delay @Krishna2323, I was waiting for @davidcardoza to confirm the expected behavior after your last update, but it’s still not fully clear to me. I’ve asked a follow-up question to clarify this point on Slack, you can check it here. |
|
I responded in Slack a few days ago, but let me know if there are any other questions. |
|
I'll provide updates today. |
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@brunovjk, after reviewing the conversation again, I think we don't need to make any changes since davidcardoza confirmed that the RBR should be shown. I believe we just need to fix the backend to resolve this bug. Could you please confirm? Thanks! |
|
@Krishna2323 thank you but I'm not sure, the RBR in the workspace chat is very strange to me, I believe we still have a bug that we should solve. I asked again on slack. |
|
It was confirmed here @Krishna2323, we will resolve the original issue only. Thank you very much for the effort. |
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@brunovjk, I added back the initial changes, could you please test? Thanks! |
|
Sure, in a moment |
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp61709_android_native.movAndroid: mWeb Chrome61709_android_web.moviOS: HybridApp61709_ios_native.moviOS: mWeb Safari61709_ios_web.movMacOS: Chrome / Safari61709_web_chrome.movMacOS: Desktop61709_web_desktop.mov |
brunovjk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I can still reproduce a backend bug, where the category violation is removed after the response from the server:
Screen.Recording.2025-06-09.at.10.38.19.mov
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/cristipaval in version: 9.1.67-0 🚀
|
|
🚀 Deployed to production by https://github.com/lakchote in version: 9.1.67-2 🚀
|

Explanation of Change
Fixed Issues
$ #60811
PROPOSAL: #60811 (comment)
Tests
Precondition:
Offline tests
Precondition:
QA Steps
Precondition:
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))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_native.mp4
Android: mWeb Chrome
android_chrome.mp4
iOS: Native
ios_native.mp4
iOS: mWeb Safari
ios_safari.mp4
MacOS: Chrome / Safari
web_chrome.mp4
MacOS: Desktop
desktop_app.mp4