Skip to content

Support for dynamic slack and rocket channels#6282

Closed
kable-wilmoth wants to merge 4 commits intoRocketChat:developfrom
kable-wilmoth:DynamicGroups
Closed

Support for dynamic slack and rocket channels#6282
kable-wilmoth wants to merge 4 commits intoRocketChat:developfrom
kable-wilmoth:DynamicGroups

Conversation

@kable-wilmoth
Copy link
Copy Markdown

@RocketChat/core

The goal of this pull request was to support Slack and Rocket channels coming and going. You had to enable/disable the slack bridge for it to pick up on changes.

Refactored the code into multiple files to allow for easier contributions in the future.
Fixed a 'not defined' error that was polluting the server logs
Improved logging/debugging messages some more

connection: 'Connection',
events: 'Events',
class: 'Class'
class: 'Class',
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Added some more logger sections

@@ -15,8 +15,13 @@ Package.onUse(function(api) {

api.addFiles('logger.js', 'server');
api.addFiles('settings.js', 'server');
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Broke up the logic from slackbridge into rocket specific and slack specific files

api.addFiles('slack.js', 'server');
api.addFiles('slackbridge.js', 'server');
api.addFiles('slashcommand/slackbridge_import.server.js', 'server');

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Exporting the objects from Rocket and Slack JS files so they could see each other

}

}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This seemed the most appropriate way to resolve 'require/dependency' so the two JS files could see each other

}

}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This seemed the most appropriate way to resolve 'require/dependency' so the two JS files could see each other

@@ -0,0 +1,58 @@
Config:
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I do not know how to implement unit tests inside Rock.Chat yet so I need to keep a script of manual tests I need to run each time

@kable-wilmoth kable-wilmoth mentioned this pull request Mar 8, 2017
@engelgabriel engelgabriel added this to the 0.54.0 milestone Mar 8, 2017
@kable-wilmoth
Copy link
Copy Markdown
Author

Will be updating to use import/export tomorrow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants