From 22f95c677a25b20926637f5a6fa6e191993e745f Mon Sep 17 00:00:00 2001 From: Brandon Murnane Date: Wed, 2 Aug 2017 22:39:14 +0100 Subject: [PATCH] Adding settings to room and bot (#444) --- api/v1/swagger.yaml | 12 +--- db/helpers/botUtils.js | 26 +++++++ db/model/bot.js | 8 +++ db/model/room.js | 22 +++++- db/model/roomType.js | 5 +- ...20170731100411-settings-array-to-object.js | 43 ++++++++++++ tests/api/v1/roomTypes/utils.js | 28 +++----- tests/api/v1/rooms/delete.js | 1 + tests/api/v1/rooms/get.js | 1 + tests/api/v1/rooms/patch.js | 1 + tests/api/v1/rooms/utils.js | 2 - tests/db/model/bot/create.js | 2 + tests/db/model/bot/utils.js | 6 ++ tests/db/model/room/create.js | 31 +++++++++ tests/db/model/roomType/create.js | 68 ------------------- tests/db/model/roomType/utils.js | 14 ++-- 16 files changed, 155 insertions(+), 115 deletions(-) create mode 100644 migrations/20170731100411-settings-array-to-object.js diff --git a/api/v1/swagger.yaml b/api/v1/swagger.yaml index 1837f8d92e..497f6048f9 100644 --- a/api/v1/swagger.yaml +++ b/api/v1/swagger.yaml @@ -7936,10 +7936,8 @@ paths: type: boolean description: If the roomType is active settings: - type: array + type: object description: Default settings for roomType - items: - type: object rules: type: array description: Default rules for roomType @@ -8068,10 +8066,8 @@ paths: type: boolean description: If the roomType is active settings: - type: array + type: object description: Default settings for roomType - items: - type: object rules: type: array description: Default rules for roomType @@ -10414,9 +10410,7 @@ definitions: isEnabled: type: boolean settings: - type: array - items: - type: object + type: object rules: type: array items: diff --git a/db/helpers/botUtils.js b/db/helpers/botUtils.js index cf28b20392..f2d98d7866 100644 --- a/db/helpers/botUtils.js +++ b/db/helpers/botUtils.js @@ -38,6 +38,13 @@ const parameterArraySchema = Joi.alternatives().try( ) ); +const settingsArraySchema = Joi.array().items( + Joi.object().keys({ + key: Joi.string().regex(/^[0-9a-z_-]+$/i).max(254).required(), + helpText: Joi.string().regex(/^\w+(\s\w+)*$/i).required(), + }) + ); + const actionArraySchema = Joi.array().items( Joi.object().keys({ name: Joi.string().regex(/^[0-9a-z_-]+$/i).required(), @@ -98,8 +105,27 @@ function validateDataArray(arr) { } } // validateDataArray +/** + * Custom validation rule that checks the settings array to have + * all valid entries. Meaning each element contains key and a value. + * + * @param {Array} arr - The array to test + * @returns {undefined} - OK + * @throws {validationError} - Invalid settings array + */ +function validateSettingsArray(arr) { + const result = Joi.validate(arr, settingsArraySchema); + + if (result.error !== null) { + throw new ValidationError({ + message: result.error.details, + }); + } +} // validateSettings + module.exports = { arrayHasValidParameters, validateActionArray, validateDataArray, + validateSettingsArray, }; // exports diff --git a/db/model/bot.js b/db/model/bot.js index 412de771e1..b811a45bee 100644 --- a/db/model/bot.js +++ b/db/model/bot.js @@ -68,6 +68,14 @@ module.exports = function bot(seq, dataTypes) { }, comment: 'List of actions a Bot can take', }, + settings: { + type: dataTypes.ARRAY(dataTypes.JSONB), + allowNull: true, + validate: { + contains: u.validateSettingsArray, + }, + comment: 'Array[ {key: name of key, helpText: describe the value it is looking for},]', + }, data: { type: dataTypes.ARRAY(dataTypes.JSON), allowNull: true, diff --git a/db/model/room.js b/db/model/room.js index c938f71233..8438754c63 100644 --- a/db/model/room.js +++ b/db/model/room.js @@ -14,6 +14,7 @@ */ const constants = require('../constants'); +const u = require('../helpers/roomTypeUtils'); const assoc = {}; @@ -33,6 +34,11 @@ module.exports = function room(seq, dataTypes) { }, comment: 'Create a named room ', }, + settings: { + type: dataTypes.JSON, + allowNull: true, + comment: 'Key/Value pairs for user specific settings', + }, active: { type: dataTypes.BOOLEAN, defaultValue: false, @@ -46,8 +52,11 @@ module.exports = function room(seq, dataTypes) { postImport(models) { assoc.type = Room.belongsTo(models.RoomType, { - foreignKey: 'type', - allowNull: false, + foreignKey: { + name: 'type', + allowNull: false, + }, + onDelete: 'CASCADE', }); assoc.writers = Room.belongsToMany(models.User, { as: 'writers', @@ -56,6 +65,15 @@ module.exports = function room(seq, dataTypes) { }); }, }, + hooks: { + afterCreate: (instance) => { + const RoomType = seq.models.RoomType; + return RoomType.findById(instance.type) + .then((roomType) => { + instance.settings = roomType.settings; + }); + }, + }, }); return Room; }; diff --git a/db/model/roomType.js b/db/model/roomType.js index 2294cf4542..3b5b5208bf 100644 --- a/db/model/roomType.js +++ b/db/model/roomType.js @@ -47,11 +47,8 @@ module.exports = function roomType(seq, dataTypes) { comment: 'Determines if room type is still enabled for use', }, settings: { - type: dataTypes.ARRAY(dataTypes.JSONB), + type: dataTypes.JSON, allowNull: true, - validate: { - contains: u.validateSettingsArray, - }, comment: 'Key/Value pairs for user specific settings', }, rules: { diff --git a/migrations/20170731100411-settings-array-to-object.js b/migrations/20170731100411-settings-array-to-object.js new file mode 100644 index 0000000000..b516e15abb --- /dev/null +++ b/migrations/20170731100411-settings-array-to-object.js @@ -0,0 +1,43 @@ +/** + * Copyright (c) 2017, salesforce.com, inc. + * All rights reserved. + * Licensed under the BSD 3-Clause license. + * For full license text, see LICENSE.txt file in the repo root or + * https://opensource.org/licenses/BSD-3-Clause + */ +'use strict'; +const u = require('../db/helpers/roomTypeUtils'); + +module.exports = { + up: function (qi, Sequelize) { + /* + Add altering commands here. + Return a promise to correctly handle asynchronicity. + + Example: + return queryInterface.createTable('users', { id: Sequelize.INTEGER }); + */ + return qi.changeColumn('RoomType', 'settings', { + type: Sequelize.JSON, + allowNull: true, + }); + }, + + down: function (qi, Sequelize) { + /* + Add reverting commands here. + Return a promise to correctly handle asynchronicity. + + Example: + return queryInterface.dropTable('users'); + */ + + return qi.changeColumn('RoomType', 'settings', { + type: Sequelize.ARRAY(Sequelize.JSONB), + allowNull: true, + validate: { + contains: u.validateSettingsArray, + }, + }); + }, +}; diff --git a/tests/api/v1/roomTypes/utils.js b/tests/api/v1/roomTypes/utils.js index 81863de8c7..0f1788e4fc 100644 --- a/tests/api/v1/roomTypes/utils.js +++ b/tests/api/v1/roomTypes/utils.js @@ -20,16 +20,10 @@ const n2 = n+'NonActive'; const standard = { name: n, isEnabled: true, - settings: [ - { - key: 'Key1', - value: 'Value1', - }, - { - key: 'Key2', - value: 'Value2', - }, - ], + settings: { + Key1: 'Value1', + Key2: 'Value2', + }, rules: [ { rule: { @@ -92,16 +86,10 @@ const standard = { const nonActive = { name: n2, isEnabled: false, - settings: [ - { - key: 'Key1', - value: 'Value1', - }, - { - key: 'Key2', - value: 'Value2', - }, - ], + settings: { + Key1: 'Value1', + Key2: 'Value2', + }, rules: [ { rule: { diff --git a/tests/api/v1/rooms/delete.js b/tests/api/v1/rooms/delete.js index 7738b2bacb..a43f5a6208 100644 --- a/tests/api/v1/rooms/delete.js +++ b/tests/api/v1/rooms/delete.js @@ -23,6 +23,7 @@ const RoomType = tu.db.RoomType; const v = require('../roomTypes/utils'); describe(`api: DELETE ${path}`, () => { + let testRoomType; let testRoom; let token; diff --git a/tests/api/v1/rooms/get.js b/tests/api/v1/rooms/get.js index ec44743306..0f38912171 100644 --- a/tests/api/v1/rooms/get.js +++ b/tests/api/v1/rooms/get.js @@ -26,6 +26,7 @@ const RoomType = tu.db.RoomType; const v = require('../roomTypes/utils'); describe(`api: GET ${path}`, () => { + let testRoomType; let testRoom; let token; diff --git a/tests/api/v1/rooms/patch.js b/tests/api/v1/rooms/patch.js index 9314144ba3..f754859d0d 100644 --- a/tests/api/v1/rooms/patch.js +++ b/tests/api/v1/rooms/patch.js @@ -24,6 +24,7 @@ const RoomType = tu.db.RoomType; const v = require('../roomTypes/utils'); describe(`api: PATCH ${path}`, () => { + let testRoomType; let testRoom; let token; diff --git a/tests/api/v1/rooms/utils.js b/tests/api/v1/rooms/utils.js index e86e4e1f8c..825f9e7894 100644 --- a/tests/api/v1/rooms/utils.js +++ b/tests/api/v1/rooms/utils.js @@ -89,8 +89,6 @@ const roomTypeSchema = { ], }; -const roomType = tu.db.RoomType.create(roomTypeSchema); - const standard = { name: n, active: true, diff --git a/tests/db/model/bot/create.js b/tests/db/model/bot/create.js index 9362808f0d..b09f31cfce 100644 --- a/tests/db/model/bot/create.js +++ b/tests/db/model/bot/create.js @@ -32,6 +32,8 @@ describe('db: bot: create: ', () => { expect(o.actions[0].parameters.length).to.equal(4); expect(o).to.have.property('data'); expect(o.data.length).to.equal(5); + expect(o).to.have.property('settings'); + expect(o.settings.length).to.equal(1); done(); }) .catch(done); diff --git a/tests/db/model/bot/utils.js b/tests/db/model/bot/utils.js index 549f481f79..d1fe8eed1d 100644 --- a/tests/db/model/bot/utils.js +++ b/tests/db/model/bot/utils.js @@ -25,6 +25,12 @@ const standard = { url: 'http://www.bar.com', ui: uiBlob, active: true, + settings: [ + { + key: 'key1', + helpText: 'help Text 1' + } + ], actions: [ { name: 'Action1', diff --git a/tests/db/model/room/create.js b/tests/db/model/room/create.js index 42acf10916..eae93185cf 100644 --- a/tests/db/model/room/create.js +++ b/tests/db/model/room/create.js @@ -38,6 +38,21 @@ describe('db: room: create: ', () => { .catch(done); }); + it('ok, room created settings default', (done) => { + RoomType.create(v.getStandard()) + .then((roomType) => { + const room = u.getStandard(); + room.type = roomType.id; + return Room.create(room); + }) + .then((o) => { + expect(o).to.have.property('settings'); + expect(o.settings.Key1).to.equal('Value1'); + done(); + }) + .catch(done); + }); + it('ok, room created active false', (done) => { RoomType.create(v.getStandard()) .then((roomType) => { @@ -70,5 +85,21 @@ describe('db: room: create: ', () => { }) .catch(done); }); + + it('fail, room type null', (done) => { + RoomType.create(v.getStandard()) + .then((roomType) => { + const room = u.getStandard(); + room.type = null; + return Room.create(room); + }) + .then(() => done(tu.valError)) + .catch((err) => { + expect(err.message.toLowerCase()).to.contain('notnull violation'); + done(); + }) + .catch(done); + }); + }); }); diff --git a/tests/db/model/roomType/create.js b/tests/db/model/roomType/create.js index 1fc935a304..96a7a8b94b 100644 --- a/tests/db/model/roomType/create.js +++ b/tests/db/model/roomType/create.js @@ -58,74 +58,6 @@ describe('db: roomType: create: ', () => { .catch(done); }); - it('fail, room type setting missing value', (done) => { - const roomtype = u.getStandard(); - roomtype.settings = [ - { - key: 'Key1', - }, - ]; - RoomType.create(roomtype) - .then(() => done(tu.valError)) - .catch((err) => { - expect(err.name).to.equal(tu.valErrorName); - expect(err.message.toLowerCase()).to.contain('validation error'); - done(); - }) - .catch(done); - }); - - it('fail, room type setting missing key', (done) => { - const roomtype = u.getStandard(); - roomtype.settings = [ - { - value: 'value1', - }, - ]; - RoomType.create(roomtype) - .then(() => done(tu.valError)) - .catch((err) => { - expect(err.name).to.equal(tu.valErrorName); - expect(err.message.toLowerCase()).to.contain('validation error'); - done(); - }) - .catch(done); - }); - - it('fail, room type setting key invalid', (done) => { - const roomtype = u.getStandard(); - roomtype.settings = [ - { - key: invalidValue, - value: 'value1', - }, - ]; - RoomType.create(roomtype) - .then(() => done(tu.valError)) - .catch((err) => { - expect(err.name).to.equal(tu.valErrorName); - expect(err.message.toLowerCase()).to.contain('validation error'); - done(); - }) - .catch(done); - }); - - it('fail, room type settings not array', (done) => { - const roomtype = u.getStandard(); - roomtype.settings = { - key: 'Key1', - value: 'Value1', - }; - RoomType.create(roomtype) - .then(() => done(tu.valError)) - .catch((err) => { - expect(err.name).to.equal(tu.valErrorName); - expect(err.message.toLowerCase()).to.contain('validation error'); - done(); - }) - .catch(done); - }); - it('fail, room type rules not array', (done) => { const roomtype = u.getStandard(); roomtype.rules = { diff --git a/tests/db/model/roomType/utils.js b/tests/db/model/roomType/utils.js index 5e329e4395..400087ce9f 100644 --- a/tests/db/model/roomType/utils.js +++ b/tests/db/model/roomType/utils.js @@ -19,16 +19,10 @@ const n = `${tu.namePrefix}TestRoomType`; const standard = { name: n, isEnabled: true, - settings: [ - { - key: 'Key1', - value: 'Value1', - }, - { - key: 'Key2', - value: 'Value2', - }, - ], + settings: { + Key1: 'Value1', + Key2: 'Value2', + }, rules: [ { rule: {