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

[HOLD for payment 2022-11-16] [$250] Chat - App is crashed after click on expensifail account which is closed #12487

Closed
kbecciv opened this issue Nov 4, 2022 · 49 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering 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

Comments

@kbecciv
Copy link

kbecciv commented Nov 4, 2022

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Action Performed:

Precondition:
Create a group conversation with account A and B ( should be expensifail account)
Close the expensifail account under domain account

  1. Go to staging.new.expensify.com
  2. Log in with account A
  3. Select the created conversation between account A and B from LHN
  4. Click on user's B avatar

Expected Result:

Details for account B should be displayed.

Actual Result:

App is crashed after click on expensifail account which is closed

Workaround:

Unknown

Platform:

Where is this issue occurring?

  • Web
  • iOS
  • Android
  • Desktop App
  • Mobile Web

Version Number: 1.2.24.0

Reproducible in staging?: Yes

Reproducible in production?: No

Email or phone of affected tester (no customers): 508-221-1196/[email protected]

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Recording.1619.mp4

image (29)

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

@kbecciv kbecciv added the DeployBlockerCash This issue or pull request should block deployment label Nov 4, 2022
@OSBotify
Copy link
Contributor

OSBotify commented Nov 4, 2022

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@melvin-bot
Copy link

melvin-bot bot commented Nov 4, 2022

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

@kbecciv
Copy link
Author

kbecciv commented Nov 4, 2022

Issue is not reproducible in production
image (30)

@marcaaron
Copy link
Contributor

Go to staging.new.expensify.com
Log in with account A
Select the created conversation between account A and B from LHN
Click on account B

Not able to reproduce this. Please list the exact testing steps that were performed in order to trigger the bug.

@kbecciv
Copy link
Author

kbecciv commented Nov 5, 2022

  1. Go to staging.new.expensify.com
  2. Log in with account A (50821-11-96/Feya86Katya
  3. Select the chat conversation with [email protected]
  4. Click on [email protected] avatar

@0xmiros
Copy link
Contributor

0xmiros commented Nov 5, 2022

Proposal

displayName: ReportUtils.getDisplayNameForParticipant({login}),

Root cause:
This line was added here - #12364
and regressed by #12399

Solution:

-               displayName: ReportUtils.getDisplayNameForParticipant({login}),
+               displayName: ReportUtils.getDisplayNameForParticipant(login),

@0xmiros
Copy link
Contributor

0xmiros commented Nov 5, 2022

@marcaaron this is clearly reproducible step:
https://staging.new.expensify.com/details?login=anonymous

@melvin-bot
Copy link

melvin-bot bot commented Nov 6, 2022

⚠️ Looks like this issue was linked to a Deploy Blocker 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 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.

@melvin-bot melvin-bot bot added the Overdue label Nov 7, 2022
@tgolen
Copy link
Contributor

tgolen commented Nov 7, 2022

Ah yes, it looks like when I changed the signature of getDisplayNameForParticipant() that I missed a spot. The proposal from @0xmiroslav looks like a good fix for it.

@marcaaron
Copy link
Contributor

@tgolen to clarify, are you saying that @0xmiroslav should be hired to fix it or that you are going to raise a PR with that solution?

@melvin-bot melvin-bot bot removed the Overdue label Nov 7, 2022
@0xmiros
Copy link
Contributor

0xmiros commented Nov 7, 2022

This is a deploy blocker. Since it's urgent, I can raise PR as soon as got assigned 🚀

@tgolen
Copy link
Contributor

tgolen commented Nov 7, 2022

If @0xmiroslav can do it now, then I'll let him do it. I'm in the middle of a few other things. I'll go ahead and assign

@marcaaron marcaaron added Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor labels Nov 7, 2022
@melvin-bot
Copy link

melvin-bot bot commented Nov 7, 2022

Triggered auto assignment to @joekaufmanexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@melvin-bot
Copy link

melvin-bot bot commented Nov 7, 2022

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

@yuwenmemon yuwenmemon removed the DeployBlockerCash This issue or pull request should block deployment label Nov 8, 2022
@yuwenmemon
Copy link
Contributor

Sounds good - removing the Deploy Blocker Label then

@joekaufmanexpensify
Copy link
Contributor

Removing the hourly label as well given the PR is merged.

@joekaufmanexpensify
Copy link
Contributor

PR still on staging. 7 day payment pending period will begin as soon as it's on prod.

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Daily KSv2 labels Nov 9, 2022
@melvin-bot melvin-bot bot changed the title [$250] Chat - App is crashed after click on expensifail account which is closed [HOLD for payment 2022-11-16] [$250] Chat - App is crashed after click on expensifail account which is closed Nov 9, 2022
@melvin-bot
Copy link

melvin-bot bot commented Nov 9, 2022

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.2.25-0 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2022-11-16. 🎊

@mvtglobally
Copy link

Issue not reproducible during KI retests. (First week)

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Nov 16, 2022
@joekaufmanexpensify
Copy link
Contributor

This just hit 7 days with no reported regressions. Going to work on bug zero checklist now!

@joekaufmanexpensify
Copy link
Contributor

Discussing two new regression tests I think we should add here.

@joekaufmanexpensify
Copy link
Contributor

Regression test sent to applause to be added! Linked in BZ checklist above.

@joekaufmanexpensify
Copy link
Contributor

Okay, I updated the bug zero checklist above to reflect who is responsible for which tasks (as the checklist itself was just updated, before after it was posted to this issue).

@marcaaron / @parasharrajat let me know if I can help with commenting on the offending PR and beginning the discussion in Slack.

In the interim, I am going to pay everyone.

@joekaufmanexpensify
Copy link
Contributor

@parasharrajat Mind accepting the offer for this one so we can pay you?

@marcaaron
Copy link
Contributor

@joekaufmanexpensify it is done!

@marcaaron
Copy link
Contributor

@parasharrajat
Copy link
Member

@joekaufmanexpensify Done.

@joekaufmanexpensify
Copy link
Contributor

Great ty! Since this PR was merge the same day the contributor was assigned to the issue, both @parasharrajat and @0xmiroslav get a 50% bonus.

Going to issue payment of $375 to both of you now!

@joekaufmanexpensify
Copy link
Contributor

@parasharrajat $375 sent!

@joekaufmanexpensify
Copy link
Contributor

@0xmiroslav $375 sent!

@joekaufmanexpensify
Copy link
Contributor

Applause found this, so no reporting payment to make.

@joekaufmanexpensify
Copy link
Contributor

Job closed.

@joekaufmanexpensify
Copy link
Contributor

This is all set. Confirming whether the issue needs to remain open until applause adds regression test.

@joekaufmanexpensify
Copy link
Contributor

Okay, confirmed we don't need to wait for applause to actually add the regression test. Opening the issue for them is sufficient.

Closing as this is all set. Thanks everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering 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
Projects
None yet
Development

No branches or pull requests

9 participants