From 0a748d7d5898779dc15e078107f69145c13cbc46 Mon Sep 17 00:00:00 2001 From: Brian Hyder Date: Mon, 5 Sep 2016 20:23:16 -0400 Subject: [PATCH] #1136 Plugin Settings Groupings --- .../plugins/plugin_validation_service.js | 12 ++- .../admin/plugins/plugin_settings.js | 80 +++++++++++++++++-- .../admin/plugins/plugin_settings.html | 54 +++++++------ plugins/sample/details.json | 22 +++-- plugins/sample/public/localization/en-US.json | 12 ++- .../plugin_validation_service_tests.js | 41 +++++++--- 6 files changed, 172 insertions(+), 49 deletions(-) diff --git a/include/service/entities/plugins/plugin_validation_service.js b/include/service/entities/plugins/plugin_validation_service.js index e38b63a05..215cee93b 100644 --- a/include/service/entities/plugins/plugin_validation_service.js +++ b/include/service/entities/plugins/plugin_validation_service.js @@ -290,8 +290,18 @@ module.exports = function (pb) { return cb(null, BaseObjectService.validationFailure(fieldPrefix, 'The setting value must be an object')); } - //validate name + //validate displayName var errors = []; + if (!v.isNonEmptyStr(setting.displayName, false)) { + errors.push(BaseObjectService.validationFailure(fieldPrefix+'.displayName', 'The setting display name must be a non-empty string')); + } + + //validate group + if (!v.isNonEmptyStr(setting.group, false)) { + errors.push(BaseObjectService.validationFailure(fieldPrefix+'.group', 'The setting group must be a non-empty string')); + } + + //validate name if (!v.isNonEmptyStr(setting.name, true)) { errors.push(BaseObjectService.validationFailure(fieldPrefix+'.name', 'The setting name must be provided')); } diff --git a/plugins/pencilblue/controllers/admin/plugins/plugin_settings.js b/plugins/pencilblue/controllers/admin/plugins/plugin_settings.js index 321f98120..ecbbac498 100755 --- a/plugins/pencilblue/controllers/admin/plugins/plugin_settings.js +++ b/plugins/pencilblue/controllers/admin/plugins/plugin_settings.js @@ -14,11 +14,12 @@ You should have received a copy of the GNU General Public License along with this program. If not, see . */ +'use strict'; module.exports = function(pb) { //pb dependencies - var util = pb.util; + var util = pb.util; /** * Interface for changing a plugin's settings @@ -83,8 +84,9 @@ module.exports = function(pb) { for (var i = 0; i < clone.length; i++) { var item = clone[i]; - item.displayName = item.name.split('_').join(' '); - item.displayName = item.displayName.charAt(0).toUpperCase() + item.displayName.slice(1); + item.id = item.name.split(' ').join('_'); + item.heading = self.deriveHeadingLabel(item); + item.displayName = self.deriveDisplayLabel(item); if (item.value === true || item.value === false) { item.type = 'checkbox'; @@ -92,11 +94,17 @@ module.exports = function(pb) { else if (util.isString(item.value)) { item.type = 'text'; } - else if (!isNaN()) { + else if (!isNaN(item.value)) { item.type = 'number'; } } + // First sort the settings alphabetically, then by heading/group + clone.sort(sortOn("displayName")); + clone.sort(sortOn("heading")); + + clone = self.insertGroupHeadings(clone); + var tabs = [ { active: 'active', @@ -129,6 +137,66 @@ module.exports = function(pb) { }); }; + PluginSettingsFormController.prototype.insertGroupHeadings = function(clone){ + if (clone.length === 0) { + return clone; + } + + var items = []; + var previous = {}; + for (var i = 0; i < clone.length; i++) { + if (previous.heading !== clone[i].heading) { + items.push({id: clone[i].heading, isHeading: true, heading: clone[i].heading}); + } + items.push(clone[i]); + previous = clone[i]; + } + return items; + }; + + function sortOn(property){ + return function(a, b){ + // Forces default heading to appear at the top of the sets of grouped settings + if(property === 'heading' && a[property] === 'default'){ + return -1; + } + else if(a[property] < b[property]){ + return -1; + }else if(a[property] > b[property]){ + return 1; + }else{ + return 0; + } + }; + } + + PluginSettingsFormController.prototype.deriveDisplayLabel = function(item){ + var label; + if (item.displayName) { + label = this.ls.g(item.displayName); + if (!label) { + label = item.displayName; + } + } + if (!label) { + label = item.name.split('_').join(' '); + label = label.charAt(0).toUpperCase() + label.slice(1); + } + return label; + }; + + PluginSettingsFormController.prototype.deriveHeadingLabel = function(item) { + var label; + if (item.group) { + label = this.ls.g(item.group); + } + if (!label || label === item.group) { + label = (item.group || 'default').split('_').join(' '); + label = label.charAt(0).toUpperCase() + label.slice(1); + } + return label; + }; + PluginSettingsFormController.prototype.post = function(cb) { var self = this; @@ -295,10 +363,10 @@ module.exports = function(pb) { */ PluginSettingsFormController.validateValue = function(value, type) { if (type === 'boolean') { - return value !== null && value !== undefined && (value === true || value === false || value === 1 || value === 0 || value == '1' || value === '0' || value.toLowerCase() === 'true' || value.toLowerCase() === 'false'); + return value !== null && value !== undefined && (value === true || value === false || value === 1 || value === 0 || value === '1' || value === '0' || value.toLowerCase() === 'true' || value.toLowerCase() === 'false'); } else if (type === 'string') { - return pb.validation.validateStr(value, true); + return pb.validation.isStr(value, true); } else if (type === 'number') { return !isNaN(value); diff --git a/plugins/pencilblue/templates/admin/plugins/plugin_settings.html b/plugins/pencilblue/templates/admin/plugins/plugin_settings.html index 4066b2fbd..3d121930a 100755 --- a/plugins/pencilblue/templates/admin/plugins/plugin_settings.html +++ b/plugins/pencilblue/templates/admin/plugins/plugin_settings.html @@ -1,29 +1,35 @@ ^tmp_admin=head^
- ^tmp_admin=elements=error_success^ - ^tmp_admin=elements=sub_nav^ - ^tmp_admin=elements=tab_nav^ -
-
-
-
-
- - -
-
- - -
-
-
- -  ^loc_generic.CANCEL^ - - ^tmp_admin=elements=save_button^ -
-
-
+ ^tmp_admin=elements=error_success^ + ^tmp_admin=elements=sub_nav^ + ^tmp_admin=elements=tab_nav^ +
+
+
+
+
+
+

