Skip to content
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 issue where IOUAction causes crash in empty chat #2110

Merged
merged 2 commits into from
Mar 29, 2021

Conversation

Julesssss
Copy link
Contributor

@Julesssss Julesssss commented Mar 26, 2021

Details

Mobile apps are crashing when the user opens a ChatReport that has an associated IOUACTION, but no messages. When we try to scrollToBottom, the index count is inaccurate -- because we are not rendering a list item for an IOUACTION.

Fixed Issues

Fixes https://github.com/Expensify/Expensify/issues/158501

Tests

  • Find two users who have not yet sent any messages to one other
  • Using the API listed below, create an IOU between these two users (either user can create it)
  • Navigate to the chat on mobile, make sure the keyboard gains focus
  • Tap outside of the input field and then back into the field a couple of times
  • The app should NOT crash!

Create an IOU transaction

curl --request POST \
  --url 'https://expensify-jules.ngrok.io/api?command=CreateIOUTransaction' \
  --header 'Content-Type: multipart/form-data; boundary=---011000010111000001101001' \
  --cookie 'email=jules%252B4%2540expensify.com; accountID=308; authToken=113FE...428E' \
  --form 'debtorEmail=jules+6@expensify.com' \
  --form currency=USD \
  --form comment=Cake \
  --form amount=110
device-2021-03-26-153831.mp4

QA Steps

Run above test ^

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

N/A

@Julesssss Julesssss requested a review from a team March 26, 2021 16:57
@Julesssss Julesssss self-assigned this Mar 26, 2021
@botify botify requested review from Jag96 and removed request for a team March 26, 2021 16:57
Copy link
Contributor

@Jag96 Jag96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When testing this, I didn't see the scroll ever get hit so I debugged and it looks like this.actionListElement.data is always undefined on both mobile and web. @Julesssss can you confirm?

image

@Julesssss
Copy link
Contributor Author

Julesssss commented Mar 29, 2021

Hey @Jag96.

I don't have a good answer for this, I also noticed the value was undefined for web. But this definitely does fix the crash (which was only affecting mobile).

Anyway, while retesting I found a better solution -- we'll simply include IOU actions in the list data, as they will be needed very soon for other IOU PRs anyway.

@Julesssss Julesssss requested a review from a team as a code owner March 29, 2021 17:17
@botify botify requested review from jasperhuangg and removed request for a team March 29, 2021 17:17
Copy link
Contributor

@Jag96 Jag96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM and makes sense!

@Jag96 Jag96 merged commit cdadc7f into master Mar 29, 2021
@Jag96 Jag96 deleted the jules-fixIOUActionCrash branch March 29, 2021 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants