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 Sandstorm activity/notification events #3743

Merged
merged 2 commits into from
Aug 3, 2016

Conversation

jparyani
Copy link
Contributor

@RocketChat/core

Sandstorm recently grew new mechanisms for notifications (see sandstorm-io/sandstorm#2174). This PR disables normal desktop notifications under Sandstorm and replaces them with the new Sandstorm notification system. I tried to be as non-invasive as possible, with the bulk of the custom code going in a new package named rocketchat-sandstorm.

I did end up editing rocketchat-lib/sendMessage to include the sandstorm session id. The only other invasive change was in rocketchat-lib/sendNotificationsOnMessage, where I thought it made the most sense to re-use the current notification logic. The new method RocketChat.Sandstorm.notify is a no-op under non-Sandstorm environments.

@jparyani jparyani force-pushed the add-sandstorm-notifications branch from 30fb960 to 31e8a2a Compare July 15, 2016 21:54
@jparyani jparyani force-pushed the add-sandstorm-notifications branch from 31e8a2a to 3044a50 Compare July 29, 2016 19:51
@Sing-Li
Copy link
Member

Sing-Li commented Aug 1, 2016

@rodrigok will this PR have any interaction with the soon-to-be-released (fusion engine) optimizations? If so, please comment on how it should be adapted. Thanks.

@jparyani jparyani force-pushed the add-sandstorm-notifications branch 2 times, most recently from 25291b4 to 4243c9b Compare August 2, 2016 21:00
@rodrigok
Copy link
Member

rodrigok commented Aug 3, 2016

@jparyani Can you fix the conflicts for us?

@jparyani jparyani force-pushed the add-sandstorm-notifications branch from 4243c9b to cd8b0b2 Compare August 3, 2016 00:59
@jparyani
Copy link
Contributor Author

jparyani commented Aug 3, 2016

Rebased and ready to merge.

@rodrigok rodrigok merged commit 0a4da11 into RocketChat:develop Aug 3, 2016
@marceloschmidt marceloschmidt added this to the 0.37.0 milestone Sep 5, 2016
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