From 0996a0b14090eb145ce44476525cc1dfd0da98e8 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 14 Apr 2016 19:52:57 +0100 Subject: [PATCH 1/2] Fixes to refactored notifications A few things I managed to break in the recent refactor --- src/notifications/PushRuleVectorState.js | 6 ++- src/notifications/StandardActions.js | 30 ++++++++++++ .../VectorPushRulesDefinitions.js | 48 ++++++++----------- 3 files changed, 53 insertions(+), 31 deletions(-) create mode 100644 src/notifications/StandardActions.js diff --git a/src/notifications/PushRuleVectorState.js b/src/notifications/PushRuleVectorState.js index 5c6934aa..019ce8a9 100644 --- a/src/notifications/PushRuleVectorState.js +++ b/src/notifications/PushRuleVectorState.js @@ -16,6 +16,8 @@ limitations under the License. 'use strict'; +var StandardActions = require('./StandardActions'); + /** * Enum for state of a push rule as defined by the Vector UI. * @readonly @@ -39,10 +41,10 @@ module.exports = { */ actionsFor: function(pushRuleVectorState) { if (pushRuleVectorState === this.ON) { - return ACTION_NOTIFY; + return StandardActions.ACTION_NOTIFY; } else if (pushRuleVectorState === this.LOUD) { - return ACTION_HIGHLIGHT_DEFAULT_SOUND; + return StandardActions.ACTION_HIGHLIGHT_DEFAULT_SOUND; } }, diff --git a/src/notifications/StandardActions.js b/src/notifications/StandardActions.js new file mode 100644 index 00000000..22a8f1db --- /dev/null +++ b/src/notifications/StandardActions.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. +*/ + +'use strict'; + +var NotificationUtils = require('./NotificationUtils'); + +var encodeActions = NotificationUtils.encodeActions; + +module.exports = { + ACTION_NOTIFY: encodeActions({notify: true}), + ACTION_NOTIFY_DEFAULT_SOUND: encodeActions({notify: true, sound: "default"}), + ACTION_NOTIFY_RING_SOUND: encodeActions({notify: true, sound: "ring"}), + ACTION_HIGHLIGHT_DEFAULT_SOUND: encodeActions({notify: true, sound: "default", highlight: true}), + ACTION_DONT_NOTIFY: encodeActions({notify: false}), + ACTION_DISABLED: null, +}; diff --git a/src/notifications/VectorPushRulesDefinitions.js b/src/notifications/VectorPushRulesDefinitions.js index 8e2a0a65..cae28a0f 100644 --- a/src/notifications/VectorPushRulesDefinitions.js +++ b/src/notifications/VectorPushRulesDefinitions.js @@ -16,17 +16,7 @@ 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; +var StandardActions = require('./StandardActions'); /** * The descriptions of rules managed by the Vector UI. @@ -38,9 +28,9 @@ module.exports = { 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 + on: StandardActions.ACTION_NOTIFY, + loud: StandardActions.ACTION_HIGHLIGHT_DEFAULT_SOUND, + off: StandardActions.ACTION_DISABLED } }, @@ -49,9 +39,9 @@ module.exports = { kind: "underride", description: "Messages in one-to-one chats", vectorStateToActions: { - on: ACTION_NOTIFY, - loud: ACTION_NOTIFY_DEFAULT_SOUND, - off: ACTION_DONT_NOTIFY + on: StandardActions.ACTION_NOTIFY, + loud: StandardActions.ACTION_NOTIFY_DEFAULT_SOUND, + off: StandardActions.ACTION_DONT_NOTIFY } }, @@ -62,9 +52,9 @@ module.exports = { kind: "underride", description: "Messages in group chats", vectorStateToActions: { - on: ACTION_NOTIFY, - loud: ACTION_NOTIFY_DEFAULT_SOUND, - off: ACTION_DONT_NOTIFY + on: StandardActions.ACTION_NOTIFY, + loud: StandardActions.ACTION_NOTIFY_DEFAULT_SOUND, + off: StandardActions.ACTION_DONT_NOTIFY } }, @@ -73,9 +63,9 @@ module.exports = { kind: "underride", description: "When I'm invited to a room", vectorStateToActions: { - on: ACTION_NOTIFY, - loud: ACTION_NOTIFY_DEFAULT_SOUND, - off: ACTION_DISABLED + on: StandardActions.ACTION_NOTIFY, + loud: StandardActions.ACTION_NOTIFY_DEFAULT_SOUND, + off: StandardActions.ACTION_DISABLED } }, @@ -84,9 +74,9 @@ module.exports = { kind: "underride", description: "Call invitation", vectorStateToActions: { - on: ACTION_NOTIFY, - loud: ACTION_NOTIFY_RING_SOUND, - off: ACTION_DISABLED + on: StandardActions.ACTION_NOTIFY, + loud: StandardActions.ACTION_NOTIFY_RING_SOUND, + off: StandardActions.ACTION_DISABLED } }, @@ -96,9 +86,9 @@ module.exports = { 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, + on: StandardActions.ACTION_DISABLED, + loud: StandardActions.ACTION_NOTIFY_DEFAULT_SOUND, + off: StandardActions.ACTION_DONT_NOTIFY, } } }; From c6b501811fe7481f06e097aad95a2124c709dbc6 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 14 Apr 2016 14:28:28 +0100 Subject: [PATCH 2/2] Move more stuff out of Notifications.js --- .../views/settings/Notifications.js | 79 ++--------- src/notifications/ContentRules.js | 125 ++++++++++++++++++ src/notifications/index.js | 1 + 3 files changed, 135 insertions(+), 70 deletions(-) create mode 100644 src/notifications/ContentRules.js diff --git a/src/components/views/settings/Notifications.js b/src/components/views/settings/Notifications.js index 2518c420..aec14e6e 100644 --- a/src/components/views/settings/Notifications.js +++ b/src/components/views/settings/Notifications.js @@ -30,6 +30,7 @@ var notifications = require('../../../notifications'); var NotificationUtils = notifications.NotificationUtils; var VectorPushRulesDefinitions = notifications.VectorPushRulesDefinitions; var PushRuleVectorState = notifications.PushRuleVectorState; +var ContentRules = notifications.ContentRules; /** * Rules that Vector used to set in order to override the actions of default rules. @@ -429,8 +430,6 @@ module.exports = React.createClass({ // HS default rules var defaultRules = {master: [], vector: {}, others: []}; - // Content/keyword rules - var contentRules = {on: [], on_but_disabled:[], loud: [], loud_but_disabled: [], other: []}; for (var kind in rulesets.global) { for (var i = 0; i < Object.keys(rulesets.global[kind]).length; ++i) { @@ -449,82 +448,22 @@ module.exports = React.createClass({ defaultRules['others'].push(r); } } - else if (kind === 'content') { - switch (PushRuleVectorState.contentRuleVectorStateKind(r)) { - case PushRuleVectorState.ON: - if (r.enabled) { - contentRules.on.push(r); - } - else { - contentRules.on_but_disabled.push(r); - } - break; - case PushRuleVectorState.LOUD: - if (r.enabled) { - contentRules.loud.push(r); - } - else { - contentRules.loud_but_disabled.push(r); - } - break; - default: - contentRules.other.push(r); - break; - } - } } } - // Decide which content rules to display in Vector UI. - // Vector displays a single global rule for a list of keywords - // whereas Matrix has a push rule per keyword. - // Vector can set the unique rule in ON, LOUD or OFF state. - // Matrix has enabled/disabled plus a combination of (highlight, sound) tweaks. - - // The code below determines which set of user's content push rules can be - // displayed by the vector UI. - // Push rules that does not fit, ie defined by another Matrix client, ends - // in self.state.externalContentRules. - // There is priority in the determination of which set will be the displayed one. - // The set with rules that have LOUD tweaks is the first choice. Then, the ones - // with ON tweaks (no tweaks). - if (contentRules.loud.length) { - self.state.vectorContentRules = { - vectorState: PushRuleVectorState.LOUD, - rules: contentRules.loud - } - self.state.externalContentRules = [].concat(contentRules.loud_but_disabled, contentRules.on, contentRules.on_but_disabled, contentRules.other); - } - else if (contentRules.loud_but_disabled.length) { - self.state.vectorContentRules = { - vectorState: PushRuleVectorState.OFF, - rules: contentRules.loud_but_disabled - } - self.state.externalContentRules = [].concat(contentRules.on, contentRules.on_but_disabled, contentRules.other); - } - else if (contentRules.on.length) { - self.state.vectorContentRules = { - vectorState: PushRuleVectorState.ON, - rules: contentRules.on - } - self.state.externalContentRules = [].concat(contentRules.on_but_disabled, contentRules.other); - } - else if (contentRules.on_but_disabled.length) { - self.state.vectorContentRules = { - vectorState: PushRuleVectorState.OFF, - rules: contentRules.on_but_disabled - } - self.state.externalContentRules = contentRules.other; - } - else { - self.state.externalContentRules = contentRules.other; - } - // Get the master rule if any defined by the hs if (defaultRules.master.length > 0) { self.state.masterPushRule = defaultRules.master[0]; } + // parse the keyword rules into our state + var contentRules = ContentRules.parseContentRules(rulesets); + self.state.vectorContentRules = { + vectorState: contentRules.vectorState, + rules: contentRules.rules, + }; + self.state.externalContentRules = contentRules.externalRules; + // Build the rules displayed in the Vector UI matrix table self.state.vectorPushRules = []; diff --git a/src/notifications/ContentRules.js b/src/notifications/ContentRules.js new file mode 100644 index 00000000..25a7bac9 --- /dev/null +++ b/src/notifications/ContentRules.js @@ -0,0 +1,125 @@ +/* +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 PushRuleVectorState = require('./PushRuleVectorState'); + +module.exports = { + /** + * Extract the keyword rules from a list of rules, and parse them + * into a form which is useful for Vector's UI. + * + * Returns an object containing: + * rules: the primary list of keyword rules + * vectorState: a PushRuleVectorState indicating whether those rules are + * OFF/ON/LOUD + * externalRules: a list of other keyword rules, with states other than + * vectorState + */ + parseContentRules: function(rulesets) { + // first categorise the keyword rules in terms of their actions + var contentRules = this._categoriseContentRules(rulesets); + + // Decide which content rules to display in Vector UI. + // Vector displays a single global rule for a list of keywords + // whereas Matrix has a push rule per keyword. + // Vector can set the unique rule in ON, LOUD or OFF state. + // Matrix has enabled/disabled plus a combination of (highlight, sound) tweaks. + + // The code below determines which set of user's content push rules can be + // displayed by the vector UI. + // Push rules that does not fit, ie defined by another Matrix client, ends + // in externalRules. + // There is priority in the determination of which set will be the displayed one. + // The set with rules that have LOUD tweaks is the first choice. Then, the ones + // with ON tweaks (no tweaks). + + if (contentRules.loud.length) { + return { + vectorState: PushRuleVectorState.LOUD, + rules: contentRules.loud, + externalRules: [].concat(contentRules.loud_but_disabled, contentRules.on, contentRules.on_but_disabled, contentRules.other), + }; + } + else if (contentRules.loud_but_disabled.length) { + return { + vectorState: PushRuleVectorState.OFF, + rules: contentRules.loud_but_disabled, + externalRules: [].concat(contentRules.on, contentRules.on_but_disabled, contentRules.other), + }; + } + else if (contentRules.on.length) { + return { + vectorState: PushRuleVectorState.ON, + rules: contentRules.on, + externalRules: [].concat(contentRules.on_but_disabled, contentRules.other), + }; + } + else if (contentRules.on_but_disabled.length) { + return { + vectorState: PushRuleVectorState.OFF, + rules: contentRules.on_but_disabled, + externalRules: contentRules.other, + } + } else { + return { + vectorState: PushRuleVectorState.ON, + rules: [], + externalRules: contentRules.other, + } + } + }, + + _categoriseContentRules: function(rulesets) { + var contentRules = {on: [], on_but_disabled:[], loud: [], loud_but_disabled: [], other: []}; + for (var kind in rulesets.global) { + for (var i = 0; i < Object.keys(rulesets.global[kind]).length; ++i) { + var r = rulesets.global[kind][i]; + + // check it's not a default rule + if (r.rule_id[0] === '.' || kind !== 'content') { + continue; + } + + r.kind = kind; // is this needed? not sure + + switch (PushRuleVectorState.contentRuleVectorStateKind(r)) { + case PushRuleVectorState.ON: + if (r.enabled) { + contentRules.on.push(r); + } + else { + contentRules.on_but_disabled.push(r); + } + break; + case PushRuleVectorState.LOUD: + if (r.enabled) { + contentRules.loud.push(r); + } + else { + contentRules.loud_but_disabled.push(r); + } + break; + default: + contentRules.other.push(r); + break; + } + } + } + return contentRules; + }, +}; diff --git a/src/notifications/index.js b/src/notifications/index.js index 9672b67c..8ed77e9d 100644 --- a/src/notifications/index.js +++ b/src/notifications/index.js @@ -20,4 +20,5 @@ module.exports = { NotificationUtils: require('./NotificationUtils'), PushRuleVectorState: require('./PushRuleVectorState'), VectorPushRulesDefinitions: require('./VectorPushRulesDefinitions'), + ContentRules: require('./ContentRules'), };