[NEW] Improve room types API and usages by mrsimpson · Pull Request #8274 · RocketChat/Rocket.Chat

mrsimpson · 2017-09-23T22:46:11Z

Addresses #8246.

Enhances the API which can be used to add room types
Replaces hard-coded values in reuse-UI-components (channel-settings, members-list)
Extends the spotlight search to include other room types

Custom room type creation UI is explicitly excluded from this PR as this will raise additional questions and is actually independent.
It's being implemented in #8264-room-types-creation-ui, PR will be started once this PR is merged to develop.

Motivation

I implemented custom room types for an enhancement in my master. When testing, I found that I needed some modifications in generic functions (as it was coded to particular room types, probably legacy code).

This PR shall make it possible that no modifications are necessary but that the roomTyes-API generically being consumed.

graywolf336 · 2017-09-25T17:24:40Z

packages/rocketchat-channel-settings/client/views/channelSettings.js

@@ -164,7 +165,10 @@ Template.channelSettings.onCreated(function() {
 			save(value, room) {
 				let nameValidation;
 				if (!RocketChat.authz.hasAllPermission('edit-room', room._id) || (room.t !== 'c' && room.t !== 'p')) {


What if we treat all of the default room types as the other "custom" types and provide sane defaults for them? That way code like this can be merged into one line and not be nested?

@graywolf336 I thought of that as well. I wanted to keep the footprint of this PR as small as possible, 100% compatible - and make the review easier.

@mrsimpson in my opinion, I would rather take the time to review the pull request now than spend time in the future following the path of the code each time it has to be interacted with...if that makes sense.

@graywolf336 This absolutely makes sense! I would propose to modify all places as I started it. Then, in one "final" commit, I would implement the defined methods for the inbuilt room-types. wdyt?

@mrsimpson Whichever way is easier for you will work for me. 👍

@graywolf336 I adapted the hard-coded values with interface calls.

graywolf336 · 2017-09-25T17:25:38Z

I am liking where this is headed, will allow for easy customization for other people when they decide to fork Rocket.Chat and customize it. 👍

…nto core/RocketChat#8264-room-types

mrsimpson · 2017-10-01T11:34:45Z

packages/rocketchat-lib/lib/roomTypesCommon.js

+
+		if (!config.canBeDeleted) {
+			config.canBeDeleted = function(room) {
+				return Meteor.isServer ?


@graywolf336 Unfortunately, the client- and server-side-method-signatures vary.
I preferred having this switch in the generic code compared to having dedicated client-side-files or redundantly implementing this. WDYT?

mrsimpson · 2017-10-01T11:36:03Z

@graywolf336 Can you please verify the merge-ability? I feel I'm done with the scope I need.
I'm sure I didn't hit all the hard-coded-places, but I'd shift it until there's a need

mrsimpson · 2017-10-05T21:42:49Z

@graywolf336 I implemented a tabbed creation dialog in assistify@3459fa8 . As discussed, I'll make that UI-stuff part of another PR once you accept this one ;)

…l over their definitions beyound the expected.

This allows for each different type of room to define which setting it wants to display instead of them being hard coded on the settings tab controller.

mrsimpson · 2017-10-13T07:23:55Z

packages/rocketchat-custom-sounds/client/admin/adminSounds.js

@@ -1,4 +1,5 @@
-/* globals RocketChatTabBar */
+import { RocketChatTabBar } from 'meteor/rocketchat:lib';


@graywolf336 well, it seems you're on a globals-killing-spree already ;)

Heh, figured I would start the process ;)

mrsimpson · 2017-10-13T07:26:22Z

packages/rocketchat-lib/client/lib/index.js

+	What is this file? Great question! To make Rocket.Chat more "modular"
+	and to make the "rocketchat:lib" package more of a core package
+	with the libraries, this index file contains the exported members
+	for the *client* pieces of code which does include the shared


@graywolf336 does that not affect the server as well?

It does, but that file is specific to the client. The server has another one dedicated to it.

…ng custom room types

graywolf336

I'm excited to have these changes, the Rocket.Chat Apps will benefit greatly from this as it proves the ground for them to provide custom room types.

…r the user is authorized to create the room type

Provide implementation for the confirmation dialogs on leave and hide

(cherry picked from commit b463b84)

graywolf336 · 2017-11-04T07:30:58Z

packages/rocketchat-lib/lib/RoomTypesCommon.js

 			}
-			return FlowRouter.route(config.route.path, routeConfig);
+
+			return FlowRouter.route(roomConfig.route.path, routeConfig);


Just realized, this method say we return void yet here we return whatever FlowRouter.route returns. 🤔

What are your thoughts @mrsimpson on changing this method?

I believe it should rerun void. It’s an anti pattern in some parts of the RC code to just make the last statement of a function return - without meaningful semantics

E. G. Create a bunch of settings and the last statement is return this.add()

graywolf336 · 2017-12-04T21:06:43Z

Closing in favor of #9009 so that we can merge it due to conflicts.

mrsimpson added 3 commits September 24, 2017 00:34

enable channel settings for custom room types

882a8d2

enable members list for custom room types

cf6ea72

satisfy es-lint

0c53dd3

RocketChat deleted a comment Sep 25, 2017

graywolf336 reviewed Sep 25, 2017

View reviewed changes

mrsimpson added 6 commits September 27, 2017 23:20

added option to specify creation template for custom room types

0e4f824

custom room types: prevent renaming

929e846

custom room types: include in global search

3321156

Custom room type: handle not-provided creation template

9d2381e

Enable custom room type creation templates

c62f9e8

Provide wrapper for room creation for better extensibility

65c4eec

mrsimpson mentioned this pull request Oct 1, 2017

Decouple 'requests and topics' from RocketChat core - based on new UI assistify/Rocket.Chat#19

Closed

mrsimpson added 2 commits October 1, 2017 10:03

Merge branch 'develop' of https://github.com/RocketChat/Rocket.Chat i…

5b6b5f8

…nto core/RocketChat#8264-room-types

Replace hard-coded room types with "interface"-method-calls

06226f2

mrsimpson commented Oct 1, 2017

View reviewed changes

thx, codacy

b2a425f

RocketChat deleted a comment Oct 2, 2017

mrsimpson changed the title ~~[NEW] Enable channel settings for custom room types~~ [NEW] Improve room types AI and usages Oct 9, 2017

mrsimpson mentioned this pull request Oct 9, 2017

Decouple help requests assistify/Rocket.Chat#75

Closed

Room definitions are more typed but extended classes have full contro…

37c3672

…l over their definitions beyound the expected.

geekgonecrazy changed the title ~~[NEW] Improve room types AI and usages~~ [NEW] Improve room types API and usages Oct 12, 2017

graywolf336 added 2 commits October 12, 2017 16:12

Move the settings configuration over to an ENUM based.

d4059ae

This allows for each different type of room to define which setting it wants to display instead of them being hard coded on the settings tab controller.

Fix the tests failing due to some items not being on all settings

17e7549

mrsimpson commented Oct 13, 2017

View reviewed changes

[DOC] Update the rocketchat:lib readme with information about providi…

a7c04e1

…ng custom room types

Fix the room name's not being displayed in the admin rooms

685593a

graywolf336 approved these changes Oct 16, 2017

View reviewed changes

graywolf336 added this to the 0.60.0 milestone Oct 16, 2017

mrsimpson added 2 commits October 17, 2017 13:40

Add creation validation configuration, e. g. in order to check whethe…

fa20b86

…r the user is authorized to create the room type

Adapt warning-text for leaving a room. Fixes #92

b2b09a3

ruKurz mentioned this pull request Oct 24, 2017

[Proxy] Custom room types (Rocket.Chat#8246) assistify/Rocket.Chat#55

Closed

mrsimpson force-pushed the core/#8264-room-types branch from 0481a5c to b2b09a3 Compare October 24, 2017 11:22

mrsimpson mentioned this pull request Oct 24, 2017

Core/#8264 room types creation ui #8629

Closed

2 tasks

mrsimpson added 5 commits October 26, 2017 07:51

Move profile in membersList to interface method

03e23cf

merge develop for smaller diff

5f3cce9

Add interface method for UI-texts.

a480c20

Provide implementation for the confirmation dialogs on leave and hide

Added a closing text constant

c5c0ece

Allow room types to provide an own "no subscriptions yet" text

916afa5

(cherry picked from commit b463b84)

graywolf336 reviewed Nov 4, 2017

View reviewed changes

graywolf336 mentioned this pull request Dec 4, 2017

[NEW] Improve room types API and usages #9009

Merged

graywolf336 closed this Dec 4, 2017

mrsimpson deleted the core/#8264-room-types branch December 7, 2020 09:08

		@@ -1,4 +1,5 @@
		/* globals RocketChatTabBar */
		import { RocketChatTabBar } from 'meteor/rocketchat:lib';

Conversation

mrsimpson commented Sep 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrsimpson Oct 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

graywolf336 commented Sep 25, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrsimpson commented Oct 1, 2017

Uh oh!

mrsimpson commented Oct 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

graywolf336 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

graywolf336 commented Dec 4, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mrsimpson commented Sep 23, 2017 •

edited

Loading

mrsimpson Oct 1, 2017 •

edited

Loading

mrsimpson commented Oct 5, 2017 •

edited

Loading