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

[$500] Clicking on room name while loading crashes the app #36271

Closed
1 of 6 tasks
m-natarajan opened this issue Feb 9, 2024 · 21 comments
Closed
1 of 6 tasks

[$500] Clicking on room name while loading crashes the app #36271

m-natarajan opened this issue Feb 9, 2024 · 21 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 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

@m-natarajan
Copy link

m-natarajan commented Feb 9, 2024

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


Version Number: 1.4.39-0
Reproducible in staging?: yes
Reproducible in production?: yes
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @getusha
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1707493474003029

Action Performed:

1.Go offline
2.Open a workspace
3.go to announce room
4.make sure the loading skeleton is shown
5.Press on the room header

Expected Result:

Opens details page

Actual Result:

App crashes

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Screen.Recording.2024-02-09.at.6.44.18.PM.mov

Screen Shot 2024-02-09 at 11 23 15 AM

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01cd42193c8752a37e
  • Upwork Job ID: 1755996151032115200
  • Last Price Increase: 2024-02-09
@m-natarajan m-natarajan added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Feb 9, 2024
Copy link

melvin-bot bot commented Feb 9, 2024

Job added to Upwork: https://www.upwork.com/jobs/~01cd42193c8752a37e

@melvin-bot melvin-bot bot changed the title Clicking on room name while loading crashes the app [$500] Clicking on room name while loading crashes the app Feb 9, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 9, 2024
Copy link

melvin-bot bot commented Feb 9, 2024

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

Copy link

melvin-bot bot commented Feb 9, 2024

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

@Natan-Asrat
Copy link

i have solved this problem countless times in my own android app so i know the solution

Copy link

melvin-bot bot commented Feb 9, 2024

📣 @Natan-Asrat! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@ZhenjaHorbach
Copy link
Contributor

ZhenjaHorbach commented Feb 9, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue.

Clicking on room name while loading crashes the app

What is the root cause of that problem?

Strange that I cannot reproduce on dev
But easy on the prod

Looks like we try to get a description from the report (Which is undefined )

Screenshot 2024-02-09 at 17 58 56

What changes do you think we should make in order to solve the problem?

I think the problem related to this line

const shouldShowReportDescription = isChatRoom && (canEditReportDescription || report.description !== '');

So we can update this line like

const shouldShowReportDescription = isChatRoom && (canEditReportDescription || report?.description !== '');

I think this fix makes sense because I noticed that in ReportDetailsPage we use report?. almost everywhere and strange that we ignore it here

Also, we have a similar place here

title={report.description}

So I think it's good to check all places where we use values fromreport

What alternative solutions did you explore? (Optional)

NA

@Natan-Asrat
Copy link

Contributor details
Your Expensify account email: natsceo@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~014ae4361b802854c6

Copy link

melvin-bot bot commented Feb 9, 2024

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

@Pujan92
Copy link
Contributor

Pujan92 commented Feb 9, 2024

I think it is fixed here

@ZhenjaHorbach
Copy link
Contributor

@ikevin127
Copy link
Contributor

ikevin127 commented Feb 9, 2024

Regression from fixed by issue #25195 , PR #35153 (merged 2h ago).
This is why it's not reproducible on dev w/ latest main and only on staging / production.

@situchan
Copy link
Contributor

situchan commented Feb 9, 2024

Regression from issue #25195 , PR #35153 (merged 2h ago).

@ikevin127 you mean fixed by #25195?
Seems like that PR fixed crash, not caused crash

@ZhenjaHorbach
Copy link
Contributor

Regression from issue #25195 , PR #35153 (merged 2h ago).

This bug is in production)
I doubt they merged this branch in prod so quickly

@situchan
Copy link
Contributor

situchan commented Feb 9, 2024

@ZhenjaHorbach if you find the case of report being undefined or null, we can continue this issue.
Otherwise can be closed.

@Pujan92
Copy link
Contributor

Pujan92 commented Feb 9, 2024

Regression seems to be from #34150 and #25195 fixed the crash.

@ZhenjaHorbach
Copy link
Contributor

@ZhenjaHorbach if you find the case of report being undefined or null, we can continue this issue. Otherwise can be closed.

Report from ONYX can be null )

I think it's easier to add a hold )
And wait when this PR will be in prod or staging
#35153

But it confuses me that we use report?. everywhere in this component
That is, it is expected that the report may be undefined or null
But ignore a few places

@situchan
Copy link
Contributor

situchan commented Feb 9, 2024

yes, Onyx values are optional and agree that we should use optional chaining everywhere.

@melvin-bot melvin-bot bot added the Overdue label Feb 12, 2024
@situchan
Copy link
Contributor

#35153 hit staging.
Now crash doesn't happen on staging.

@melvin-bot melvin-bot bot removed the Overdue label Feb 12, 2024
@situchan
Copy link
Contributor

@ZhenjaHorbach thoughts on #36271 (comment)?

@ZhenjaHorbach
Copy link
Contributor

@ZhenjaHorbach thoughts on #36271 (comment)?

I tried to reproduce the problem associated with this line)

const shouldShowReportDescription = isChatRoom && (canEditReportDescription || report.description !== '');

But I didn't succeed)
And there are no more crashes

@situchan
Copy link
Contributor

ok @dylanexpensify let's close as fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 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

7 participants