From d1d3e5dd8c80fe0b4cf7285946e523fde8b90c5a Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Tue, 23 Sep 2014 16:16:19 -0400 Subject: [PATCH] Remove broken `debouncePromise` and clean up some deprecations --- .../discourse/controllers/search.js.es6 | 21 +++-- .../javascripts/discourse/lib/debounce.js | 28 ------ .../discourse/lib/search-for-term.js.es6 | 66 +++++++++++++ .../javascripts/discourse/lib/search.js | 88 ----------------- .../discourse/views/choose-topic.js.es6 | 10 +- .../lib/debounce-promise-test.js.es6 | 94 ------------------- 6 files changed, 81 insertions(+), 226 deletions(-) create mode 100644 app/assets/javascripts/discourse/lib/search-for-term.js.es6 delete mode 100644 app/assets/javascripts/discourse/lib/search.js delete mode 100644 test/javascripts/lib/debounce-promise-test.js.es6 diff --git a/app/assets/javascripts/discourse/controllers/search.js.es6 b/app/assets/javascripts/discourse/controllers/search.js.es6 index 8306574ba7d..24c327a012f 100644 --- a/app/assets/javascripts/discourse/controllers/search.js.es6 +++ b/app/assets/javascripts/discourse/controllers/search.js.es6 @@ -1,10 +1,14 @@ +import searchForTerm from 'discourse/lib/search-for-term'; + +var _dontSearch = false; + export default Em.Controller.extend(Discourse.Presence, { contextChanged: function(){ - if(this.get('searchContextEnabled')){ - this._dontSearch = true; + if (this.get('searchContextEnabled')) { + _dontSearch = true; this.set('searchContextEnabled', false); - this._dontSearch = false; + _dontSearch = false; } }.observes("searchContext"), @@ -23,7 +27,7 @@ export default Em.Controller.extend(Discourse.Presence, { }.property('searchContext'), searchContextEnabledChanged: function(){ - if(this._dontSearch){ return; } + if (_dontSearch) { return; } this.newSearchNeeded(); }.observes('searchContextEnabled'), @@ -33,14 +37,15 @@ export default Em.Controller.extend(Discourse.Presence, { var term = (this.get('term') || '').trim(); if (term.length >= Discourse.SiteSettings.min_search_term_length) { this.set('loading', true); - this.searchTerm(term, this.get('typeFilter')); + + Ember.run.debounce(this, 'searchTerm', term, this.get('typeFilter'), 400); } else { this.setProperties({ content: null }); } this.set('selectedIndex', 0); }.observes('term', 'typeFilter'), - searchTerm: Discourse.debouncePromise(function(term, typeFilter) { + searchTerm: function(term, typeFilter) { var self = this; var context; @@ -48,7 +53,7 @@ export default Em.Controller.extend(Discourse.Presence, { context = this.get('searchContext'); } - return Discourse.Search.forTerm(term, { + searchForTerm(term, { typeFilter: typeFilter, searchContext: context }).then(function(results) { @@ -57,7 +62,7 @@ export default Em.Controller.extend(Discourse.Presence, { }).catch(function() { self.set('loading', false); }); - }, 400), + }, showCancelFilter: function() { if (this.get('loading')) return false; diff --git a/app/assets/javascripts/discourse/lib/debounce.js b/app/assets/javascripts/discourse/lib/debounce.js index f56dc688b47..571e146ae2f 100644 --- a/app/assets/javascripts/discourse/lib/debounce.js +++ b/app/assets/javascripts/discourse/lib/debounce.js @@ -21,31 +21,3 @@ Discourse.debounce = function(func, wait) { Ember.run.debounce(null, later, wait); }; }; - -/** - Debounce a javascript function that returns a promise. If it's called too soon it - will return a promise that is never resolved. - - @method debouncePromise - @module Discourse - @param {function} func The function to debounce - @param {Number} wait how long to wait -**/ -Discourse.debouncePromise = function(func, wait) { - var self, args, promise; - var later = function() { - func.apply(self, args).then(function (funcResult) { - promise.resolve(funcResult); - }); - }; - - return function() { - self = this; - args = arguments; - promise = Ember.Deferred.create(); - - Ember.run.debounce(null, later, wait); - - return promise; - }; -}; diff --git a/app/assets/javascripts/discourse/lib/search-for-term.js.es6 b/app/assets/javascripts/discourse/lib/search-for-term.js.es6 new file mode 100644 index 00000000000..0a78ee37e39 --- /dev/null +++ b/app/assets/javascripts/discourse/lib/search-for-term.js.es6 @@ -0,0 +1,66 @@ +export default function searchForTerm(term, opts) { + if (!opts) opts = {}; + + // Only include the data we have + var data = { term: term, include_blurbs: 'true' }; + if (opts.typeFilter) data.type_filter = opts.typeFilter; + if (opts.searchForId) data.search_for_id = true; + + if (opts.searchContext) { + data.search_context = { + type: opts.searchContext.type, + id: opts.searchContext.id + }; + } + + return Discourse.ajax('/search/', { data: data }).then(function(results){ + // Topics might not be included + if (!results.topics) { results.topics = []; } + if (!results.users) { results.users = []; } + if (!results.posts) { results.posts = []; } + if (!results.categories) { results.categories = []; } + + var topicMap = {}; + results.topics = results.topics.map(function(topic){ + topic = Discourse.Topic.create(topic); + topicMap[topic.id] = topic; + return topic; + }); + + results.posts = results.posts.map(function(post){ + post = Discourse.Post.create(post); + post.set('topic', topicMap[post.topic_id]); + return post; + }); + + results.users = results.users.map(function(user){ + user = Discourse.User.create(user); + return user; + }); + + results.categories = results.categories.map(function(category){ + category = Discourse.Category.list().findProperty('id', category.id); + return category; + }); + + var r = results.grouped_search_result; + results.resultTypes = []; + + // TODO: consider refactoring front end to take a better structure + [['topic','posts'],['user','users'],['category','categories']].forEach(function(pair){ + var type = pair[0], name = pair[1]; + if(results[name].length > 0) { + results.resultTypes.push({ + results: results[name], + displayType: (opts.searchContext && opts.searchContext.type === 'topic' && type === 'topic') ? 'post' : type, + type: type, + more: r['more_' + name] + }); + } + }); + + var noResults = !!((results.topics.length === 0) && (results.posts.length === 0) && (results.categories.length === 0)); + + return noResults ? null : Em.Object.create(results); + }); +} diff --git a/app/assets/javascripts/discourse/lib/search.js b/app/assets/javascripts/discourse/lib/search.js deleted file mode 100644 index 565e0e26912..00000000000 --- a/app/assets/javascripts/discourse/lib/search.js +++ /dev/null @@ -1,88 +0,0 @@ -/** - This component helps with Searching - - @class Search - @namespace Discourse - @module Discourse -**/ -Discourse.Search = { - - /** - Search for a term, with an optional filter. - - @method forTerm - @param {String} term The term to search for - @param {Object} opts Options for searching - @param {String} opts.typeFilter Filter our results to one type only - @param {Ember.Object} opts.searchContext data to help searching within a context (say, a category or user) - @return {Promise} a promise that resolves the search results - **/ - forTerm: function(term, opts) { - if (!opts) opts = {}; - - // Only include the data we have - var data = { term: term, include_blurbs: 'true' }; - if (opts.typeFilter) data.type_filter = opts.typeFilter; - if (opts.searchForId) data.search_for_id = true; - - if (opts.searchContext) { - data.search_context = { - type: opts.searchContext.type, - id: opts.searchContext.id - }; - } - - var promise = Discourse.ajax('/search', { data: data }); - - promise.then(function(results){ - // Topics might not be included - if (!results.topics) { results.topics = []; } - - var topicMap = {}; - results.topics = results.topics.map(function(topic){ - topic = Discourse.Topic.create(topic); - topicMap[topic.id] = topic; - return topic; - }); - - results.posts = results.posts.map(function(post){ - post = Discourse.Post.create(post); - post.set('topic', topicMap[post.topic_id]); - return post; - }); - - results.users = results.users.map(function(user){ - user = Discourse.User.create(user); - return user; - }); - - results.categories = results.categories.map(function(category){ - category = Discourse.Category.list().findProperty('id', category.id); - return category; - }); - - var r = results.grouped_search_result; - results.resultTypes = []; - - // TODO: consider refactoring front end to take a better structure - [['topic','posts'],['user','users'],['category','categories']].forEach(function(pair){ - var type = pair[0], name = pair[1]; - if(results[name].length > 0) { - results.resultTypes.push({ - results: results[name], - displayType: (opts.searchContext && opts.searchContext.type === 'topic' && type === 'topic') ? 'post' : type, - type: type, - more: r['more_' + name] - }); - } - }); - - var noResults = !!((results.topics.length === 0) && (results.posts.length === 0) && (results.categories.length === 0)); - - return noResults ? null : Em.Object.create(results); - }); - - return promise; - } - -}; diff --git a/app/assets/javascripts/discourse/views/choose-topic.js.es6 b/app/assets/javascripts/discourse/views/choose-topic.js.es6 index fafcbd2e5ca..b064528efa7 100644 --- a/app/assets/javascripts/discourse/views/choose-topic.js.es6 +++ b/app/assets/javascripts/discourse/views/choose-topic.js.es6 @@ -1,11 +1,5 @@ -/** - This view presents the user with a widget to choose a topic. +import searchForTerm from 'discourse/lib/search-for-term'; - @class ChooseTopicView - @extends Discourse.View - @namespace Discourse - @module Discourse -**/ export default Discourse.View.extend({ templateName: 'choose_topic', @@ -30,7 +24,7 @@ export default Discourse.View.extend({ self.setProperties({ topics: null, loading: false }); return; } - Discourse.Search.forTerm(title, {typeFilter: 'topic', searchForId: true}).then(function (results) { + searchForTerm(title, {typeFilter: 'topic', searchForId: true}).then(function (results) { if (results && results.posts && results.posts.length > 0) { self.set('topics', results.posts.mapBy('topic')); } else { diff --git a/test/javascripts/lib/debounce-promise-test.js.es6 b/test/javascripts/lib/debounce-promise-test.js.es6 deleted file mode 100644 index 2dac257bd80..00000000000 --- a/test/javascripts/lib/debounce-promise-test.js.es6 +++ /dev/null @@ -1,94 +0,0 @@ -var clock, original, debounced, originalPromiseResolvesWith, callback; - -var nothingFired = function(additionalMessage) { - ok(!original.called, "original function is not called " + additionalMessage); - ok(!callback.called, "debounced promise is not resolved " + additionalMessage); -}; - -var originalAndCallbackFiredOnce = function(additionalMessage) { - ok(original.calledOnce, "original function is called once " + additionalMessage); - ok(callback.calledOnce, "debounced promise is resolved once " + additionalMessage); -}; - -module("Discourse.debouncePromise", { - setup: function() { - clock = sinon.useFakeTimers(); - - originalPromiseResolvesWith = null; - original = sinon.spy(function() { - var promise = Ember.Deferred.create(); - promise.resolve(originalPromiseResolvesWith); - return promise; - }); - - debounced = Discourse.debouncePromise(original, 100); - callback = sinon.spy(); - }, - - teardown: function() { - clock.restore(); - } -}); - -test("delays execution till the end of the timeout", function() { - debounced().then(callback); - nothingFired("immediately after calling debounced function"); - - clock.tick(99); - nothingFired("just before the end of the timeout"); - - clock.tick(1); - originalAndCallbackFiredOnce("exactly at the end of the timeout"); -}); - -test("executes only once, no matter how many times debounced function is called during the timeout", function() { - debounced().then(callback); - debounced().then(callback); - - clock.tick(100); - originalAndCallbackFiredOnce("(second call was supressed)"); -}); - -test("prolongs the timeout when the debounced function is called for the second time during the timeout", function() { - debounced().then(callback); - - clock.tick(50); - debounced().then(callback); - - clock.tick(50); - nothingFired("at the end of the original timeout"); - - clock.tick(50); - originalAndCallbackFiredOnce("exactly at the end of the prolonged timeout"); -}); - -test("preserves last call's context and params when executing delayed function", function() { - var firstObj = {}; - var secondObj = {}; - - debounced.call(firstObj, "first"); - debounced.call(secondObj, "second"); - - clock.tick(100); - ok(original.calledOn(secondObj), "the context of the second of two subsequent calls is preserved"); - ok(original.calledWithExactly("second"), "param passed during the second of two subsequent calls is preserved"); -}); - -test("can be called again after timeout passes", function() { - debounced().then(callback); - - clock.tick(100); - debounced().then(callback); - - clock.tick(100); - ok(original.calledTwice, "original function is called for the second time"); - ok(callback.calledTwice, "debounced promise is resolved for the second time"); -}); - -test("passes resolved value from the original promise as a param to the debounced promise's callback", function() { - originalPromiseResolvesWith = "original promise return value"; - debounced().then(callback); - - clock.tick(100); - ok(callback.calledWith("original promise return value")); -});