From ff5dff45f540756c425ae6159edf1284f684db2d Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 13 Apr 2016 17:16:03 +0100 Subject: [PATCH] Start Notifications component refactor Factor some things out of the mega Notifications component, and add a dummy unit test to show willing --- .../views/settings/Notifications.js | 227 ++---------------- src/notifications/NotificationUtils.js | 89 +++++++ src/notifications/PushRuleVectorState.js | 78 ++++++ .../VectorPushRulesDefinitions.js | 104 ++++++++ src/notifications/index.js | 23 ++ test/all-tests.js | 10 +- .../notifications/PushRuleVectorState-test.js | 30 +++ 7 files changed, 348 insertions(+), 213 deletions(-) create mode 100644 src/notifications/NotificationUtils.js create mode 100644 src/notifications/PushRuleVectorState.js create mode 100644 src/notifications/VectorPushRulesDefinitions.js create mode 100644 src/notifications/index.js create mode 100644 test/unit-tests/notifications/PushRuleVectorState-test.js diff --git a/src/components/views/settings/Notifications.js b/src/components/views/settings/Notifications.js index 6831d788..2518c420 100644 --- a/src/components/views/settings/Notifications.js +++ b/src/components/views/settings/Notifications.js @@ -22,174 +22,14 @@ var MatrixClientPeg = require('matrix-react-sdk/lib/MatrixClientPeg'); var UserSettingsStore = require('matrix-react-sdk/lib/UserSettingsStore'); var Modal = require('matrix-react-sdk/lib/Modal'); -/** - * Enum for state of a push rule as defined by the Vector UI. - * @readonly - * @enum {string} - */ -var PushRuleVectorState = { - /** The push rule is disabled */ - OFF: "off", - /** The user will receive push notification for this rule */ - ON: "on", - /** The user will receive push notification for this rule with sound and - highlight if this is legitimate */ - LOUD: "loud", -}; +var notifications = require('../../../notifications'); -// Encodes a dictionary of { -// "notify": true/false, -// "sound": string or undefined, -// "highlight: true/false, -// } -// to a list of push actions. -function encodeActions(action) { - var notify = action.notify; - var sound = action.sound; - var highlight = action.highlight; - if (notify) { - var actions = ["notify"]; - if (sound) { - actions.push({"set_tweak": "sound", "value": sound}); - } - if (highlight) { - actions.push({"set_tweak": "highlight"}); - } else { - actions.push({"set_tweak": "highlight", "value": false}); - } - return actions; - } else { - return ["dont_notify"]; - } -} +// TODO: this "view" component still has far to much application logic in it, +// which should be factored out to other files. -// Decode a list of actions to a dictionary of { -// "notify": true/false, -// "sound": string or undefined, -// "highlight: true/false, -// } -// If the actions couldn't be decoded then returns null. -function decodeActions(actions) { - var notify = false; - var sound = null; - var highlight = false; - - for (var i = 0; i < actions.length; ++i) { - var action = actions[i]; - if (action === "notify") { - notify = true; - } else if (action === "dont_notify") { - notify = false; - } else if (typeof action === 'object') { - if (action.set_tweak === "sound") { - sound = action.value - } else if (action.set_tweak === "highlight") { - highlight = action.value; - } else { - // We don't understand this kind of tweak, so give up. - return null; - } - } else { - // We don't understand this kind of action, so give up. - return null; - } - } - - if (highlight === undefined) { - // If a highlight tweak is missing a value then it defaults to true. - highlight = true; - } - - var result = {notify: notify, highlight: highlight}; - if (sound !== null) { - result.sound = sound; - } - return result; -} - -var ACTION_NOTIFY = encodeActions({notify: true}); -var ACTION_NOTIFY_DEFAULT_SOUND = encodeActions({notify: true, sound: "default"}); -var ACTION_NOTIFY_RING_SOUND = encodeActions({notify: true, sound: "ring"}); -var ACTION_HIGHLIGHT_DEFAULT_SOUND = encodeActions({notify: true, sound: "default", highlight: true}); -var ACTION_DONT_NOTIFY = encodeActions({notify: false}); -var ACTION_DISABLED = null; - - -/** - * The descriptions of rules managed by the Vector UI. - */ -var VectorPushRulesDefinitions = { - - // Messages containing user's display name - // (skip contains_user_name which is too geeky) - ".m.rule.contains_display_name": { - kind: "underride", - description: "Messages containing my name", - vectorStateToActions: { // The actions for each vector state, or null to disable the rule. - on: ACTION_NOTIFY, - loud: ACTION_HIGHLIGHT_DEFAULT_SOUND, - off: ACTION_DISABLED - } - }, - - // Messages just sent to the user in a 1:1 room - ".m.rule.room_one_to_one": { - kind: "underride", - description: "Messages in one-to-one chats", - vectorStateToActions: { - on: ACTION_NOTIFY, - loud: ACTION_NOTIFY_DEFAULT_SOUND, - off: ACTION_DONT_NOTIFY - } - }, - - // Messages just sent to a group chat room - // 1:1 room messages are catched by the .m.rule.room_one_to_one rule if any defined - // By opposition, all other room messages are from group chat rooms. - ".m.rule.message": { - kind: "underride", - description: "Messages in group chats", - vectorStateToActions: { - on: ACTION_NOTIFY, - loud: ACTION_NOTIFY_DEFAULT_SOUND, - off: ACTION_DONT_NOTIFY - } - }, - - // Invitation for the user - ".m.rule.invite_for_me": { - kind: "underride", - description: "When I'm invited to a room", - vectorStateToActions: { - on: ACTION_NOTIFY, - loud: ACTION_NOTIFY_DEFAULT_SOUND, - off: ACTION_DISABLED - } - }, - - // Incoming call - ".m.rule.call": { - kind: "underride", - description: "Call invitation", - vectorStateToActions: { - on: ACTION_NOTIFY, - loud: ACTION_NOTIFY_RING_SOUND, - off: ACTION_DISABLED - } - }, - - // Notifications from bots - ".m.rule.suppress_notices": { - kind: "override", - description: "Messages sent by bot", - vectorStateToActions: { - // .m.rule.suppress_notices is a "negative" rule, we have to invert its enabled value for vector UI - on: ACTION_DISABLED, - loud: ACTION_NOTIFY_DEFAULT_SOUND, - off: ACTION_DONT_NOTIFY, - } - } -}; +var NotificationUtils = notifications.NotificationUtils; +var VectorPushRulesDefinitions = notifications.VectorPushRulesDefinitions; +var PushRuleVectorState = notifications.PushRuleVectorState; /** * Rules that Vector used to set in order to override the actions of default rules. @@ -206,9 +46,9 @@ var LEGACY_RULES = { }; function portLegacyActions(actions) { - var decoded = decodeActions(actions); + var decoded = NotificationUtils.decodeActions(actions); if (decoded !== null) { - return encodeActions(decoded); + return NotificationUtils.encodeActions(decoded); } else { // We don't recognise one of the actions here, so we don't try to // canonicalise them. @@ -216,7 +56,6 @@ function portLegacyActions(actions) { } } - module.exports = React.createClass({ displayName: 'Notififications', @@ -330,40 +169,6 @@ module.exports = React.createClass({ } }, - _actionsFor: function(pushRuleVectorState) { - if (pushRuleVectorState === PushRuleVectorState.ON) { - return ACTION_NOTIFY; - } - else if (pushRuleVectorState === PushRuleVectorState.LOUD) { - return ACTION_HIGHLIGHT_DEFAULT_SOUND; - } - }, - - // Determine whether a content rule is in the PushRuleVectorState.ON category or in PushRuleVectorState.LOUD - // regardless of its enabled state. Returns undefined if it does not match these categories. - _contentRuleVectorStateKind: function(rule) { - var stateKind; - - // Count tweaks to determine if it is a ON or LOUD rule - var tweaks = 0; - for (var j in rule.actions) { - var action = rule.actions[j]; - if (action.set_tweak === 'sound' || - (action.set_tweak === 'highlight' && action.value)) { - tweaks++; - } - } - switch (tweaks) { - case 0: - stateKind = PushRuleVectorState.ON; - break; - case 2: - stateKind = PushRuleVectorState.LOUD; - break; - } - return stateKind; - }, - _setPushRuleVectorState: function(rule, newPushRuleVectorState) { if (rule && rule.vectorState !== newPushRuleVectorState) { @@ -379,7 +184,7 @@ module.exports = React.createClass({ if (rule.rule) { var actions = ruleDefinition.vectorStateToActions[newPushRuleVectorState]; - if (actions === ACTION_DISABLED) { + if (!actions) { // The new state corresponds to disabling the rule. deferreds.push(cli.setPushRuleEnabled('global', rule.rule.kind, rule.rule.rule_id, false)); } @@ -425,7 +230,7 @@ module.exports = React.createClass({ switch (newPushRuleVectorState) { case PushRuleVectorState.ON: if (rule.actions.length !== 1) { - actions = this._actionsFor(PushRuleVectorState.ON); + actions = PushRuleVectorState.actionsFor(PushRuleVectorState.ON); } if (this.state.vectorContentRules.vectorState === PushRuleVectorState.OFF) { @@ -435,7 +240,7 @@ module.exports = React.createClass({ case PushRuleVectorState.LOUD: if (rule.actions.length !== 3) { - actions = this._actionsFor(PushRuleVectorState.LOUD); + actions = PushRuleVectorState.actionsFor(PushRuleVectorState.LOUD); } if (this.state.vectorContentRules.vectorState === PushRuleVectorState.OFF) { @@ -521,7 +326,7 @@ module.exports = React.createClass({ // when creating the new rule. // Thus, this new rule will join the 'vectorContentRules' set. if (self.state.vectorContentRules.rules.length) { - pushRuleVectorStateKind = self._contentRuleVectorStateKind(self.state.vectorContentRules.rules[0]); + pushRuleVectorStateKind = PushRuleVectorState.contentRuleVectorStateKind(self.state.vectorContentRules.rules[0]); } else { // ON is default @@ -536,13 +341,13 @@ module.exports = React.createClass({ if (self.state.vectorContentRules.vectorState !== PushRuleVectorState.OFF) { deferreds.push(cli.addPushRule ('global', 'content', keyword, { - actions: self._actionsFor(pushRuleVectorStateKind), + actions: PushRuleVectorState.actionsFor(pushRuleVectorStateKind), pattern: keyword })); } else { deferreds.push(self._addDisabledPushRule('global', 'content', keyword, { - actions: self._actionsFor(pushRuleVectorStateKind), + actions: PushRuleVectorState.actionsFor(pushRuleVectorStateKind), pattern: keyword })); } @@ -645,7 +450,7 @@ module.exports = React.createClass({ } } else if (kind === 'content') { - switch (self._contentRuleVectorStateKind(r)) { + switch (PushRuleVectorState.contentRuleVectorStateKind(r)) { case PushRuleVectorState.ON: if (r.enabled) { contentRules.on.push(r); @@ -757,7 +562,7 @@ module.exports = React.createClass({ var state = PushRuleVectorState[stateKey]; var vectorStateToActions = ruleDefinition.vectorStateToActions[state]; - if (vectorStateToActions === ACTION_DISABLED) { + if (!vectorStateToActions) { // No defined actions means that this vector state expects a disabled default hs rule if (rule.enabled === false) { vectorState = state; diff --git a/src/notifications/NotificationUtils.js b/src/notifications/NotificationUtils.js new file mode 100644 index 00000000..c8aeb468 --- /dev/null +++ b/src/notifications/NotificationUtils.js @@ -0,0 +1,89 @@ +/* +Copyright 2016 OpenMarket Ltd + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +'use strict'; + +module.exports = { + // Encodes a dictionary of { + // "notify": true/false, + // "sound": string or undefined, + // "highlight: true/false, + // } + // to a list of push actions. + encodeActions: function(action) { + var notify = action.notify; + var sound = action.sound; + var highlight = action.highlight; + if (notify) { + var actions = ["notify"]; + if (sound) { + actions.push({"set_tweak": "sound", "value": sound}); + } + if (highlight) { + actions.push({"set_tweak": "highlight"}); + } else { + actions.push({"set_tweak": "highlight", "value": false}); + } + return actions; + } else { + return ["dont_notify"]; + } + }, + + // Decode a list of actions to a dictionary of { + // "notify": true/false, + // "sound": string or undefined, + // "highlight: true/false, + // } + // If the actions couldn't be decoded then returns null. + decodeActions: function(actions) { + var notify = false; + var sound = null; + var highlight = false; + + for (var i = 0; i < actions.length; ++i) { + var action = actions[i]; + if (action === "notify") { + notify = true; + } else if (action === "dont_notify") { + notify = false; + } else if (typeof action === 'object') { + if (action.set_tweak === "sound") { + sound = action.value + } else if (action.set_tweak === "highlight") { + highlight = action.value; + } else { + // We don't understand this kind of tweak, so give up. + return null; + } + } else { + // We don't understand this kind of action, so give up. + return null; + } + } + + if (highlight === undefined) { + // If a highlight tweak is missing a value then it defaults to true. + highlight = true; + } + + var result = {notify: notify, highlight: highlight}; + if (sound !== null) { + result.sound = sound; + } + return result; + }, +}; diff --git a/src/notifications/PushRuleVectorState.js b/src/notifications/PushRuleVectorState.js new file mode 100644 index 00000000..5c6934aa --- /dev/null +++ b/src/notifications/PushRuleVectorState.js @@ -0,0 +1,78 @@ +/* +Copyright 2016 OpenMarket Ltd + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +'use strict'; + +/** + * Enum for state of a push rule as defined by the Vector UI. + * @readonly + * @enum {string} + */ +module.exports = { + /** The push rule is disabled */ + OFF: "off", + + /** The user will receive push notification for this rule */ + ON: "on", + + /** The user will receive push notification for this rule with sound and + highlight if this is legitimate */ + LOUD: "loud", + + /** + * Convert a PushRuleVectorState to a list of actions + * + * @return [object] list of push-rule actions + */ + actionsFor: function(pushRuleVectorState) { + if (pushRuleVectorState === this.ON) { + return ACTION_NOTIFY; + } + else if (pushRuleVectorState === this.LOUD) { + return ACTION_HIGHLIGHT_DEFAULT_SOUND; + } + }, + + /** + * Convert a pushrule's actions to a PushRuleVectorState. + * + * Determines whether a content rule is in the PushRuleVectorState.ON + * category or in PushRuleVectorState.LOUD, regardless of its enabled + * state. Returns undefined if it does not match these categories. + */ + contentRuleVectorStateKind: function(rule) { + var stateKind; + + // Count tweaks to determine if it is a ON or LOUD rule + var tweaks = 0; + for (var j in rule.actions) { + var action = rule.actions[j]; + if (action.set_tweak === 'sound' || + (action.set_tweak === 'highlight' && action.value)) { + tweaks++; + } + } + switch (tweaks) { + case 0: + stateKind = this.ON; + break; + case 2: + stateKind = this.LOUD; + break; + } + return stateKind; + }, +}; diff --git a/src/notifications/VectorPushRulesDefinitions.js b/src/notifications/VectorPushRulesDefinitions.js new file mode 100644 index 00000000..8e2a0a65 --- /dev/null +++ b/src/notifications/VectorPushRulesDefinitions.js @@ -0,0 +1,104 @@ +/* +Copyright 2016 OpenMarket Ltd + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +'use strict'; + +var NotificationUtils = require('./NotificationUtils'); + +var encodeActions = NotificationUtils.encodeActions; +var decodeActions = NotificationUtils.decodeActions; + +const ACTION_NOTIFY = encodeActions({notify: true}); +const ACTION_NOTIFY_DEFAULT_SOUND = encodeActions({notify: true, sound: "default"}); +const ACTION_NOTIFY_RING_SOUND = encodeActions({notify: true, sound: "ring"}); +const ACTION_HIGHLIGHT_DEFAULT_SOUND = encodeActions({notify: true, sound: "default", highlight: true}); +const ACTION_DONT_NOTIFY = encodeActions({notify: false}); +const ACTION_DISABLED = null; + +/** + * The descriptions of rules managed by the Vector UI. + */ +module.exports = { + // Messages containing user's display name + // (skip contains_user_name which is too geeky) + ".m.rule.contains_display_name": { + kind: "underride", + description: "Messages containing my name", + vectorStateToActions: { // The actions for each vector state, or null to disable the rule. + on: ACTION_NOTIFY, + loud: ACTION_HIGHLIGHT_DEFAULT_SOUND, + off: ACTION_DISABLED + } + }, + + // Messages just sent to the user in a 1:1 room + ".m.rule.room_one_to_one": { + kind: "underride", + description: "Messages in one-to-one chats", + vectorStateToActions: { + on: ACTION_NOTIFY, + loud: ACTION_NOTIFY_DEFAULT_SOUND, + off: ACTION_DONT_NOTIFY + } + }, + + // Messages just sent to a group chat room + // 1:1 room messages are catched by the .m.rule.room_one_to_one rule if any defined + // By opposition, all other room messages are from group chat rooms. + ".m.rule.message": { + kind: "underride", + description: "Messages in group chats", + vectorStateToActions: { + on: ACTION_NOTIFY, + loud: ACTION_NOTIFY_DEFAULT_SOUND, + off: ACTION_DONT_NOTIFY + } + }, + + // Invitation for the user + ".m.rule.invite_for_me": { + kind: "underride", + description: "When I'm invited to a room", + vectorStateToActions: { + on: ACTION_NOTIFY, + loud: ACTION_NOTIFY_DEFAULT_SOUND, + off: ACTION_DISABLED + } + }, + + // Incoming call + ".m.rule.call": { + kind: "underride", + description: "Call invitation", + vectorStateToActions: { + on: ACTION_NOTIFY, + loud: ACTION_NOTIFY_RING_SOUND, + off: ACTION_DISABLED + } + }, + + // Notifications from bots + ".m.rule.suppress_notices": { + kind: "override", + description: "Messages sent by bot", + vectorStateToActions: { + // .m.rule.suppress_notices is a "negative" rule, we have to invert its enabled value for vector UI + on: ACTION_DISABLED, + loud: ACTION_NOTIFY_DEFAULT_SOUND, + off: ACTION_DONT_NOTIFY, + } + } +}; diff --git a/src/notifications/index.js b/src/notifications/index.js new file mode 100644 index 00000000..9672b67c --- /dev/null +++ b/src/notifications/index.js @@ -0,0 +1,23 @@ +/* +Copyright 2016 OpenMarket Ltd + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +'use strict'; + +module.exports = { + NotificationUtils: require('./NotificationUtils'), + PushRuleVectorState: require('./PushRuleVectorState'), + VectorPushRulesDefinitions: require('./VectorPushRulesDefinitions'), +}; diff --git a/test/all-tests.js b/test/all-tests.js index ec300820..2ed16932 100644 --- a/test/all-tests.js +++ b/test/all-tests.js @@ -3,5 +3,11 @@ // Our master test file: uses the webpack require API to find our test files // and run them -var context = require.context('./app-tests', true, /\.jsx?$/); -context.keys().forEach(context); +// ideally these unit tests could be run under nodejs rather than in a browser +// via karma, but having two separate test frameworks in the same project +// seems confusing +var unit_tests = require.context('./unit-tests', true, /\.js$/); +unit_tests.keys().forEach(unit_tests); + +var app_tests = require.context('./app-tests', true, /\.jsx?$/); +app_tests.keys().forEach(app_tests); diff --git a/test/unit-tests/notifications/PushRuleVectorState-test.js b/test/unit-tests/notifications/PushRuleVectorState-test.js new file mode 100644 index 00000000..48084f08 --- /dev/null +++ b/test/unit-tests/notifications/PushRuleVectorState-test.js @@ -0,0 +1,30 @@ +/* +Copyright 2016 OpenMarket Ltd + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +var notifications = require('notifications'); + +var prvs = notifications.PushRuleVectorState; + +var expect = require('expect'); + +describe("PushRuleVectorState", function() { + describe("contentRuleVectorStateKind", function() { + it("should understand normal notifications", function () { + expect(prvs.contentRuleVectorStateKind(["notify"])). + toEqual(prvs.ON); + }); + }); +});