diff --git a/app/assets/javascripts/discourse/components/onebox.js b/app/assets/javascripts/discourse/components/onebox.js index 372deb5f0..63a5ef42c 100644 --- a/app/assets/javascripts/discourse/components/onebox.js +++ b/app/assets/javascripts/discourse/components/onebox.js @@ -1,89 +1,106 @@ /** A helper for looking up oneboxes and displaying them - For now it only stores in a var, in future we can change it so it uses localStorage. + For now it only stores in a local Javascript Object, in future we can change it so it uses localStorage + or some other mechanism. - @class Notification - @extends Discourse.Model + @class Onebox @namespace Discourse @module Discourse **/ -Discourse.Onebox = (function() { +Discourse.Onebox = { - var cache, load, localCache, lookup, lookupCache; - localCache = {}; + // The cache is just a JS Object + localCache: {}, - cache = function(url, contents) { - localCache[url] = contents; - return null; - }; + // A cache of failed URLs + failedCache: {}, - lookupCache = function(url) { - var cached; - cached = localCache[url]; - if (cached && cached.then) { - return null; - } else { - return cached; + /** + Perform a lookup of a onebox based an anchor element. It will insert a loading + indicator and remove it when the loading is complete or fails. + + @method load + @param {HTMLElement} e the anchor element whose onebox we want to look up + @param {Boolean} refresh true if we want to force a refresh of the onebox + **/ + load: function(e, refresh) { + + var $elem = $(e); + + // If the onebox has loaded, return + if ($elem.data('onebox-loaded')) return; + if ($elem.hasClass('loading-onebox')) return; + + var url = e.href; + + // Unless we're forcing a refresh... + if (!refresh) { + // If we have it in our cache, return it. + var cached = this.localCache[url]; + if (cached) return cached; + + // If the request failed, don't do anything + var failed = this.failedCache[url]; + if (failed) return; } - }; - lookup = function(url, refresh, callback) { - var cached; - cached = localCache[url]; - if (refresh && cached && !cached.then) { - cached = null; - } - if (cached) { - if (cached.then) { - cached.then(callback(lookupCache(url))); - } else { - callback(cached); - } - return false; - } else { - cache(url, jQuery.get("/onebox", { - url: url, - refresh: refresh - }, function(html) { - cache(url, html); - return callback(html); - })); - return true; - } - }; + // Add the loading CSS class + $elem.addClass('loading-onebox'); - load = function(e, refresh) { - var $elem, loading, url; - if (!refresh) refresh = false; + // Retrieve the onebox + var promise = $.ajax({ + type: 'GET', + url: "/onebox", + data: { url: url, refresh: refresh } + }); - url = e.href; - $elem = $(e); - if ($elem.data('onebox-loaded')) { - return; - } - loading = lookup(url, refresh, function(html) { + // We can call this when loading is complete + var loadingFinished = function() { $elem.removeClass('loading-onebox'); $elem.data('onebox-loaded'); - if (!html) { - return; - } - if (html.trim().length === 0) { - return; - } - return $elem.replaceWith(html); - }); - if (loading) { - return $elem.addClass('loading-onebox'); } - }; - return { - load: load, - lookup: lookup, - lookupCache: lookupCache - }; + var onebox = this; + promise.then(function(html) { -})(); + // successfully loaded onebox + loadingFinished(); + + onebox.localCache[url] = html; + $elem.replaceWith(html); + + }, function() { + // If the request failed log it as such + onebox.failedCache[url] = true; + loadingFinished(); + }); + + }, + + /** + Return the cached contents of a Onebox + + @method lookupCache + @param {String} url the url of the onebox + @return {String} the cached contents of the onebox or null if not found + **/ + lookupCache: function(url) { + return this.localCache[url]; + }, + + /** + Store the contents of a Onebox in our local cache. + + @method cache + @private + @param {String} url the url of the onebox we crawled + @param {String} contents the contents we want to cache + **/ + cache: function(url, contents) { + this.localCache[url] = contents; + } + +}; diff --git a/app/assets/javascripts/discourse/views/composer_view.js b/app/assets/javascripts/discourse/views/composer_view.js index 9ac40a0d4..3ec36729e 100644 --- a/app/assets/javascripts/discourse/views/composer_view.js +++ b/app/assets/javascripts/discourse/views/composer_view.js @@ -152,19 +152,27 @@ Discourse.ComposerView = Discourse.View.extend({ // Called after the preview renders. Debounced for performance afterRender: Discourse.debounce(function() { - var $wmdPreview, refresh, - _this = this; - $wmdPreview = $('#wmd-preview'); - if ($wmdPreview.length === 0) { - return; - } + var $wmdPreview = $('#wmd-preview'); + if ($wmdPreview.length === 0) return; + Discourse.SyntaxHighlighting.apply($wmdPreview); - refresh = this.get('controller.content.post.id') !== void 0; + + var post = this.get('controller.content.post'); + var refresh = false; + + // If we are editing a post, we'll refresh its contents once. This is a feature that + // allows a user to refresh its contents once. + if (post && post.blank('refreshedPost')) { + refresh = true + post.set('refreshedPost', true); + } + + // Load the post processing effects $('a.onebox', $wmdPreview).each(function(i, e) { - return Discourse.Onebox.load(e, refresh); + Discourse.Onebox.load(e, refresh); }); - return $('span.mention', $wmdPreview).each(function(i, e) { - return Discourse.Mention.load(e, refresh); + $('span.mention', $wmdPreview).each(function(i, e) { + Discourse.Mention.load(e, refresh); }); }, 100), diff --git a/app/controllers/onebox_controller.rb b/app/controllers/onebox_controller.rb index 9eb72cd3c..8478269e9 100644 --- a/app/controllers/onebox_controller.rb +++ b/app/controllers/onebox_controller.rb @@ -4,7 +4,16 @@ class OneboxController < ApplicationController def show Oneboxer.invalidate(params[:url]) if params[:refresh].present? - render text: Oneboxer.preview(params[:url]) + + result = Oneboxer.preview(params[:url]) + result.strip! if result.present? + + # If there is no result, return a 404 + if result.blank? + render nothing: true, status: 404 + else + render text: result + end end end diff --git a/spec/controllers/onebox_controller_spec.rb b/spec/controllers/onebox_controller_spec.rb index 99247add5..89175acfd 100644 --- a/spec/controllers/onebox_controller_spec.rb +++ b/spec/controllers/onebox_controller_spec.rb @@ -2,14 +2,46 @@ require 'spec_helper' describe OneboxController do - it 'asks the oneboxer for the preview' do - Oneboxer.expects(:preview).with('http://google.com') - xhr :get, :show, url: 'http://google.com' - end + let(:url) { "http://google.com" } it 'invalidates the cache if refresh is passed' do - Oneboxer.expects(:invalidate).with('http://google.com') - xhr :get, :show, url: 'http://google.com', refresh: true + Oneboxer.expects(:invalidate).with(url) + xhr :get, :show, url: url, refresh: true + end + + describe "found onebox" do + + let(:body) { "this is the onebox body"} + + before do + Oneboxer.expects(:preview).with(url).returns(body) + xhr :get, :show, url: url + end + + it 'returns success' do + response.should be_success + end + + it 'returns the onebox response in the body' do + response.body.should == body + end + + end + + describe "missing onebox" do + + it "returns 404 if the onebox is nil" do + Oneboxer.expects(:preview).with(url).returns(nil) + xhr :get, :show, url: url + response.response_code.should == 404 + end + + it "returns 404 if the onebox is an empty string" do + Oneboxer.expects(:preview).with(url).returns(" \t ") + xhr :get, :show, url: url + response.response_code.should == 404 + end + end end diff --git a/spec/javascripts/components/onebox_spec.js b/spec/javascripts/components/onebox_spec.js index 6eaa48b90..70a288856 100644 --- a/spec/javascripts/components/onebox_spec.js +++ b/spec/javascripts/components/onebox_spec.js @@ -2,19 +2,22 @@ describe("Discourse.Onebox", function() { + var anchor; + beforeEach(function() { spyOn(jQuery, 'ajax').andCallThrough(); + anchor = $("")[0]; }); it("Stops rapid calls with cache true", function() { - Discourse.Onebox.lookup('http://bla.com', true, function(c) { return c; }); - Discourse.Onebox.lookup('http://bla.com', true, function(c) { return c; }); + Discourse.Onebox.load(anchor, true); + Discourse.Onebox.load(anchor, true); expect(jQuery.ajax.calls.length).toBe(1); }); it("Stops rapid calls with cache false", function() { - Discourse.Onebox.lookup('http://bla.com/a', false, function(c) { return c; }); - Discourse.Onebox.lookup('http://bla.com/a', false, function(c) { return c; }); + Discourse.Onebox.load(anchor, false); + Discourse.Onebox.load(anchor, false); expect(jQuery.ajax.calls.length).toBe(1); });