[Search] Fix maxResults vs maxNumber

Typo fix in generic search. #91.

platform/search/bundle.json

@@ -13,7 +13,7 @@
             {
                 "key": "SearchController",
                 "implementation": "controllers/SearchController.js",
-                "depends": [ "$scope", "searchService" ]
+                "depends": [ "$scope", "searchService", "throttle" ]
             },
             {
                 "key": "SearchMenuController",

platform/search/src/controllers/SearchController.js

@@ -30,7 +30,7 @@ define(function () {
     var INITIAL_LOAD_NUMBER = 20,
         LOAD_INCREMENT = 20;
     
-    function SearchController($scope, searchService) {
+    function SearchController($scope, searchService, throttle) {
         // numResults is the amount of results to display. Will get increased. 
         // fullResults holds the most recent complete searchService response object
         var numResults = INITIAL_LOAD_NUMBER,
@@ -88,15 +88,6 @@ define(function () {
         function search(maxResults) {
             var inputText = $scope.ngModel.input;
             
-            if (inputText !== '' && inputText !== undefined) {
-                // We are starting to load.
-                $scope.loading = true;
-                
-                // Update whether the file tree should be displayed 
-                // Hide tree only when starting search 
-                $scope.ngModel.search = true;
-            }
-            
             if (!maxResults) {
                 // Reset 'load more'
                 numResults = INITIAL_LOAD_NUMBER;
@@ -129,7 +120,21 @@ define(function () {
              *   that this function should return. If not provided, search
              *   service default will be used. 
              */
-            search: search,
+            search: function (maxResults) {
+                // Show that we are loading before starting the throttled function 
+                // so that the user knows the input was detected. 
+                if ($scope.ngModel.input !== '' && $scope.ngModel.input !== undefined) {
+                    // We are starting to load.
+                    $scope.loading = true;
+
+                    // Update whether the file tree should be displayed 
+                    // Hide tree only when starting search 
+                    $scope.ngModel.search = true;
+                }
+                
+                // Call a throttled search
+                return throttle(search)(maxResults);
+            },
             
             /**
              * Checks to see if there are more search results to display. If the answer is

platform/search/src/services/GenericSearchProvider.js

@@ -79,27 +79,28 @@ define(
             // Handles responses from the web worker. Namely, the results of 
             // a search request. 
             function handleResponse(event) {
-                var ids = [],
-                    id;
+                var ids = [];
                 
                 // If we have the results from a search 
                 if (event.data.request === 'search') {
-                    // Convert the ids given from the web worker into domain objects
-                    for (id in event.data.results) {
-                        ids.push(id);
-                    }
+                    
+                    // Get the ids into an array
+                    event.data.results.forEach(function (result) {
+                        ids.push(result.id);
+                    });
+                    
+                    // Get domainObjects with the ids
                     objectService.getObjects(ids).then(function (objects) {
-                        var searchResults = [],
-                            id;
+                        var searchResults = [];
                         
-                        // Create searchResult objects
-                        for (id in objects) {
+                        // Create searchResult objects with the gotten domainObjects
+                        event.data.results.forEach(function (result) {
                             searchResults.push({
-                                object: objects[id],
-                                id: id,
-                                score: event.data.results[id]
+                                object: objects[result.id],
+                                id: result.id,
+                                score: result.score
                             });
-                        }
+                        });
                         
                         // Resove the promise corresponding to this 
                         pendingQueries[event.data.timestamp].resolve({

platform/search/src/services/GenericSearchWorker.js

@@ -93,14 +93,14 @@
      *           * timestamp: The time identifier from when the query was made
      */
     function search(data) {
-        // This results dictionary will have domain object ID keys which 
-        // point to the value the domain object's score. 
-        var results = {},
+        // This results array will have domain object IDs and scores together
+        // [{score: number, id: string}]
+        var results = [],
             input = data.input.toLocaleLowerCase(),
             terms = convertToTerms(input),
             message = {
                 request: 'search',
-                results: {},
+                results: [],
                 total: 0,
                 timestamp: data.timestamp,
                 timedOut: false
@@ -111,6 +111,7 @@
         
         // If the user input is empty, we want to have no search results.
         if (input !== '') {
+            // Start getting search results
             for (i = 0; i < indexedItems.length; i += 1) {
                 // If this is taking too long, then stop
                 if (Date.now() > data.timestamp + data.timeout) {
@@ -121,23 +122,26 @@
                 // Score and add items
                 score = scoreItem(indexedItems[i], input, terms);
                 if (score > 0) {
-                    results[indexedItems[i].id] = score;
+                    results.push({id: indexedItems[i].id, score: score});
                     message.total += 1;
                 }
             }
         }
         
-        // Truncate results if there are more than maxResults
-        if (message.total > data.maxResults) {
-            i = 0;
-            for (id in results) {
-                message.results[id] = results[id];
-                i += 1;
-                if (i >= data.maxResults) {
-                    break;
-                }
+        // Sort the results by score 
+        results.sort(function (a, b) {
+            if (a.score > b.score) {
+                return -1;
+            } else if (b.score > a.score) {
+                return 1;
+            } else {
+                return 0;
             }
-            // TODO: This seems inefficient.
+        });
+        
+        // Truncate results if there are more than maxNumber
+        if (message.total > data.maxNumber) {
+            message.results = results.slice(0, data.maxNumber);
         } else {
             message.results = results;
         }

platform/search/test/controllers/SearchControllerSpec.js

@@ -36,6 +36,7 @@ define(
         describe("The search controller", function () {
             var mockScope,
                 mockSearchService,
+                mockThrottle,
                 mockPromise,
                 mockSearchResult,
                 mockDomainObject,
@@ -73,6 +74,11 @@ define(
                 );
                 mockSearchService.query.andReturn(mockPromise);
                 
+                mockThrottle = jasmine.createSpy('throttle');
+                mockThrottle.andCallFake(function (fn) {
+                    return fn;
+                });
+                
                 mockTypes = [{key: 'mock.type', name: 'Mock Type', glyph: '?'}];
                 
                 mockSearchResult = jasmine.createSpyObj(
@@ -86,7 +92,7 @@ define(
                 mockSearchResult.object = mockDomainObject;
                 mockDomainObject.getModel.andReturn({name: 'Mock Object', type: 'mock.type'});
                 
-                controller = new SearchController(mockScope, mockSearchService, mockTypes);
+                controller = new SearchController(mockScope, mockSearchService, mockThrottle);
                 controller.search();
             });
             

platform/search/test/services/GenericSearchProviderSpec.js

@@ -172,10 +172,10 @@ define(
                     event = {
                         data: {
                             request: "search",
-                            results: {
-                                1: 1,
-                                2: 2
-                            },
+                            results: [
+                                {id: 1, score: 1},
+                                {id: 2, score: 2}
+                            ],
                             total: 2,
                             timedOut: false,
                             timestamp: timestamp

platform/search/test/services/GenericSearchWorkerSpec.js

@@ -35,7 +35,17 @@ define(
                 numObjects = 5;
             
             beforeEach(function () {
-                var i;
+                // See first it()
+            });
+            
+            it("searches can reach all objects", function () {
+                var flag = false,
+                    workerOutput,
+                    resultsLength = 0,
+                    i;
+                
+                // This for loop is being done only once in the first it() becuase 
+                //  duplicate checking is done outside fo the worker
                 for (i = 0; i < numObjects; i += 1) {
                     worker.postMessage(
                         {
@@ -49,12 +59,6 @@ define(
                         }
                     );
                 }
-            });
-            
-            it("searches can reach all objects", function () {
-                var flag = false,
-                    workerOutput,
-                    resultsLength = 0;
                 
                 // Search something that should return all objects
                 runs(function () {
@@ -70,12 +74,8 @@ define(
                 });
                 
                 worker.onmessage = function (event) {
-                    var id;
-                    
                     workerOutput = event.data;
-                    for (id in workerOutput.results) {
-                        resultsLength += 1;
-                    }
+                    resultsLength = event.data.results.length;
                     flag = true;
                 };
                 
@@ -108,12 +108,8 @@ define(
                 });
                 
                 worker.onmessage = function (event) {
-                    var id;
-                    
                     workerOutput = event.data;
-                    for (id in workerOutput.results) {
-                        resultsLength += 1;
-                    }
+                    resultsLength = event.data.results.length;
                     flag = true;
                 };
                 
@@ -124,7 +120,6 @@ define(
                 runs(function () {
                     expect(workerOutput).toBeDefined();
                     expect(resultsLength).toEqual(1);
-                    expect(workerOutput.results[2]).toBeDefined();
                 });
             });
         });