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

[NEW] Broadcast Channels #9950

Merged
merged 9 commits into from
Apr 21, 2018
Merged

[NEW] Broadcast Channels #9950

merged 9 commits into from
Apr 21, 2018

Conversation

ggazzo
Copy link
Member

@ggazzo ggazzo commented Mar 1, 2018

@RocketChat/core
closes #9556

75viehlmgc

hdzmmiwp4q

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-9950 March 1, 2018 03:58 Inactive
@ggazzo ggazzo temporarily deployed to rocket-chat-pr-9950 March 1, 2018 04:08 Inactive
@RocketChat RocketChat deleted a comment Mar 1, 2018
@RocketChat RocketChat deleted a comment Mar 1, 2018
@RocketChat RocketChat deleted a comment Mar 1, 2018
@@ -219,10 +219,10 @@ Template.channelSettingsEditing.onCreated(function() {
isToggle: true,
processing: new ReactiveVar(false),
canView() {
return RocketChat.roomTypes.roomTypes[room.t].allowRoomSettingChange(room, RoomSettingsEnum.READ_ONLY);
return !room.broadcast && RocketChat.roomTypes.roomTypes[room.t].allowRoomSettingChange(room, RoomSettingsEnum.READ_ONLY);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we please create a new room type of broadcast instead of polluting these methods. The room types api was intentionally changed for these style of changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

I will take a look. @graywolf336

Copy link
Member

@sampaiodiego sampaiodiego Mar 1, 2018

Choose a reason for hiding this comment

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

@graywolf336 I think for this case we need a new property instead of a new room type, because we thought cool having the ability to chose within a public or private broadcast room, so this way we're using all validations already implemented.

edit: ok looking at the final code I got you meant @graywolf336 😁 it is still a new room property but is using roomTypes methods 👍

@ggazzo ggazzo temporarily deployed to rocket-chat-pr-9950 March 1, 2018 06:46 Inactive
@ggazzo ggazzo temporarily deployed to rocket-chat-pr-9950 March 1, 2018 06:49 Inactive
@ggazzo ggazzo temporarily deployed to rocket-chat-pr-9950 March 1, 2018 06:51 Inactive
@ggazzo ggazzo temporarily deployed to rocket-chat-pr-9950 March 1, 2018 06:58 Inactive
@RocketChat RocketChat deleted a comment Mar 1, 2018
@RocketChat RocketChat deleted a comment Mar 1, 2018
@RocketChat RocketChat deleted a comment Mar 1, 2018
@sampaiodiego sampaiodiego temporarily deployed to rocket-chat-pr-9950 March 1, 2018 12:06 Inactive
@RocketChat RocketChat deleted a comment Mar 1, 2018
@RocketChat RocketChat deleted a comment Mar 1, 2018
@RocketChat RocketChat deleted a comment Mar 1, 2018
Copy link
Member

@sampaiodiego sampaiodiego left a comment

Choose a reason for hiding this comment

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

Looks like public broadcast channels does not work.

@ggazzo ggazzo temporarily deployed to rocket-chat-pr-9950 March 1, 2018 20:01 Inactive
@ggazzo ggazzo temporarily deployed to rocket-chat-pr-9950 March 1, 2018 20:18 Inactive
@sampaiodiego sampaiodiego added this to the 0.64.0 milestone Apr 12, 2018
@sampaiodiego sampaiodiego added this to Desireable in 0.64.0 via automation Apr 12, 2018
@theorenck theorenck moved this from Desireable to Ready to merge in 0.64.0 Apr 18, 2018
@theorenck theorenck moved this from Ready to merge to Review/QA in 0.64.0 Apr 19, 2018
@ggazzo ggazzo temporarily deployed to rocket-chat-pr-9950 April 20, 2018 21:31 Inactive
@sampaiodiego sampaiodiego temporarily deployed to rocket-chat-pr-9950 April 20, 2018 21:52 Inactive
sampaiodiego
sampaiodiego previously approved these changes Apr 20, 2018
@sampaiodiego sampaiodiego changed the title [WIP][NEW] Broadcast Channels [NEW] Broadcast Channels Apr 20, 2018
@sampaiodiego sampaiodiego temporarily deployed to rocket-chat-pr-9950 April 20, 2018 22:23 Inactive
@sampaiodiego sampaiodiego dismissed graywolf336’s stale review April 20, 2018 22:25

I'll make sure to create an issue to do the changes requested.

@sampaiodiego sampaiodiego moved this from Review/QA to Ready to merge in 0.64.0 Apr 20, 2018
sampaiodiego
sampaiodiego previously approved these changes Apr 20, 2018
@rodrigok rodrigok temporarily deployed to rocket-chat-pr-9950 April 20, 2018 23:51 Inactive
@rodrigok rodrigok merged commit 136e9ca into develop Apr 21, 2018
0.64.0 automation moved this from Ready to merge to Done Apr 21, 2018
@rodrigok rodrigok deleted the broadcast branch April 21, 2018 01:44
@rodrigok rodrigok mentioned this pull request Apr 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
0.64.0
  
Done
Development

Successfully merging this pull request may close these issues.

Broadcast channels
5 participants