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

Show default workspace rooms #7301

Merged
merged 4 commits into from
Jan 24, 2022
Merged

Show default workspace rooms #7301

merged 4 commits into from
Jan 24, 2022

Conversation

neil-marcellini
Copy link
Contributor

@neil-marcellini neil-marcellini commented Jan 18, 2022

Details

Make the default #announce and #admins rooms show up immediately after creating a new workspace. Also, when the workspace is deleted update the room names to show that they have been deleted. For more context see the design doc here.

Deleting workspaces currently doesn't work on IOS and Android. See the issue here.

Fixed Issues

$ #5946

Tests

  1. Log in with a brand new account
  2. Create a workspace from the green plus button at the bottom left.
  3. Check for the #announce and #admins rooms in the left hand navigation.

For all platforms except iOS and Android:

  1. Go to Settings > Workspace
  2. Click the three dots in the header and select delete workspace, and confirm delete.
  3. Confirm that the room names change to #announce (deleted) and #admins (deleted)
  • Verify that no errors appear in the JS console

QA Steps

Same as tests above.

  • Verify that no errors appear in the JS console

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

web

Mobile Web

mobile-web

Desktop

desktop

iOS

IOS

Android

android

@neil-marcellini neil-marcellini requested a review from a team as a code owner January 18, 2022 23:44
@neil-marcellini neil-marcellini self-assigned this Jan 18, 2022
@MelvinBot MelvinBot requested review from nickmurray47 and removed request for a team January 18, 2022 23:45
@neil-marcellini neil-marcellini marked this pull request as draft January 19, 2022 16:46
@neil-marcellini neil-marcellini force-pushed the neil-show-default-rooms branch from b0ca5c2 to eceae08 Compare January 19, 2022 23:10
@neil-marcellini neil-marcellini force-pushed the neil-show-default-rooms branch from eceae08 to 8773bce Compare January 20, 2022 21:42
@neil-marcellini neil-marcellini changed the title [WIP] Show default workspace rooms Show default workspace rooms Jan 21, 2022
@neil-marcellini neil-marcellini marked this pull request as ready for review January 21, 2022 20:27
Copy link
Contributor

@nickmurray47 nickmurray47 left a comment

Choose a reason for hiding this comment

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

Following QA steps, seems like creating a new workspace has doubled the default workspace rooms.

Screen Shot 2022-01-21 at 2 13 35 PM

@neil-marcellini
Copy link
Contributor Author

Following QA steps, seems like creating a new workspace has doubled the default workspace rooms.

Screen Shot 2022-01-21 at 2 13 35 PM

@nickmurray47
Based on the design doc I think that is the expected behavior. Each work space gets its own #announce and #admins rooms. You can see in the subtext which workspace they belong to. I do think it's a bit clunky to have multiple default rooms especially if they both have a default workspace name, but we can always change that down the road. How do you think that should be handled?

I also edited the testing steps to clarify that a brand new account should be used and one workspace should be created with the green plus button.

@nickmurray47
Copy link
Contributor

nickmurray47 commented Jan 22, 2022

ok, thanks for the explanation @neil-marcellini! The code lgtm as far as I can tell but also going to loop in the design doc author just to confirm this is the expected behavior.

I do think it's a bit clunky to have multiple default rooms especially if they both have a default workspace name

Yeah, so there's going to be default rooms for the user when they sign up which will have the same names as the default rooms for the first workspace that's created? It was confusing to me on the first run-through because I'd expect the default rooms for a user to not exist at all then, i.e. the default rooms should only be there once a workspace is created.

@neil-marcellini
Copy link
Contributor Author

Yeah, so there's going to be default rooms for the user when they sign up which will have the same names as the default rooms for the first workspace that's created? It was confusing to me on the first run-through because I'd expect the default rooms for a user to not exist at all then, i.e. the default rooms should only be there once a workspace is created.

When I make new accounts there are no default rooms and no workspace until you press the button to create a workspace. Could you please test again?

@yuwenmemon
Copy link
Contributor

Yeah @nickmurray47 I would guess those default rooms are from some other corporate policy that account has open. They're not getting created with new accounts, right?

@nickmurray47
Copy link
Contributor

When I make new accounts there are no default rooms and no workspace until you press the button to create a workspace. Could you please test again?

Interesting, will re-test again and see if there's any difference. This was with a brand-new account so would assume it was not under any old policies.

Copy link
Contributor

@nickmurray47 nickmurray47 left a comment

Choose a reason for hiding this comment

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

re-tested and QA worked well!

@nickmurray47 nickmurray47 merged commit 2a502f5 into main Jan 24, 2022
@nickmurray47 nickmurray47 deleted the neil-show-default-rooms branch January 24, 2022 22:01
@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.

@mvtglobally
Copy link

@neil-marcellini Should it be tested on All platforms?

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @nickmurray47 in version: 1.1.32-1 🚀

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

@OSBotify
Copy link
Contributor

OSBotify commented Feb 1, 2022

🚀 Deployed to production by @roryabraham in version: 1.1.33-3 🚀

platform result
🤖 android 🤖 success ✅
🖥 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.

5 participants