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

Add the NewRoomPage #6314

Merged
merged 53 commits into from
Dec 10, 2021
Merged

Add the NewRoomPage #6314

merged 53 commits into from
Dec 10, 2021

Conversation

jasperhuangg
Copy link
Contributor

@jasperhuangg jasperhuangg commented Nov 16, 2021

Details

This adds the New Room Page, which allows users to create a new room on a workspace.

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/176850

Tests

  1. Click "New Room" in the global create menu. Verify it takes you to <baseURL>/workspace/new-room.
  2. Verify the Room Name input functions correctly:
    • Type spaces, verify that they appear as underscores.
    • Try to type non-alphanumeric characters (besides underscores) into the room name input, verify nothing shows up.
    • Turn your caps-lock and type in the room name input, verify everything appears as lowercase.
  3. Click the dropdown. Verify that all the workspaces (policies) shown match up to the "Free Plan" policies shown for the same account in OldDot (Settings > Policies)
  4. Type test_room_name into 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>).
    • NOTE: The room name in the header, the room's entry in the LHN, and the default text won't show up correctly yet. Just verify that you are navigated to a new report. I will open up a separate PR for this, since it'll make this one too cluttered.
  5. Navigate back to <baseURL>/workspace/new-room. Select the same workspace you used in [4]. Then, type test_room_name into the Room Name input. Verify that the A room with this name already exists error message displays (in real-time as you finish typing test_room_name).
  6. Change the workspace in the workspace dropdown. Verify the error disappears. Reselect the same workspace, and verify the error appears again.
  7. Delete the last e off of the room name. Verify the error disappears. Add the last e back, and verify the error appears again.

QA Steps

Same as the above tests, just use these credentials on staging:

  • username: margrett952@labeledhf.com
  • password: asdfASDF00

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

web.mp4

Mobile Web

mweb

Desktop

desktop.mp4

iOS

ios.mp4

Android

android.mp4

@jasperhuangg jasperhuangg self-assigned this Nov 16, 2021
@jasperhuangg jasperhuangg marked this pull request as ready for review November 18, 2021 20:48
@jasperhuangg jasperhuangg requested a review from a team as a code owner November 18, 2021 20:48
@MelvinBot MelvinBot removed the request for review from a team November 18, 2021 20:48
@TomatoToaster TomatoToaster self-assigned this Dec 8, 2021
@TomatoToaster
Copy link
Contributor

@yuwenmemon, @sketchydroide, this is ready for another look I've resolved the merge conflicts and put this feature behind the default rooms beta (that made the most sense to me).

I decided not to add the UI for the Report page in this PR, so after creating a room it will look blank and weird (like in the test steps). I'm working on a report details page for Policy Rooms so I'll add that UI in another PR instead.

@yuwenmemon
Copy link
Contributor

Nice, thanks for taking this over @TomatoToaster!

Copy link
Contributor

@yuwenmemon yuwenmemon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@yuwenmemon
Copy link
Contributor

@sketchydroide all yours

@sketchydroide sketchydroide merged commit 45ce0ee into main Dec 10, 2021
@sketchydroide sketchydroide deleted the jasper-ucprRNFrontEnd branch December 10, 2021 15:07
@OSBotify
Copy link
Contributor

✋ 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
Copy link
Contributor

🚀 Deployed to staging by @sketchydroide in version: 1.1.19-5 🚀

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

@mountiny
Copy link
Contributor

@TomatoToaster Seems like there was a regression found on staging here regarding the error when the same room name is inputted.

@mountiny
Copy link
Contributor

mountiny commented Dec 12, 2021

A PR to fix the mal-functioning room name check can be found here.

I have also tried the credentials listed in PR on staging, but there was no option to create a new room there so I assume that account does not have the betas required to QA this.

@TomatoToaster
Copy link
Contributor

Just wanted to note here officially, since there have a lot of bugs around this PR. The core test/QA for this PR do pass on staging. The other issues created capture some bugs that I'll resolve in separate PRs.

image

image

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @Julesssss in version: 1.1.21-1 🚀

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants