-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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 IOU badge disappearing after sending money to same user #8816
Conversation
Sorry @marcaaron. Github decided to auto-select you as a recent code owner when I marked as ready for review. Please ignore! |
* Examples of usage include 'receieving a push notification', or 'paying an IOU', because both of these cases can only | ||
* occur for an iouReport that is currently open (notifications are not sent for closed iouReports, and you cannot pay a | ||
* closed IOU). | ||
* Examples of correct usage include 'receieving a push notification', or 'paying an IOU', because both of these cases can only | ||
* occur for an iouReport that is currently open (notifications are not sent for closed iouReports, and you cannot pay a closed | ||
* IOU). Send Money is an incorrect use case, because these IOUReports are never associated with the chatReport and this would | ||
* prevent outstanding IOUs from showing. |
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.
Honestly this logic is super confusing and should be done on the backend without F.E needing to do this. I'm not going to do that though, as we're about to refactor our APIs for offline -- and because this will probably need to change anyway as part of Manual Requests.
I'll do this at some point
No worries. I am tracking changes to a few important files like |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Cherry-picked to staging by @sketchydroide in version: 1.1.57-8 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
🚀 Deployed to production by @chiragsalian in version: 1.1.57-17 🚀
|
Details
IOU Balance gets hidden from LHN for both users if they send money to someone they have an outstanding IOU with. The IOU card in the chat list also gets hidden.
Fixed Issues
$ #8352
Tests
A) Verify that sending money does not make IOU Badges disappear
B) Verify that the IOUBadge does correctly disappear after an IOUReport is paid
C) Verify that no errors appear in the JS console
PR Review Checklist
Contributor (PR Author) Checklist
### Fixed Issues
section aboveTests
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
filesSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)/** comment above it */
displayName
propertythis
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)PR Reviewer Checklist
### Fixed Issues
section aboveTests
sectionQA steps
sectiontoggleReport
and notonIconClick
).src/languages/*
filesSTYLE.md
) were followed/** comment above it */
displayName
propertythis
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)QA Steps
Please run above tests ^
Screenshots
Web
Mobile Web
Desktop
Currently unable to build. Please confirm Desktop!
iOS
Android