[$250] Room - #announce room can be created via room mention

[IuliiaHerets]

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

---

Version Number: 9.0.44-0
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Create a new workspace.
3. Go to FAB > Start chat > Room.
4. Enter #announce as room name and save it.
5. Note that it shows error that the room name cannot be #announce.
6. Go to workspace chat.
7. Send message containing #announce mention.
8. Click Yes from the whisper to create the room.
9. Note that #announce room can be created via mention.
10. Invite 3 members to the workspace.
11. Note that now there are two #announce rooms.

Expected Result:

In Step 8, app should prevent #announce room from being created via room mention.

Actual Result:

In Step 8. #announce room can be via room mention.
In Step 11, after inviting 3 members to the workspace, it results in two #announce rooms.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6623434_1727973755589.20241004_003551.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021845791038434812550
• Upwork Job ID: 1845791038434812550
• Last Price Increase: 2024-10-28
• Automatic offers:
• allgandalf | Reviewer | 104651165
• mkzie2 | Contributor | 104651168

Issue Owner

Current Issue Owner: @allgandalf

[melvin-bot]

Triggered auto assignment to @sonialiap (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.

[IuliiaHerets]

@sonialiap FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[Nodebrute]

Proposal

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

#announce room can be created via room mention

What is the root cause of that problem?

We are adding #announce in reserved names

App/src/CONST.ts

Line 1011 in 96aceca

RESERVED_ROOM_NAMES: ['#admins', '#announce'],

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

We can remove #announce from here

App/src/CONST.ts

Line 1011 in 96aceca

RESERVED_ROOM_NAMES: ['#admins', '#announce'],

We should also cleanup in other places where we are using #announce

What alternative solutions did you explore? (Optional)

[mkzie2]

Edited by proposal-police: This proposal was edited at 2024-10-14 21:42:57 UTC.

Proposal

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

In Step 8. #announce room can be via room mention.
In Step 11, after inviting 3 members to the workspace, it results in two #announce rooms.

What is the root cause of that problem?

• The policyAnnounce is just created if the policy has 3 or more members. This is handled in PR.

• In this bug, when policyAnnounce is not created, then we send a message with room mention #announce, BE returns the actionable whisper, that is incorrect.

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

• First, BE should not return actionable whisper message when user send #announce message.

• Then, we need to modify the error message when creating a new room as well as when editing a room name. When user try creating a room with "announce" as name:

1. If the policyAnnounce room has not been created yet, should show "This name is reserved for future use. Please choose a different name"

2. If the policyAnnounce room has been, should show "A room with this name already exists" or "Announce is a default room on all workspaces. Please choose another name."

• Below is the code changes:

1. Introduce a function to check whether the policy has policyAnnounce or not:

function hasAnnounceRoom(reports: OnyxCollection<Report>, policyID: string): boolean {
    return Object.values(reports ?? {}).some((report) => report && report.policyID === policyID && isAnnounceRoom(report));
}

2. Update the validation:

App/src/pages/workspace/WorkspaceNewRoomPage.tsx

Line 161 in 821edc5

} else if (ValidationUtils.isReservedRoomName(values.roomName)) {

            } else if (values.roomName === '#announce') {
                if (!ValidationUtils.hasAnnounceRoom(reports, values.policyID ?? '-1')) {
                    ErrorUtils.addErrorMessage(errors, 'roomName', 'This name is reserved for future use. Please choose a different name');
                } else {
                    ErrorUtils.addErrorMessage(errors, 'roomName', translate('newRoomPage.roomAlreadyExistsError'));
                }
            } else if (ValidationUtils.isReservedRoomName(values.roomName)) {

• The similar fix should be applied to:

App/src/pages/settings/Report/RoomNamePage.tsx

Line 66 in e7c24bb

} else if (ValidationUtils.isReservedRoomName(values.roomName)) {

What alternative solutions did you explore? (Optional)

• First, BE should not return actionable whisper message when user send #announce message.

• Then update the validation:

App/src/pages/workspace/WorkspaceNewRoomPage.tsx

Line 161 in 821edc5

} else if (ValidationUtils.isReservedRoomName(values.roomName)) {

            } else if (values.roomName === '#announce') {
                ErrorUtils.addErrorMessage(errors, 'roomName', 'The name #announce is a default room title and cannot be used in a new name, please select a new name');
            } else if (ValidationUtils.isReservedRoomName(values.roomName)) {

• The similar fix should be applied to:

App/src/pages/settings/Report/RoomNamePage.tsx

Line 66 in e7c24bb

} else if (ValidationUtils.isReservedRoomName(values.roomName)) {

[mkzie2]

We only create a workspace when 3 or more participants are added. So, should we update the error message for creating a new room? When a user tries to create a room named 'announce,' my suggestion is:

1. If the policyAnnounce room has not been created yet, should show "This name is reserved for future use. Please choose a different name" - This message informs the user that the room doesn't exist yet but could be created in the future.

2. If the policyAnnounce room has been, should show "A room with this name already exists" or "Announce is a default room on all workspaces. Please choose another name." - This clarifies that the room is already created."

What do you think? @Expensify/design @sonialiap

[sonialiap]

@IuliiaHerets can you please test this and confirm the URL of the new announce room you seem to be creating with the mention? I my test the mention links to the workspace announce room, so no new room is being created. I can't repo

Screen.Recording.2024-10-08.at.4.30.24.PM.mov

[MelvinBot]

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

[IuliiaHerets]

@sonialiap QA team can repro this issue. The link from both rooms are different. Also, there are two #announce in LHN

20241010_082743.mp4

[melvin-bot]

@sonialiap Whoops! This issue is 2 days overdue. Let's get this updated quick!

[sonialiap]

Thanks for the replication and showing the URL for the confirmation. Does indeed look like we're duplicating chat rooms which is a bug

[melvin-bot]

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

[melvin-bot]

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

[sonialiap]

@Expensify/design problem: users can create rooms with the name #announce. Two solution ideas:

one

#50173 (comment)

We only create a workspace when 3 or more participants are added. So, should we update the error message for creating a new room? When a user tries to create a room named 'announce,' my suggestion is:

1. If the policyAnnounce room has not been created yet, show "This name is reserved for future use. Please choose a different name" - This message informs the user that the room doesn't exist yet but could be created in the future.
2. If the policyAnnounce room has been, should show "A room with this name already exists" or "Announce is a default room on all workspaces. Please choose another name." - This clarifies that the room is already created.

two

Or do we want to have a single message that says something like, The name #announce is a default room title and cannot be used in a new name, please select a new name. regardless of whether #announce already exists or if it hasn't been created yet?

[allgandalf]

was about to post that thanks @sonialiap 😄

[dannymcclain]

Or do we want to have a single message that says something like, The name #announce is a default room title and cannot be used in a new name, please select a new name. regardless of whether #announce already exists or if it hasn't been created yet?

I think this idea is ok... But also, if the user wants an #announce room, I feel like we should just give it to them at that point. From a technical standpoint I'm not sure how this could work, but this situation is feeling a little silly to me:

User: "Hey I need an #announce room!"
Expensify: "Nope. Can't do it. That's a default workspace room."
User: "Ok. I want it. For my workspace."
Expensify: "Nope. We'll probably automatically create that room later when you have more members."
User: "...Ok. Can I have it now instead?"
Expensify: "Nope 🙃"

Curious what the rest of the @Expensify/design team thinks. But I guess what I'm wondering is if there's any way to just go ahead and actually create the announce room from the get go when a workspace is created (and hide it or something? Unless someone tries to find it?) or to just go ahead and create it if it hasn't been created yet.

[shawnborton]

I think I am more in the camp of keeping #announce as a protected room name since we have some logic that determines if/when the announce room gets created depending on how many members your workspace has. So I basically agree with Sonia's two above.

[allgandalf]

@yuwenmemon should we take inputs from design team

[shawnborton]

Let us know what questions you have, though it seems like the linked comment above is more of a code-related question than it is anything related to the UI/UX.

[melvin-bot]

@yuwenmemon, @sonialiap, @allgandalf, @mkzie2 Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot]

