[CP Staging] Fix same name policy rooms and policy room title and mobile app crashing when creating policy room #6724
Conversation
|
|
MelvinBot
requested review from
aldo-expensify
and removed request for
a team
mountiny
changed the title
| // For a basic policy room, return its original name | ||
| if (ReportUtils.isPolicyRoom({chatType})) { | ||
| return fullReport.reportName; | ||
| } |
There was a problem hiding this comment.
The isDefaultPolicy does not check for basic policy room, so we need to add this check here too.
| const isDefaultChatRoom = ReportUtils.isDefaultRoom(props.report); | ||
| const title = isDefaultChatRoom | ||
| const title = isDefaultChatRoom || isPolicyRoom |
There was a problem hiding this comment.
We need to show similar style of title for both, default and policy rooms. I need to keep them separate though as the default dooms have subtitle and avatar, which is a different behaviour and design than policy room.
There was a problem hiding this comment.
Aha! I should've known there would be consequences for the UI being incomplete on mobile. Good work @mountiny. Good thing this is behind a beta.
|
Since there is fixing a deploy blocker, I'm going to merge this rn. |
|
@TomatoToaster looks like this was merged without passing tests. Please add a note explaining why this was done and remove the |
[CP Staging] Fix same name policy rooms and policy room title and mobile app crashing when creating policy room (cherry picked from commit ab6b887)
|
Hmmm weird note because it definitely passed the checks on commit: |
|
🚀 Deployed to staging by @TomatoToaster in version: 1.1.20-3 🚀
|
|
🚀 Deployed to production by @Julesssss in version: 1.1.21-1 🚀
|
cc @TomatoToaster asking you for a review as you have worked on the original PR.
Adding
CP Staginglabel as it fixes two deploy blockers.Details
For the new Create room page, we are fixing parsing the
reportName. Currently, we have been turning thereportNameinto an empty string, therefore we have not recognized that the value is same (if nothing is typed, the value is#).At the same time, on mobile as the
reportNameis an empty string, this line is false and nothing is shown in theViewcomponent, which causes the app to fail. By making sure thereportNamewont be an empty string, this should not happen.This also fixes the policy room title not showing.
Fixed Issues
$ #6709
$ #6712
$ #6731
Tests
<baseURL>/workspace/new-room.test_room_nameinto the room name input and select a workspace. Click "Create Room". Verify that the room is created and that you navigate to the room (<baseURL>/r/<reportID>).<baseURL>/workspace/new-room. Select the same workspace you used in [4]. Then, typetest_room_nameinto the Room Name input. Verify that theA room with this name already existserror message displays (in real-time as you finish typingtest_room_name).eoff of the room name. Verify the error disappears. Add the lasteback, and verify the error appears again.QA Steps
Same as the above tests with account which has appropriate betas accessible.
Tested On
Screenshots
web.mp4
With the policy room title:
