Merge pull request #382 from smartdevicelink/feature/0180-choice-uniqueness

Feature/0180 choice uniqueness

Author: crokita

lib/js/src/manager/screen/choiceset/ChoiceCell.js

@@ -29,6 +29,7 @@
 * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 * POSSIBILITY OF SUCH DAMAGE.
 */
+import { SdlArtwork } from '../../file/filetypes/SdlArtwork.js';
 
 class ChoiceCell {
     /**
@@ -40,6 +41,7 @@ class ChoiceCell {
         this._text = null;
         this._secondaryText = null;
         this._tertiaryText = null;
+        this._uniqueText = null;
         this._voiceCommands = null;
         this._artwork = null;
         this._secondaryArtwork = null;
@@ -51,6 +53,7 @@ class ChoiceCell {
         }
 
         this._text = text;
+        this._uniqueText = text;
     }
 
     /**
@@ -112,6 +115,29 @@ class ChoiceCell {
         return this;
     }
 
+    /**
+     * Get the state unique text. USED INTERNALLY
+     * @private
+     * @returns {String} - The uniqueText to be used in place of primaryText when core does not support identical names for ChoiceSets
+     */
+    _getUniqueText () {
+        return this._uniqueText;
+    }
+
+    /**
+     * Primary text of the cell to be displayed on the module. Used to distinguish cells with the
+     * same `text` but other fields are different. This is autogenerated by the screen manager.
+     * Attempting to use cells that are exactly the same (all text and artwork fields are the same)
+     * will not cause this to be used. This will not be used when connected to modules supporting RPC 7.1+.
+     * @private
+     * @param {String} uniqueText - The uniqueText to be used in place of primaryText when core does not support identical names for ChoiceSets
+     * @returns {ChoiceCell} - A reference to this instance to support method chaining
+     */
+    _setUniqueText (uniqueText) {
+        this._uniqueText = uniqueText;
+        return this;
+    }
+
     /**
      * Get the state voiceCommands
      * @returns {String[]|null} - The list of voice command strings
@@ -251,6 +277,27 @@ class ChoiceCell {
         }
         return true;
     }
+
+    /**
+     * Creates a deep copy of the object
+     * @returns {ChoiceCell} - A deep copy of the object
+     */
+    clone () {
+        const clonedParams = Object.assign({}, this); // shallow copy. copy all objects afterwards
+
+        if (clonedParams._artwork !== null) {
+            clonedParams._artwork = Object.assign(new SdlArtwork(), clonedParams._artwork);
+        }
+        if (clonedParams._secondaryArtwork !== null) {
+            clonedParams._secondaryArtwork = Object.assign(new SdlArtwork(), clonedParams._secondaryArtwork);
+        }
+
+        if (Array.isArray(this.getVoiceCommands())) {
+            clonedParams._voiceCommands = this.getVoiceCommands().map(vc => vc);
+        }
+
+        return Object.assign(new ChoiceCell(this.getText()), clonedParams);
+    }
 }
 
 // MAX ID for cells - Reasoning is from Java library: cannot use Integer.MAX_INT as the value is too high.

lib/js/src/manager/screen/choiceset/ChoiceSet.js

@@ -36,6 +36,12 @@ import { VrHelpItem } from '../../../rpc/structs/VrHelpItem.js';
 class ChoiceSet {
     /**
      * Create a new instance of ChoiceSet
+     * Initialize with a title, choices, and listener. It will use the default timeout and layout, all other properties (such as prompts) will be null.
+     * WARNING: If you display multiple cells with the same title with the only uniquing property between cells being different `vrCommands` or a feature 
+     * that is not displayed on the head unit (e.g. if the head unit doesn't display `secondaryArtwork` and that's the only uniquing property between two cells) 
+     * then the cells may appear to be the same to the user in `Manual` mode. This only applies to RPC connections >= 7.1.0.
+     * WARNING: On < 7.1.0 connections, the title cell will be automatically modified among cells that have the same title when they are preloaded, so they will 
+     * always appear differently on-screen when they are displayed. Unique text will be created by appending " (2)", " (3)", etc.
      * @class
      * @param {String} title - The choice set's title
      * @param {ChoiceCell[]} choices - The choices to be displayed to the user for interaction

lib/js/src/manager/screen/choiceset/_ChoiceSetManagerBase.js

@@ -130,12 +130,14 @@ class _ChoiceSetManagerBase extends _SubManagerBase {
      * @param {ChoiceCell[]} choices - A list of ChoiceCell objects that will be part of a choice set later
      * @returns {Promise} - A promise that resolves to a Boolean of whether the operation is a success
      */
-    async preloadChoices (choices) {
+    async preloadChoices (choices = null) {
         if (this._getState() === _SubManagerBase.ERROR) {
             console.warn('ChoiceSetManager: Choice Manager In Error State');
             return false;
         }
-        const choicesToUpload = choices.map(choice => choice); // shallow copy
+
+        const choicesToUpload = this._getChoicesToBeUploadedWithArray(choices);
+
         this._removeChoicesFromChoices(this._preloadedChoices, choicesToUpload);
         this._removeChoicesFromChoices(this._pendingPreloadChoices, choicesToUpload);
 
@@ -167,6 +169,25 @@ class _ChoiceSetManagerBase extends _SubManagerBase {
         });
     }
 
+    /**
+     * Checks if 2 or more cells have the same text/title. In case this condition is true, this function will handle the presented issue by adding "(count)".
+     * E.g. Choices param contains 2 cells with text/title "Address" will be handled by updating the uniqueText/uniqueTitle of the second cell to "Address (2)".
+     * @param {ChoiceCell[]} choices - A list of ChoiceCell objects to be uploaded
+     */
+    _addUniqueNamesToCells (choices) {
+        const dictCounter = {}; // create a string to number hash for counting similar primary texts
+
+        choices.forEach(choice => {
+            const choiceName = choice.getText();
+            if (dictCounter[choiceName] === undefined) {
+                dictCounter[choiceName] = 1; // unique text
+            } else { // found a duplicate
+                dictCounter[choiceName] += 1;
+                choice._setUniqueText(`${choiceName} (${dictCounter[choiceName]})`);
+            }
+        });
+    }
+
     /**
      * Deletes choices that were sent previously
      * @param {ChoiceCell[]} choices - A list of ChoiceCell objects
@@ -276,15 +297,16 @@ class _ChoiceSetManagerBase extends _SubManagerBase {
             }
         }
 
-        const uniqueChoiceTexts = {};
+        const uniqueChoiceCells = [];
         const uniqueVoiceCommands = {};
         let choiceCellWithVoiceCommandCount = 0;
         let allVoiceCommandsCount = 0;
 
         for (let index = 0; index < choices.length; index++) {
-            const choiceText = choices[index].getText();
             const choiceVoiceCommands = choices[index].getVoiceCommands();
-            uniqueChoiceTexts[choiceText] = true;
+            if (uniqueChoiceCells.findIndex(choice => choice.equals(choices[index])) === -1) {
+                uniqueChoiceCells.push(choices[index]);
+            }
 
             if (choiceVoiceCommands !== null) {
                 choiceCellWithVoiceCommandCount++;
@@ -297,9 +319,8 @@ class _ChoiceSetManagerBase extends _SubManagerBase {
             }
         }
 
-        // Cell text MUST be unique
-        if (Object.keys(uniqueChoiceTexts).length < choices.length) {
-            console.error('ChoiceSetManager: Attempted to create a choice set with duplicate cell text. Cell text must be unique. The choice set will not be set.');
+        if (uniqueChoiceCells.length !== choices.length) {
+            console.error('Attempted to create a choice set with a duplicate cell. Cell must have a unique value other than its primary text. The choice set will not be set.');
             return false;
         }
 
@@ -579,6 +600,22 @@ class _ChoiceSetManagerBase extends _SubManagerBase {
             .setKeypressMode(KeypressMode.RESEND_CURRENT_ENTRY);
     }
 
+    /**
+     * Modifies the choices names depending on SDL version
+     * @param {ChoiceCell[]} choices - The first list of choices
+     * @returns {ChoiceCell[]} - A deep copy of the name modified choices
+     */
+    _getChoicesToBeUploadedWithArray (choices) {
+        // If we're running on a connection < RPC 7.1, we need to de-duplicate cells because presenting them will fail if we have the same cell primary text.
+        if (choices !== null && this._lifecycleManager.getSdlMsgVersion() !== null
+                && (this._lifecycleManager.getSdlMsgVersion().getMajorVersion() < 7
+                || (this._lifecycleManager.getSdlMsgVersion().getMajorVersion() === 7 && this._lifecycleManager.getSdlMsgVersion().getMinorVersion() === 0))) {
+            // version if 7.0.0 or lower
+            this._addUniqueNamesToCells(choices);
+        }
+        return choices.map(choice => choice.clone()); // deep copy
+    }
+
     /**
      * Listen for DISPLAYS capability updates
      * @private

lib/js/src/manager/screen/choiceset/_PreloadChoicesOperation.js

@@ -166,7 +166,7 @@ class _PreloadChoicesOperation extends _Task {
             vrCommands = this._isVrOptional ? null : [`${cell._getChoiceId()}`]; // stringified choice id
         }
 
-        const menuName = this._shouldSendChoiceText() ? cell.getText() : null;
+        const menuName = this._shouldSendChoiceText() ? cell._getUniqueText() : null;
         if (menuName === null) {
             console.error('PreloadChoicesOperation: Could not convert Choice Cell to CreateInteractionChoiceSet. It will not be shown');
             return null;
@@ -247,4 +247,4 @@ class _PreloadChoicesOperation extends _Task {
     }
 }
 
-export { _PreloadChoicesOperation };
+export { _PreloadChoicesOperation };

tests/managers/screen/choiceset/ChoiceCellTests.js

@@ -17,6 +17,7 @@ module.exports = function (appClient) {
             Validator.assertNull(choiceCell.getTertiaryText());
             Validator.assertNull(choiceCell.getArtwork());
             Validator.assertNull(choiceCell.getSecondaryArtwork());
+            Validator.assertNull(choiceCell._getUniqueText());
         });
 
         it('testSettersAndGetters', function () {
@@ -37,6 +38,11 @@ module.exports = function (appClient) {
             Validator.assertEquals(choiceCell.getSecondaryArtwork(), artwork);
             Validator.assertEquals(choiceCell._getChoiceId(), MAX_ID);
 
+            Validator.assertEquals(choiceCell._getUniqueText(), choiceCell.getText());
+
+            choiceCell._setUniqueText('hi');
+            Validator.assertEquals(choiceCell._getUniqueText(), 'hi');
+
             choiceCell.setText('hello');
             Validator.assertEquals(choiceCell.getText(), 'hello');
         });
@@ -62,8 +68,13 @@ module.exports = function (appClient) {
                 .setSecondaryText(Test.GENERAL_STRING)
                 .setTertiaryText(Test.GENERAL_STRING);
 
+            // UniqueText should not be taken into consideration when checking equality
+            choiceCell._setUniqueText('1');
+            choiceCell2._setUniqueText('2');
+            choiceCell3._setUniqueText('3');
+
             Validator.assertTrue(choiceCell.equals(choiceCell2));
             Validator.assertTrue(!choiceCell.equals(choiceCell3));
         });
     });
-};
+};

tests/managers/screen/choiceset/ChoiceSetManagerTests.js

@@ -49,6 +49,12 @@ module.exports = function (appClient) {
         });
 
         it('testSetupChoiceSet', function () {
+            const stub = sinon.stub(sdlManager._lifecycleManager, 'getSdlMsgVersion')
+                .returns(new SDL.rpc.structs.SdlMsgVersion()
+                    .setMajorVersion(7)
+                    .setMinorVersion(0)
+                    .setPatchVersion(0));
+
             const choiceSetSelectionListener = new SDL.manager.screen.choiceset.ChoiceSetSelectionListener()
                 .setOnChoiceSelected(() => {})
                 .setOnError(() => {});
@@ -57,34 +63,45 @@ module.exports = function (appClient) {
             const choiceSet1 = new SDL.manager.screen.choiceset.ChoiceSet('test', [], choiceSetSelectionListener);
             Validator.assertTrue(!csm._setUpChoiceSet(choiceSet1));
 
-            // cells cant have duplicate text
+            // Identical cells will not be allowed
             const cell1 = new SDL.manager.screen.choiceset.ChoiceCell('test');
             const cell2 = new SDL.manager.screen.choiceset.ChoiceCell('test');
             const choiceSet2 = new SDL.manager.screen.choiceset.ChoiceSet('test', [cell1, cell2], choiceSetSelectionListener);
             Validator.assertTrue(!csm._setUpChoiceSet(choiceSet2));
 
-            // cells cannot mix and match VR / non-VR
+            // cells that have duplicate text will be allowed if there is another property to make them unique
+            // because a unique name will be assigned and used
             const cell3 = new SDL.manager.screen.choiceset.ChoiceCell('test')
-                .setVoiceCommands(['Test']);
-            const cell4 = new SDL.manager.screen.choiceset.ChoiceCell('test2');
+                .setSecondaryText('text 1');
+            const cell4 = new SDL.manager.screen.choiceset.ChoiceCell('test')
+                .setSecondaryText('text 2');
             const choiceSet3 = new SDL.manager.screen.choiceset.ChoiceSet('test', [cell3, cell4], choiceSetSelectionListener);
-            Validator.assertTrue(!csm._setUpChoiceSet(choiceSet3));
+            Validator.assertTrue(csm._setUpChoiceSet(choiceSet3));
 
-            // VR Commands must be unique
+            // cells cannot mix and match VR / non-VR
             const cell5 = new SDL.manager.screen.choiceset.ChoiceCell('test')
                 .setVoiceCommands(['Test']);
-            const cell6 = new SDL.manager.screen.choiceset.ChoiceCell('test2')
-                .setVoiceCommands(['Test']);
+            const cell6 = new SDL.manager.screen.choiceset.ChoiceCell('test2');
             const choiceSet4 = new SDL.manager.screen.choiceset.ChoiceSet('test', [cell5, cell6], choiceSetSelectionListener);
             Validator.assertTrue(!csm._setUpChoiceSet(choiceSet4));
 
-            // Passing Case
+            // VR Commands must be unique
             const cell7 = new SDL.manager.screen.choiceset.ChoiceCell('test')
                 .setVoiceCommands(['Test']);
             const cell8 = new SDL.manager.screen.choiceset.ChoiceCell('test2')
-                .setVoiceCommands(['Test2']);
+                .setVoiceCommands(['Test']);
             const choiceSet5 = new SDL.manager.screen.choiceset.ChoiceSet('test', [cell7, cell8], choiceSetSelectionListener);
-            Validator.assertTrue(csm._setUpChoiceSet(choiceSet5));
+            Validator.assertTrue(!csm._setUpChoiceSet(choiceSet5));
+
+            // Passing Case
+            const cell9 = new SDL.manager.screen.choiceset.ChoiceCell('test')
+                .setVoiceCommands(['Test']);
+            const cell10 = new SDL.manager.screen.choiceset.ChoiceCell('test2')
+                .setVoiceCommands(['Test2']);
+            const choiceSet6 = new SDL.manager.screen.choiceset.ChoiceSet('test', [cell9, cell10], choiceSetSelectionListener);
+            Validator.assertTrue(csm._setUpChoiceSet(choiceSet6));
+
+            stub.restore();
         });
 
         it('testUpdateIdsOnChoices', function () {
@@ -273,5 +290,29 @@ module.exports = function (appClient) {
             // restore state
             csm._defaultMainWindowCapability = originCapability;
         });
+
+        it('testAddUniqueNamesToCells', function () {
+            const cell1 = new SDL.manager.screen.choiceset.ChoiceCell('McDonalds')
+                .setSecondaryText('1 mile away');
+            const cell2 = new SDL.manager.screen.choiceset.ChoiceCell('McDonalds')
+                .setSecondaryText('2 miles away');
+            const cell3 = new SDL.manager.screen.choiceset.ChoiceCell('Starbucks')
+                .setSecondaryText('3 miles away');
+            const cell4 = new SDL.manager.screen.choiceset.ChoiceCell('McDonalds')
+                .setSecondaryText('4 miles away');
+            const cell5 = new SDL.manager.screen.choiceset.ChoiceCell('Starbucks')
+                .setSecondaryText('5 miles away');
+            const cell6 = new SDL.manager.screen.choiceset.ChoiceCell('Meijer')
+                .setSecondaryText('6 miles away');
+
+            csm._addUniqueNamesToCells([cell1, cell2, cell3, cell4, cell5, cell6]);
+
+            Validator.assertEquals(cell1._getUniqueText(), 'McDonalds');
+            Validator.assertEquals(cell2._getUniqueText(), 'McDonalds (2)');
+            Validator.assertEquals(cell3._getUniqueText(), 'Starbucks');
+            Validator.assertEquals(cell4._getUniqueText(), 'McDonalds (3)');
+            Validator.assertEquals(cell5._getUniqueText(), 'Starbucks (2)');
+            Validator.assertEquals(cell6._getUniqueText(), 'Meijer');
+        });
     });
-};
+};