{{item.heading}}

+
+
+
+ + +
+
+ + +
+
+
+
+ +  ^loc_generic.CANCEL^ + + ^tmp_admin=elements=save_button^ +
+
+
^tmp_angular=admin=plugins=plugin_settings^ ^tmp_admin=footer^ diff --git a/plugins/sample/details.json b/plugins/sample/details.json index 0ebf34a2b..98ecbe02c 100755 --- a/plugins/sample/details.json +++ b/plugins/sample/details.json @@ -18,15 +18,21 @@ }, "settings": [ { + "displayName": "SAMPLE_NUMBER_SETTING", + "group": "SAMPLE_GROUP1", "name": "pencilblue_sample_number_setting", "value": 123 }, { - "name": "pencilblue_sample_string_setting", + "displayName": "SAMPLE_STRING_SETTING", + "group": "SAMPLE_GROUP1", + "name": "pencilblue_sample_string_setting", "value": "abc" }, { - "name": "pencilblue_sample_boolean_setting", + "displayName": "SAMPLE_BOOL_SETTING", + "group": "SAMPLE_GROUP2", + "name": "pencilblue_sample_boolean_setting", "value": true } ], @@ -42,15 +48,21 @@ "theme": { "settings": [ { - "name": "pencilblue_sample_theme_number_setting", + "displayName": "SAMPLE_BOOL_THEME_SETTING", + "group": "SAMPLE_GROUP1", + "name": "pencilblue_sample_theme_number_setting", "value": 456 }, { - "name": "pencilblue_sample_theme_string_setting", + "displayName": "SAMPLE_BOOL_THEME_SETTING", + "group": "SAMPLE_GROUP2", + "name": "pencilblue_sample_theme_string_setting", "value": "789" }, { - "name": "pencilblue_sample_theme_boolean_setting", + "displayName": "SAMPLE_BOOL_THEME_SETTING", + "group": "SAMPLE_GROUP2", + "name": "pencilblue_sample_theme_boolean_setting", "value": true } ], diff --git a/plugins/sample/public/localization/en-US.json b/plugins/sample/public/localization/en-US.json index e9c2a2225..9e0e7b275 100644 --- a/plugins/sample/public/localization/en-US.json +++ b/plugins/sample/public/localization/en-US.json @@ -1,4 +1,12 @@ { "SAMPLE_HELLO_WORLD": "Hello World!!!", - "PARAMETERIZED_KEY": "This is a parameterized key. I can pass a string value like, {someString}, or a number like {someNumber}. I can even pass JSON like, {json}" -} \ No newline at end of file + "PARAMETERIZED_KEY": "This is a parameterized key. I can pass a string value like, {someString}, or a number like {someNumber}. I can even pass JSON like, {json}", + "SAMPLE_STRING_SETTING": "Sample String Setting", + "SAMPLE_NUMBER_SETTING": "Sample Number Setting", + "SAMPLE_BOOL_SETTING": "Sample Boolean Setting", + "SAMPLE_STRING_THEME_SETTING": "Sample String Theme Setting", + "SAMPLE_NUMBER_THEME_SETTING": "Sample Number Theme Setting", + "SAMPLE_BOOL_THEME_SETTING": "Sample Boolean Theme Setting", + "SAMPLE_GROUP1": "Sample Setting Group 1", + "SAMPLE_GROUP2": "Sample Setting Group 2" +} diff --git a/test/include/service/entities/plugins/plugin_validation_service_tests.js b/test/include/service/entities/plugins/plugin_validation_service_tests.js index 2b945bf5f..787fa32e5 100644 --- a/test/include/service/entities/plugins/plugin_validation_service_tests.js +++ b/test/include/service/entities/plugins/plugin_validation_service_tests.js @@ -1,26 +1,45 @@ //dependencies +var util = require('util'); var should = require('should'); -var Configuration = require('../../../../../include/config.js'); -var Lib = require('../../../../../lib'); +var TestHelpers = require('../../../../test_helpers.js'); describe('PluginValidationService', function() { - var pb = null; - var PluginValidationService = null; - before('Initialize the Environment with the default configuration', function () { - this.timeout(10000); - - pb = new Lib(Configuration.getBaseConfig()); - PluginValidationService = pb.PluginValidationService; - }); + TestHelpers.registerReset(); describe('PluginValidationService.validateSettingValue', function() { [1, 2.0, true, false, '', 'hello'].forEach(function(val) { it('should return true when passed a value type of '+(typeof val), function() { - PluginValidationService.validateSettingValue(val).should.eql(true); + this.pb.PluginValidationService.validateSettingValue(val).should.eql(true); + }); + }); + }); + + describe('PluginValidationService.validateSetting', function() { + + [1, 2.0, true, false, '', 'hello'].forEach(function(val) { + + it('should callback with a validation error when not passed an object as the setting with value: '+(typeof val), function(done) { + var service = new this.pb.PluginValidationService(); + service.validateSetting(val, {}, function(err, validationError) { + validationError.should.be.type('object'); + done(); + }); + }); + }); + + [1, 2.0, true, {}, []].forEach(function(val) { + + it('should callback with a validation error when the group is provided and not a non-empty string: '+util.inspect(val), function(done) { + var service = new this.pb.PluginValidationService(); + service.validateSetting({group: val, name: 'abc', value: '123'}, {index: 0}, function(err, validationErrors) { + validationErrors.length.should.eql(1); + validationErrors[0].field.should.eql('settings[0].group'); + done(); + }); }); }); });