Merge pull request #6374 from bsclifton/fix-separator-part2

Updated sanitization logic for separators

app/common/lib/menuUtil.js

@@ -161,7 +161,7 @@ const isItemValid = (currentItem, previousItem) => {
  * - entries which don't have a label or type
  */
 module.exports.sanitizeTemplateItems = (template) => {
-  const result = template.reduce((previousValue, currentValue, currentIndex, array) => {
+  const reduced = template.reduce((previousValue, currentValue, currentIndex, array) => {
     const result = currentIndex === 1 ? [] : previousValue
     if (currentIndex === 1) {
       if (isItemValid(previousValue)) {
@@ -177,7 +177,17 @@ module.exports.sanitizeTemplateItems = (template) => {
     return result
   })
 
-  return Array.isArray(result)
-    ? result
-    : [result]
+  const result = Array.isArray(reduced)
+    ? reduced
+    : [reduced]
+
+  if (result.length > 0 && result[0] === CommonMenu.separatorMenuItem) {
+    result.shift()
+  }
+
+  if (result.length > 0 && result[result.length - 1] === CommonMenu.separatorMenuItem) {
+    result.pop()
+  }
+
+  return result
 }

test/unit/app/common/lib/menuUtilTest.js

@@ -244,9 +244,9 @@ describe('menuUtil tests', function () {
       assert.deepEqual(result, expectedResult)
     })
     it('removes duplicate menu separators', function () {
-      const template = [separator, separator, {label: 'lol'}]
+      const template = [{label: 'lol1'}, separator, separator, {label: 'lol2'}]
       const result = menuUtil.sanitizeTemplateItems(template)
-      const expectedResult = [separator, {label: 'lol'}]
+      const expectedResult = [{label: 'lol1'}, separator, {label: 'lol2'}]
       assert.deepEqual(result, expectedResult)
     })
     it('removes items which are missing label or type', function () {
@@ -267,5 +267,23 @@ describe('menuUtil tests', function () {
       const expectedResult = [{label: 'lol'}]
       assert.deepEqual(result, expectedResult)
     })
+    it('does not allow the list to start with a separator', function () {
+      const template = [separator, {label: 'lol'}]
+      const result = menuUtil.sanitizeTemplateItems(template)
+      const expectedResult = [{label: 'lol'}]
+      assert.deepEqual(result, expectedResult)
+    })
+    it('does not allow the list to end with a separator', function () {
+      const template = [{label: 'lol'}, separator]
+      const result = menuUtil.sanitizeTemplateItems(template)
+      const expectedResult = [{label: 'lol'}]
+      assert.deepEqual(result, expectedResult)
+    })
+    it('does not allow only a separator', function () {
+      const template = [separator]
+      const result = menuUtil.sanitizeTemplateItems(template)
+      const expectedResult = []
+      assert.deepEqual(result, expectedResult)
+    })
   })
 })