@yuwenmemon, @sonialiap, @allgandalf, @mkzie2 Still overdue 6 days?! Let's take care of this!

[allgandalf]

To address this, we can introduce a new property in the report action, such as linkedActionableActionID, within the #test-room message. This property would store the ID of the actionable message associated with the #test-room message, allowing us to retrieve the actionable message from the #test-room message.

@mkzie2 can you explain more here, i didnt quite understand by:

introduce a new property in the report action, such as linkedActionableActionID, within the #test-room message

This property would store the ID

What id? are you suggesting the reportaction id we randomly generate ?

[mkzie2]

Assume the #test-room and its actionable message are as follows:

{
    "actionName": "ADDCOMMENT",   
    "reportActionID": "5429041207648521625"
},
{
    "actionName": "ACTIONABLEREPORTMENTIONWHISPER",   
    "reportActionID": "5429041207648521629"
}

I propose adding a new property, linkedActionableActionID, to the #test-room message, resulting in:

{
    "actionName": "ADDCOMMENT",   
    "reportActionID": "5429041207648521625",
    "linkedActionableActionID": "5429041207648521629"
},
{
    "actionName": "ACTIONABLEREPORTMENTIONWHISPER",   
    "reportActionID": "5429041207648521629",
}

[melvin-bot]

@yuwenmemon, @sonialiap, @allgandalf, @mkzie2 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

@yuwenmemon, @sonialiap, @allgandalf, @mkzie2 Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot]

@yuwenmemon, @sonialiap, @allgandalf, @mkzie2 Still overdue 6 days?! Let's take care of this!

[allgandalf]

@mkzie2 I don't think so that is desirable

[allgandalf]

@mkzie2 are you still working on this one? can you think of different approach

[allgandalf]

bump @mkzie2

[mkzie2]

• With the new behavior:

1. User type: #test-room (Non existing room).

2. The #test-room message should be highlighted.

3. Then, BE returns the actionable message "Heads up, #test-room doesn't exist yet. Do you want to create it?"

4. The #test-room message is still highlighted.

5. If the user chooses "No", we remove the highlight.
   If the user chooses "Yes," we keep the highlight.

I could only find the solution here #50173 (comment). Let's see what the internal team thinks about it @yuwenmemon.

Alternatively, we could modify the behavior above to:

1. User type: #test-room (Non existing room).

2. The #test-room message should not be highlighted.

3. Then, BE returns the actionable message "Heads up, #test-room doesn't exist yet. Do you want to create it?"

4. The #test-room message should not be highlighted.

5. If the user chooses "No", the #test-room message should not be highlighted.
   If the user chooses "Yes," the #test-room message should be highlighted.

Personally, I believe the second approach makes more sense, as it only highlights #test-room if the room actually exists. Let’s see what the design team thinks @shawnborton.

[dannymcclain]

Personally, I believe the second approach makes more sense, as it only highlights #test-room if the room actually exists. Let’s see what the design team thinks @shawnborton.

Yeah I tend to agree. I think it makes sense to only highlight things we know exist. cc @Expensify/design

[shawnborton]

Totally, that makes sense to me too 👍

[dubielzyk-expensify]

Agree with the designers here as well 👍 Seems clearer to me

[melvin-bot]

@yuwenmemon, @sonialiap, @allgandalf, @mkzie2 Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot]

@yuwenmemon, @sonialiap, @allgandalf, @mkzie2 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[mkzie2]

The PR #56460 is still waiting for @allgandalf review.