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

[$250] [Bug] - Second message disappear when typing second message quickly after sending 1st one reported by @puneetlath. #11490

Closed
kavimuru opened this issue Oct 1, 2022 · 22 comments
Assignees
Labels
Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review

Comments

@kavimuru
Copy link

kavimuru commented Oct 1, 2022

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. send a message
  2. immediately start typing another
  3. after a second, everything I’m typing goes away (notice how the I disappears in the second message).
    If you're having difficulties reproducing, try other chats, specifically ones that have a shit-ton of messages. I (@mallenexpensify ) can replicate in Version 1.2.12-0 by attempting a couple times in different chats, feel free to test me with too mallen@expensify.com.

Expected Result:

No disappearing of the message

Actual Result:

after a second, everything I’m typing goes away (notice how the I disappears in the second message)

Workaround:

Unknown

Platform:

Where is this issue occurring?

  • Desktop App

Version Number: Version 1.2.12-0
Reproducible in staging?: yes
Reproducible in production?: yes
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:
Expensify/Expensify Issue URL:
Issue reported by: @puneetlath
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1664559360112699

View all open jobs on GitHub

https://user-images.githubusercontent.com/43996225/193375849-020b480a-e22b-44f6-a64d-c818d1b04030.mp4
https://user-images.githubusercontent.com/43996225/193375850-4b10d3f7-94d1-4235-bff8-d37687a9f7bf.mov

@kavimuru kavimuru added the Needs Reproduction Reproducible steps needed label Oct 1, 2022
@Puneet-here
Copy link
Contributor

@kavimuru, a lil misunderstanding here. It's reported by @puneetlath.

@puneetlath puneetlath changed the title [Bug] - Second message disappear when typing second message quickly after sending 1st one reported by @Puneet-here [Bug] - Second message disappear when typing second message quickly after sending 1st one Oct 1, 2022
@kavimuru kavimuru changed the title [Bug] - Second message disappear when typing second message quickly after sending 1st one [Bug] - Second message disappear when typing second message quickly after sending 1st one reported by @puneetlath. Oct 3, 2022
@melvin-bot melvin-bot bot added the Monthly KSv2 label Oct 4, 2022
@mallenexpensify mallenexpensify added Daily KSv2 Engineering and removed Monthly KSv2 Needs Reproduction Reproducible steps needed labels Oct 6, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 6, 2022

Triggered auto assignment to @ctkochan22 (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@melvin-bot
Copy link

melvin-bot bot commented Oct 7, 2022

Triggered auto assignment to @sonialiap (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@melvin-bot
Copy link

melvin-bot bot commented Oct 7, 2022

Triggered auto assignment to Contributor-plus team member for initial proposal review - @mananjadhav (External)

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 7, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 7, 2022

Current assignee @ctkochan22 is eligible for the External assigner, not assigning anyone new.

@melvin-bot melvin-bot bot changed the title [Bug] - Second message disappear when typing second message quickly after sending 1st one reported by @puneetlath. [$250] [Bug] - Second message disappear when typing second message quickly after sending 1st one reported by @puneetlath. Oct 7, 2022
@tienifr
Copy link
Contributor

tienifr commented Oct 7, 2022

Proposal

Problem

Originally, reportID is string.
When we add comment or add attachment, the result is returned from API updated the reportID as number.

That make this.props.report.reportID === prevProps.report.reportID is false and then trigger function this.updateComment(this.props.comment);. At that time, this.props.comment is empty as default
=> text typed in compose box disappears

if (this.props.report.reportID === prevProps.report.reportID) {
return;
}
this.updateComment(this.props.comment);
}

Solution

We'll use this.props.reportID instead of this.props.report.reportID because reportID is converted to string when we pass to ReportActionCompose

<ReportActionCompose
onSubmit={this.props.onSubmitComment}
reportID={this.props.report.reportID.toString()}
reportActions={this.props.reportActions}
report={this.props.report}
isComposerFullSize={this.props.isComposerFullSize}
/>

In

if (this.props.report.reportID === prevProps.report.reportID) {
return;
}
this.updateComment(this.props.comment);
}

-        if (this.props.report.reportID === prevProps.report.reportID) {
+        if (this.props.reportID === prevProps.reportID) {
Screen.Recording.2022-10-07.at.16.06.17.mp4

@mananjadhav
Copy link
Collaborator

mananjadhav commented Oct 7, 2022

@sonialiap @ctkochan22 I have some issues that were recently assigned and I won't get to this sooner. Can we please assign another C+?

@tienifr's proposal is pending to be reviewed.

@Expensify/contributor-plus

@mananjadhav mananjadhav removed their assignment Oct 7, 2022
@rushatgabhane
Copy link
Member

rushatgabhane commented Oct 7, 2022

This bug could be classified as fire🔥 and we need to fix it urgently

the result is returned from API updated the reportID as number

@ctkochan22 backend bug, right?

@trjExpensify
Copy link
Contributor

@youssef-lr - you're actively working on a PR for this, right? If so, can we assign this to you and replace it with an Internal label..

@youssef-lr
Copy link
Contributor

@trjExpensify PR ready!

@trjExpensify
Copy link
Contributor

Woop! Discussed in thread, assigning to @youssef-lr and marking as Internal.

@trjExpensify trjExpensify assigned youssef-lr and unassigned ctkochan22 Oct 7, 2022
@trjExpensify trjExpensify added Internal Requires API changes or must be handled by Expensify staff and removed External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors labels Oct 7, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 7, 2022

A Contributor Manager will be assigned to issue payment via Upwork if we deploy an associated Pull Request to production. Per Contributing.md.

@melvin-bot
Copy link

melvin-bot bot commented Oct 7, 2022

⚠️ Looks like this issue was linked to a possible regression on PRODUCTION here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a production regression has occurred a Root Cause Analysis is required. Please follow the instructions here.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@youssef-lr
Copy link
Contributor

@trjExpensify server-side fix deployed to staging and I can confirm I can longer reproduce this.

@mallenexpensify
Copy link
Contributor

Thanks for the quick fix @youssef-lr !!!

@melvin-bot
Copy link

melvin-bot bot commented Oct 17, 2022

@youssef-lr Huh... This is 4 days overdue. Who can take care of this?

@youssef-lr
Copy link
Contributor

This has been fixed. Closing!

@melvin-bot
Copy link

melvin-bot bot commented Oct 18, 2022

Triggered auto assignment to @davidcardoza because issue was reported by a contributor who needs to be compensated if this issue is fixed.
@puneetlath. reported this issue. Please review to confirm the PR fixes the bug / feature request they reported. If so, issue payment for reporting.

@mallenexpensify
Copy link
Contributor

hehe, it was an issue where Puneet, a contributor, was initially inc. as the reporter but Puneet, who works here, was the one who posted the bug. Closing

@mallenexpensify
Copy link
Contributor

@youssef-lr this is an edge case issue where the External label was added then removed. While the job was Help Wanted, @tienifr submitted the proposal above. Since they weren't hired they're technically not due compensation but, if you referenced their code when working on your fix, we should compensate them. Did you?

@youssef-lr
Copy link
Contributor

@mallenexpensify this was fixed in Web-E. We didn't end up changing code in App.

@mallenexpensify
Copy link
Contributor

Thanks @youssef-lr ! Thanks @tienifr for submitting the proposal as well. closing... again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests