Migrate welcome message to room description for report #34150

esh-g · 2024-01-09T15:08:20Z

Details

Fixed Issues

$ #32615
PROPOSAL: #32615 (comment)

Tests

Create a new workspace.
Go to '+' -> 'start chat'
Create a new workspace room with a custom description.
Make sure the description is shown in the header as well as in the Central pane above composer.
Click on the header
Click on description
Edit the description and try adding markdown
Make sure the description is changed in header and above composer
Make sure markdown is not rendered in Header.
[The markdown is still not supported on backend (as of 31/Jan)]

Verify that no errors appear in the JS console

Offline tests

Go offline.
Create a new workspace room with a description
Make sure the description is shown in chat mast head and header.
Click on header and edit the description.
Make sure the description menu item gets greyed out after editing offline.
Make sure markdown is rendered correctly.

QA Steps

Create a new workspace.
Go to '+' -> 'start chat'
Create a new workspace room with a custom description.
Make sure the description is shown in the header as well as in the Central pane above composer.
Click on the header
Click on description
Edit the description and try adding markdown
Make sure the description is changed in header and above composer
Make sure markdown is not rendered in Header.
[The markdown is still not supported on backend (as of 31/Jan)]

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

Screen.Recording.2024-02-01.at.3.38.10.PM.mov

Android: mWeb Chrome

Screen.Recording.2024-01-31.at.7.09.07.PM.mov

iOS: Native

fcaab924-1f36-4c33-961a-7dfe1347c158.mp4

iOS: mWeb Safari

Screen.Recording.2024-01-30.at.10.09.56.PM.mov

MacOS: Chrome / Safari

Screen.Recording.2024-01-30.at.10.01.40.PM.mov

Screen.Recording.2024-01-31.at.9.11.46.PM.mov

MacOS: Desktop

Screen.Recording.2024-01-30.at.10.07.08.PM.mov

melvin-bot · 2024-01-09T15:08:25Z

@rushatgabhane Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

esh-g · 2024-01-09T15:11:10Z

Some things that are pending:

Update AddWorkspaceRoom to accept description instead of welcomeMessage on backend.
Make the Backend capable of saving the description as html instead of converting to plain text.
Make sure the translations and copy added here is as expected. (for room description)
Look into how to reconcile tasks/description with report/description (currently, I made it /roomDescription)

@rushatgabhane Could you please also confirm the test steps? I'll attach the videos once I know they are correct!

puneetlath · 2024-01-10T17:49:46Z

Update AddWorkspaceRoom to accept description instead of welcomeMessage on backend.

This is in progress. I'll let you know when it's live.

Make the Backend capable of saving the description as html instead of converting to plain text.

This is in-progress. We'll save it as JSON `{'html': 'this is the description'}' like we do for the welcome message today.

Make sure the translations and copy added here is as expected. (for room description)

Can you post these in the #expensify-open-source channel and I'll make sure they get confirmed.

Look into how to reconcile tasks/description with report/description (currently, I made it /roomDescription)

Can you expand on this? What is the problem?

esh-g · 2024-01-13T09:54:24Z

Can you post these in the #expensify-open-source channel and I'll make sure they get confirmed.

@puneetlath I have posted the translations here

Look into how to reconcile tasks/description with report/description (currently, I made it /roomDescription)
Can you expand on this? What is the problem?

@puneetlath Currently, the path r/:reportID/description is used only for setting task descriptions. So, what I did was that I set the report description path to r/:reportID/roomDescription as it was only specific to a room. But I'm not sure if we mean to reconcile these routes somehow or rename them to /description and /taskDescription or maybe just use /description for both.

esh-g · 2024-01-13T10:00:57Z

gentle bump @rushatgabhane on the review...

src/pages/ReportDescriptionPage.js

src/pages/ReportDetailsPage.js

rushatgabhane · 2024-01-15T05:32:22Z

@esh-g thank you for the changes. The app doesn't crash now

rushatgabhane · 2024-01-15T05:35:30Z

Bug: Can't set room description when creating a room inside a workspace that the user doesn't own. They are a member tho.

Prereq: be part of another user's workspace.

Create new room and set description whilst creating the room

Expected: room description is set and can be changed.
Actual: room description is not set, and user can't update it.

rushatgabhane · 2024-01-15T05:39:40Z

Bug: when creating a new room with a descpription, the description doesn't persist.

New room -> set description when creating the room
Expected: room description persists.
Actual: description disappears

Screen.Recording.2024-01-15.at.11.08.50.mov

esh-g · 2024-01-15T11:06:19Z

@rushatgabhane This is because backend doesn't save the description for now. I already highlighted this here: #32615 (comment)

puneetlath · 2024-01-15T16:41:49Z

Update AddWorkspaceRoom to accept description instead of welcomeMessage on backend.

This is live now on staging.

Make the Backend capable of saving the description as html instead of converting to plain text.

This is live now on staging.

Make sure the translations and copy added here is as expected. (for room description)

Asked in the internal channel. Should hear back on this soon.

@puneetlath Currently, the path r/:reportID/description is used only for setting task descriptions. So, what I did was that I set the report description path to r/:reportID/roomDescription as it was only specific to a room. But I'm not sure if we mean to reconcile these routes somehow or rename them to /description and /taskDescription or maybe just use /description for both.

I think we could use /description for both. Unless it creates some sort of problem. Ultimately, they are both for setting the "description" field on a report.

esh-g · 2024-01-16T14:52:53Z

@puneetlath @rushatgabhane I have another question:
When a workspace member (who isn't an admin) creates a new room, they are able to set the description on creation but are not able to edit it since we currently only allow admins. Is this something that should be changed?

esh-g · 2024-01-16T16:07:56Z

I think we could use /description for both. Unless it creates some sort of problem. Ultimately, they are both for setting the "description" field on a report.

To do that, I'm thinking of making the /description correspond to ReportDescriptionPage and then check if it is a task:

If task: Render the TaskDescription page
else: Render RoomDescriptionPage(new)

puneetlath · 2024-01-16T20:41:49Z

When a workspace member (who isn't an admin) creates a new room, they are able to set the description on creation but are not able to edit it since we currently only allow admins. Is this something that should be changed?

Yes, any workspace member who is also a member of the room should be able to change the description.

To do that, I'm thinking of making the /description correspond to ReportDescriptionPage and then check if it is a task:

Makes sense to me. cc @rushatgabhane in case you have any concerns.

rushatgabhane · 2024-01-17T05:00:05Z

To do that, I'm thinking of making the /description correspond to ReportDescriptionPage and then check if it is a task:

Makes sense to me. cc @rushatgabhane in case you have any concerns.

To achieve same route paths /description, we would be pushing the logic from router to code.

Also, from product perspective, the task description is same as a room description. i.e. a short summary of what the task is / what the room is for.

I'm okay with this.

puneetlath · 2024-02-05T18:32:56Z

Is it a bug that the room description isn't in this menu? Or was that not part of the scope?

puneetlath · 2024-02-05T18:34:06Z

It seems like it wasn't part of the scope, though I think it should be. We can handle that as a follow-up.

puneetlath · 2024-02-05T18:42:53Z

NAB: I feel like we should allow this input to grow to the height of the page:

puneetlath

Some very minor comments, great job with this!

src/libs/ReportUtils.ts

src/pages/RoomDescriptionPage.tsx

Co-authored-by: Puneet Lath <puneet@expensify.com>

OSBotify · 2024-02-05T19:10:06Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2024-02-05T23:09:01Z

🚀 Cherry-picked to staging by https://github.com/thienlnam in version: 1.4.36-5 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

OSBotify · 2024-02-05T23:30:46Z

🚀 Cherry-picked to staging by https://github.com/thienlnam in version: 1.4.36-5 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

lanitochka17 · 2024-02-06T14:14:28Z

This PR is failing because of issue #35901

The issue is reproducible in: Web, mWeb/Chrome, Android, IOS

Bug6369339_1707227714073.Recording__2100.mp4

esh-g · 2024-02-06T14:15:58Z

Yes, this is because the backend markdown handling is pending
cc @puneetlath

puneetlath · 2024-02-06T15:15:42Z

Yep, that's a known issue that is being worked on.

OSBotify · 2024-02-07T09:51:10Z

🚀 Deployed to production by https://github.com/Julesssss in version: 1.4.37-7 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

mananjadhav · 2024-02-28T16:55:14Z

@ishpaul777 @esh-g We set disablePressOnEnter={false} in WorkspaceNewRoomPage in this commit, can we confirm why we explicitly set this? I am currently reviewing this issue.

ishpaul777 · 2024-02-28T17:08:03Z

I think this was intentional to resolve #34150 (comment)

aimane-chnaif · 2024-03-13T21:17:11Z

src/pages/RoomDescriptionPage.tsx

+            <FullPageNotFoundView shouldShow={!ReportUtils.canEditReportDescription(report, policy)}>
+                <HeaderWithBackButton
+                    title={translate('reportDescriptionPage.roomDescription')}
+                    onBackButtonPress={() => Navigation.goBack(ROUTES.REPORT_WITH_ID_DETAILS.getRoute(report.reportID))}


This approach isn't perfect after we enable deep linking to room description page from room header.
Now no longer need param in goBack.

allgandalf · 2024-08-27T06:15:20Z

src/libs/actions/Report.ts


    const optimisticData: OnyxUpdate[] = [
        {
            onyxMethod: Onyx.METHOD.MERGE,
            key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
-            value: {welcomeMessage: parsedWelcomeMessage},
+            value: {description: parsedDescription, pendingFields: {description: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE}},


We should had created a optimistic action here, this caused #46046

Migrate welcome message to room description for report

cf01991

esh-g requested a review from a team as a code owner January 9, 2024 15:08

melvin-bot bot requested review from rushatgabhane and removed request for a team January 9, 2024 15:08

esh-g mentioned this pull request Jan 9, 2024

[HOLD for payment 2024-02-15] [HOLD for payment 2024-02-14] [$1000] HIGH: Show "Room description" everywhere (2/2, external) #32615

Closed

esh-g added 2 commits January 9, 2024 20:49

Fix style 💫

a1942f8

Fix Permissions

5be28c7

rushatgabhane suggested changes Jan 14, 2024

View reviewed changes

src/pages/ReportDescriptionPage.js Outdated Show resolved Hide resolved

src/pages/ReportDescriptionPage.js Outdated Show resolved Hide resolved

src/pages/ReportDetailsPage.js Outdated Show resolved Hide resolved

esh-g added 2 commits January 14, 2024 22:16

Requested changes

6e7fc27

Hide Description conditionally

e28f6cd

Update spanish translations

10ef66b

Merge branch 'main' into room-description

e9c31ba

Fix header for tasks

03567bd

esh-g added 3 commits January 19, 2024 22:29

Reconcile description route and permission

6685a9a

Merge branch 'main' into room-description

ec7426f

Fix lint

dd34052

puneetlath requested changes Feb 5, 2024

View reviewed changes

src/libs/ReportUtils.ts Outdated Show resolved Hide resolved

src/pages/RoomDescriptionPage.tsx Outdated Show resolved Hide resolved

src/pages/RoomDescriptionPage.tsx Outdated Show resolved Hide resolved

esh-g and others added 3 commits February 6, 2024 00:18

Update src/libs/ReportUtils.ts

5630493

Co-authored-by: Puneet Lath <puneet@expensify.com>

Update src/pages/RoomDescriptionPage.tsx

00f6681

Co-authored-by: Puneet Lath <puneet@expensify.com>

Update src/pages/RoomDescriptionPage.tsx

2df9043

Co-authored-by: Puneet Lath <puneet@expensify.com>

puneetlath approved these changes Feb 5, 2024

View reviewed changes

puneetlath merged commit 7511ec4 into Expensify:main Feb 5, 2024
15 checks passed

github-actions bot mentioned this pull request Feb 5, 2024

Deploy Checklist: New Expensify 2024-02-05 #35857

Closed

66 tasks

lanitochka17 mentioned this pull request Feb 6, 2024

Room - Adding markdown to room description appears and then disappears #35901

Closed

6 tasks

FitseTLT mentioned this pull request Feb 7, 2024

[HOLD for payment 2024-03-11] [$500] [Wave Collect] [Ideal Nav] Navigating back from the Room Description page opens the Room Settings page #36054

Closed

6 tasks

ishpaul777 mentioned this pull request Feb 7, 2024

[HOLD for Payment 2024-03-07][$250] Room - Room with long description can not be created #36073

Closed

6 tasks

Pujan92 mentioned this pull request Feb 9, 2024

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

Closed

6 tasks

situchan mentioned this pull request Feb 15, 2024

[$500] LHN - Announce room on invited workspace member is not bold #35122

Closed

6 tasks

jeremy-croff mentioned this pull request Feb 28, 2024

[HOLD for payment 2024-09-11] ][$500] Lack of maximum height for the description field for room hides other options on the page #37357

Closed

6 tasks

mananjadhav mentioned this pull request Feb 28, 2024

[$500] Android - Rooms can be created when the user presses the Return key while typing in the description #36624

Closed

6 tasks

aimane-chnaif reviewed Mar 13, 2024

View reviewed changes

rayane-djouah mentioned this pull request Jul 29, 2024

allow press on enter on limit amount page #46177

Merged

50 tasks

allgandalf reviewed Aug 27, 2024

View reviewed changes

allgandalf mentioned this pull request Aug 27, 2024

[$250] When we set description of a room for the first time, optimistically it is shown as a comment in the LHN and later updates to system message #46046

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate welcome message to room description for report #34150

Migrate welcome message to room description for report #34150

esh-g commented Jan 9, 2024 •

edited

Loading

melvin-bot bot commented Jan 9, 2024

esh-g commented Jan 9, 2024 •

edited

Loading

puneetlath commented Jan 10, 2024

esh-g commented Jan 13, 2024 •

edited

Loading

esh-g commented Jan 13, 2024

rushatgabhane commented Jan 15, 2024

rushatgabhane commented Jan 15, 2024 •

edited

Loading

rushatgabhane commented Jan 15, 2024

esh-g commented Jan 15, 2024

puneetlath commented Jan 15, 2024

esh-g commented Jan 16, 2024

esh-g commented Jan 16, 2024

puneetlath commented Jan 16, 2024

rushatgabhane commented Jan 17, 2024

puneetlath commented Feb 5, 2024

puneetlath commented Feb 5, 2024

puneetlath commented Feb 5, 2024

puneetlath left a comment

OSBotify commented Feb 5, 2024

OSBotify commented Feb 5, 2024

OSBotify commented Feb 5, 2024

lanitochka17 commented Feb 6, 2024 •

edited

Loading

esh-g commented Feb 6, 2024

puneetlath commented Feb 6, 2024

OSBotify commented Feb 7, 2024

mananjadhav commented Feb 28, 2024

ishpaul777 commented Feb 28, 2024

aimane-chnaif Mar 13, 2024

allgandalf Aug 27, 2024 •

edited

Loading

Migrate welcome message to room description for report #34150

Migrate welcome message to room description for report #34150

Conversation

esh-g commented Jan 9, 2024 • edited Loading

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

melvin-bot bot commented Jan 9, 2024

esh-g commented Jan 9, 2024 • edited Loading

puneetlath commented Jan 10, 2024

esh-g commented Jan 13, 2024 • edited Loading

esh-g commented Jan 13, 2024

rushatgabhane commented Jan 15, 2024

rushatgabhane commented Jan 15, 2024 • edited Loading

rushatgabhane commented Jan 15, 2024

esh-g commented Jan 15, 2024

puneetlath commented Jan 15, 2024

esh-g commented Jan 16, 2024

esh-g commented Jan 16, 2024

puneetlath commented Jan 16, 2024

rushatgabhane commented Jan 17, 2024

puneetlath commented Feb 5, 2024

puneetlath commented Feb 5, 2024

puneetlath commented Feb 5, 2024

puneetlath left a comment

Choose a reason for hiding this comment

OSBotify commented Feb 5, 2024

OSBotify commented Feb 5, 2024

OSBotify commented Feb 5, 2024

lanitochka17 commented Feb 6, 2024 • edited Loading

esh-g commented Feb 6, 2024

puneetlath commented Feb 6, 2024

OSBotify commented Feb 7, 2024

mananjadhav commented Feb 28, 2024

ishpaul777 commented Feb 28, 2024

aimane-chnaif Mar 13, 2024

Choose a reason for hiding this comment

allgandalf Aug 27, 2024 • edited Loading

Choose a reason for hiding this comment

esh-g commented Jan 9, 2024 •

edited

Loading

esh-g commented Jan 9, 2024 •

edited

Loading

esh-g commented Jan 13, 2024 •

edited

Loading

rushatgabhane commented Jan 15, 2024 •

edited

Loading

lanitochka17 commented Feb 6, 2024 •

edited

Loading

allgandalf Aug 27, 2024 •

edited

Loading