Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.51k stars 2.87k forks source link

Room – Unable to save room description with numbers only #51789

Open lanitochka17 opened 1 day ago

lanitochka17 commented 1 day ago

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: 9.0.56-0 Reproducible in staging?: Y Reproducible in production?: N If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Email or phone of affected tester (no customers): applausetester+jp_11311024@applause.expensifail.com Issue reported by: Applause - Internal Team

Action Performed:

  1. Go to https://staging.new.expensify.com/
  2. Log
  3. Create a workspace
  4. Create a room without description
  5. Open room chat
  6. Click on header, open description
  7. Enter few numbers into description and save

Expected Result:

Description is saved

Actual Result:

Description is empty

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/9a6cf043-6c88-4b89-a187-2c8d6b505534

View all open jobs on GitHub

melvin-bot[bot] commented 1 day ago

Triggered auto assignment to @Christinadobrzyn (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

melvin-bot[bot] commented 1 day ago

Triggered auto assignment to @Gonals (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

melvin-bot[bot] commented 1 day ago

💬 A slack conversation has been started in #expensify-open-source

github-actions[bot] commented 1 day ago

:wave: 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.
lanitochka17 commented 1 day ago

We think that this bug might be related to #wave-collect - Release 1

lanitochka17 commented 1 day ago

Production:

https://github.com/user-attachments/assets/a711e12c-4fac-4ebe-b514-3f62c6e459b5

NJ-2020 commented 1 day ago

Regression PR: https://github.com/Expensify/App/pull/50838

It's because when we set the room description to a number e.g 2222222 and when displaying the room description, we use getReportDescription function for the report description value

Inside this function we will parse the report.description using JSON parse and return the html value, if there's an error meaning the value could not be parse or something else, we will simply return report.description https://github.com/Expensify/App/blob/6c33bba59c3305d59c96b265183473bc7ffed2e0/src/libs/ReportUtils.ts#L4261-L4271

When we set the room description something like testsd not a number, it will work because the JSON parse cannot parse string value which will run this return statement https://github.com/Expensify/App/blob/6c33bba59c3305d59c96b265183473bc7ffed2e0/src/libs/ReportUtils.ts#L4269-L4271

But when we set a number value inside the room description, the JSON parse did not throw an error instead returning the report description number in type of number not string

So when there's no error this catch block will not run https://github.com/Expensify/App/blob/6c33bba59c3305d59c96b265183473bc7ffed2e0/src/libs/ReportUtils.ts#L4269-L4271 But instead it will run this https://github.com/Expensify/App/blob/6c33bba59c3305d59c96b265183473bc7ffed2e0/src/libs/ReportUtils.ts#L4267-L4268 Since it's number so when we return objectDescription.html ?? '' it will return empty string


Instead of returning objectDescription.html ?? '' we can return this:

const reportDescription = report?.description;
const objectDescription = JSON.parse(reportDescription) as {html: string};
return objectDescription.html ?? report?.description ?? '';
MuaazArshad commented 1 day ago

Proposal

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

Unable to save room description with numbers only.

What is the root cause of that problem?

We are passing only strings for the room description; if we receive a number, it cannot be processed as a string. here https://github.com/Expensify/App/blob/6c33bba59c3305d59c96b265183473bc7ffed2e0/src/libs/ReportUtils.ts#L4265-L4271

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

We should cast numbers to string before processing like this

try {
        const reportDescription = report?.description;
        const objectDescription = JSON.parse(reportDescription) as {html: string};
        const newObjectDescription = {html:String(objectDescription)};
        return newObjectDescription.html ?? '';
    } catch (error) {
        return report?.description ?? '';
    }

What alternative solutions did you explore? (Optional)

Christinadobrzyn commented 1 day ago

@Gonals I can reproduce this, can this go external? Is it a blocker?

Beamanator commented 1 day ago

@NJ-2020 -thanks for finding what looks like the regression, i pinged the PR authors in slack 🙏