diff --git a/app/assets/javascripts/discourse/adapters/rest.js.es6 b/app/assets/javascripts/discourse/adapters/rest.js.es6 index bd5661235..ec25f2d13 100644 --- a/app/assets/javascripts/discourse/adapters/rest.js.es6 +++ b/app/assets/javascripts/discourse/adapters/rest.js.es6 @@ -6,6 +6,16 @@ export function Result(payload, responseJson) { this.target = null; } +const ajax = Discourse.ajax; + +// We use this to make sure 404s are caught +function rethrow(error) { + if (error.status === 404) { + throw "404: " + error.responseText; + } + throw(error); +} + export default Ember.Object.extend({ pathFor(store, type, findArgs) { let path = "/" + Ember.String.underscore(store.pluralize(type)); @@ -31,17 +41,18 @@ export default Ember.Object.extend({ }, findAll(store, type) { - return Discourse.ajax(this.pathFor(store, type)); + return ajax(this.pathFor(store, type)).catch(rethrow); }, + find(store, type, findArgs) { - return Discourse.ajax(this.pathFor(store, type, findArgs)); + return ajax(this.pathFor(store, type, findArgs)).catch(rethrow); }, update(store, type, id, attrs) { const data = {}; data[Ember.String.underscore(type)] = attrs; - return Discourse.ajax(this.pathFor(store, type, id), { method: 'PUT', data }).then(function(json) { + return ajax(this.pathFor(store, type, id), { method: 'PUT', data }).then(function(json) { return new Result(json[type], json); }); }, @@ -50,13 +61,13 @@ export default Ember.Object.extend({ const data = {}; const typeField = Ember.String.underscore(type); data[typeField] = attrs; - return Discourse.ajax(this.pathFor(store, type), { method: 'POST', data }).then(function (json) { + return ajax(this.pathFor(store, type), { method: 'POST', data }).then(function (json) { return new Result(json[typeField], json); }); }, destroyRecord(store, type, record) { - return Discourse.ajax(this.pathFor(store, type, record.get('id')), { method: 'DELETE' }); + return ajax(this.pathFor(store, type, record.get('id')), { method: 'DELETE' }); } }); diff --git a/app/assets/javascripts/discourse/controllers/queued-posts.js.es6 b/app/assets/javascripts/discourse/controllers/queued-posts.js.es6 index a597c2cb4..90939e3b7 100644 --- a/app/assets/javascripts/discourse/controllers/queued-posts.js.es6 +++ b/app/assets/javascripts/discourse/controllers/queued-posts.js.es6 @@ -1,16 +1,16 @@ +import { popupAjaxError } from 'discourse/lib/ajax-error'; + +function updateState(state) { + return function(post) { + post.update({ state }).then(() => { + this.get('model').removeObject(post); + }).catch(popupAjaxError); + }; +} + export default Ember.Controller.extend({ - actions: { - approve(post) { - post.update({ state: 'approved' }).then(() => { - this.get('model').removeObject(post); - }); - }, - - reject(post) { - post.update({ state: 'rejected' }).then(() => { - this.get('model').removeObject(post); - }); - } + approve: updateState('approved'), + reject: updateState('rejected') } }); diff --git a/app/assets/javascripts/discourse/lib/ajax-error.js.es6 b/app/assets/javascripts/discourse/lib/ajax-error.js.es6 index 0117c94a5..f1a34add6 100644 --- a/app/assets/javascripts/discourse/lib/ajax-error.js.es6 +++ b/app/assets/javascripts/discourse/lib/ajax-error.js.es6 @@ -1,30 +1,38 @@ +function extractError(error) { + if (error instanceof Error) { + Ember.Logger.error(error.stack); + } + + if (typeof error === "string") { + Ember.Logger.error(error); + } + + let parsedError; + if (error.responseText) { + try { + const parsedJSON = $.parseJSON(error.responseText); + if (parsedJSON.errors) { + parsedError = parsedJSON.errors[0]; + } else if (parsedJSON.failed) { + parsedError = parsedJSON.message; + } + } catch(ex) { + // in case the JSON doesn't parse + Ember.Logger.error(ex.stack); + } + } + return parsedError || I18n.t('generic_error'); +} + export function throwAjaxError(undoCallback) { return function(error) { - if (error instanceof Error) { - Ember.Logger.error(error.stack); - } - - if (typeof error === "string") { - Ember.Logger.error(error); - } - // If we provided an `undo` callback if (undoCallback) { undoCallback(error); } - let parsedError; - if (error.responseText) { - try { - const parsedJSON = $.parseJSON(error.responseText); - if (parsedJSON.errors) { - parsedError = parsedJSON.errors[0]; - } else if (parsedJSON.failed) { - parsedError = parsedJSON.message; - } - } catch(ex) { - // in case the JSON doesn't parse - Ember.Logger.error(ex.stack); - } - } - throw parsedError || I18n.t('generic_error'); + throw extractError(error); }; } + +export function popupAjaxError(err) { + bootbox.alert(extractError(err)); +} diff --git a/app/assets/javascripts/discourse/models/rest.js.es6 b/app/assets/javascripts/discourse/models/rest.js.es6 index a35da6edf..8bb5d0250 100644 --- a/app/assets/javascripts/discourse/models/rest.js.es6 +++ b/app/assets/javascripts/discourse/models/rest.js.es6 @@ -3,24 +3,30 @@ import Presence from 'discourse/mixins/presence'; const RestModel = Ember.Object.extend(Presence, { isNew: Ember.computed.equal('__state', 'new'), isCreated: Ember.computed.equal('__state', 'created'), + isSaving: false, afterUpdate: Ember.K, update(props) { + if (this.get('isSaving')) { return Ember.RSVP.reject(); } + props = props || this.updateProperties(); const type = this.get('__type'), store = this.get('store'); const self = this; + self.set('isSaving', true); return store.update(type, this.get('id'), props).then(function(res) { self.setProperties(self.__munge(res.payload || res.responseJson)); self.afterUpdate(res); return res; - }); + }).finally(() => this.set('isSaving', false)); }, _saveNew(props) { + if (this.get('isSaving')) { return Ember.RSVP.reject(); } + props = props || this.createProperties(); const type = this.get('__type'), @@ -28,6 +34,7 @@ const RestModel = Ember.Object.extend(Presence, { adapter = store.adapterFor(type); const self = this; + self.set('isSaving', true); return adapter.createRecord(store, type, props).then(function(res) { if (!res) { throw "Received no data back from createRecord"; } @@ -40,7 +47,7 @@ const RestModel = Ember.Object.extend(Presence, { res.target = self; return res; - }); + }).finally(() => this.set('isSaving', false)); }, createProperties() { diff --git a/app/assets/javascripts/discourse/models/store.js.es6 b/app/assets/javascripts/discourse/models/store.js.es6 index e54eaee4a..61ce9f581 100644 --- a/app/assets/javascripts/discourse/models/store.js.es6 +++ b/app/assets/javascripts/discourse/models/store.js.es6 @@ -36,7 +36,7 @@ export default Ember.Object.extend({ if (typeof findArgs === "object") { return self._resultSet(type, result); } else { - return self._hydrate(type, result[Ember.String.underscore(type)]); + return self._hydrate(type, result[Ember.String.underscore(type)], result); } }); }, @@ -48,7 +48,7 @@ export default Ember.Object.extend({ const typeName = Ember.String.underscore(self.pluralize(type)), totalRows = result["total_rows_" + typeName] || result.get('totalRows'), loadMoreUrl = result["load_more_" + typeName], - content = result[typeName].map(obj => self._hydrate(type, obj)); + content = result[typeName].map(obj => self._hydrate(type, obj, result)); resultSet.setProperties({ totalRows, loadMoreUrl }); resultSet.get('content').pushObjects(content); @@ -86,7 +86,7 @@ export default Ember.Object.extend({ _resultSet(type, result) { const typeName = Ember.String.underscore(this.pluralize(type)), - content = result[typeName].map(obj => this._hydrate(type, obj)), + content = result[typeName].map(obj => this._hydrate(type, obj, result)), totalRows = result["total_rows_" + typeName] || content.length, loadMoreUrl = result["load_more_" + typeName]; @@ -111,10 +111,39 @@ export default Ember.Object.extend({ return this.container.lookup('adapter:' + type) || this.container.lookup('adapter:rest'); }, - _hydrate(type, obj) { + _hydrateEmbedded(obj, root) { + const self = this; + Object.keys(obj).forEach(function(k) { + const m = /(.+)\_id$/.exec(k); + if (m) { + const subType = m[1]; + const collection = root[self.pluralize(subType)]; + if (collection) { + const found = collection.findProperty('id', obj[k]); + if (found) { + const hydrated = self._hydrate(subType, found, root); + if (hydrated) { + obj[subType] = hydrated; + delete obj[k]; + } + } + } + } + }); + }, + + _hydrate(type, obj, root) { if (!obj) { throw "Can't hydrate " + type + " of `null`"; } if (!obj.id) { throw "Can't hydrate " + type + " without an `id`"; } + root = root || obj; + + // Experimental: If serialized with a certain option we'll wire up embedded objects + // automatically. + if (root.__rest_serializer === "1") { + this._hydrateEmbedded(obj, root); + } + _identityMap[type] = _identityMap[type] || {}; const existing = _identityMap[type][obj.id]; diff --git a/app/assets/javascripts/discourse/routes/queued-posts.js.es6 b/app/assets/javascripts/discourse/routes/queued-posts.js.es6 index 58b28dd8f..700b9871a 100644 --- a/app/assets/javascripts/discourse/routes/queued-posts.js.es6 +++ b/app/assets/javascripts/discourse/routes/queued-posts.js.es6 @@ -5,4 +5,3 @@ export default DiscourseRoute.extend({ return this.store.find('queuedPost', {status: 'new'}); } }); - diff --git a/app/assets/javascripts/discourse/routes/topic-from-params.js.es6 b/app/assets/javascripts/discourse/routes/topic-from-params.js.es6 index 9c414daac..62dd9e5db 100644 --- a/app/assets/javascripts/discourse/routes/topic-from-params.js.es6 +++ b/app/assets/javascripts/discourse/routes/topic-from-params.js.es6 @@ -1,6 +1,9 @@ // This route is used for retrieving a topic based on params export default Discourse.Route.extend({ + // Avoid default model hook + model: function() { return; }, + setupController: function(controller, params) { params = params || {}; params.track_visit = true; diff --git a/app/assets/javascripts/discourse/templates/queued-posts.hbs b/app/assets/javascripts/discourse/templates/queued-posts.hbs index 2119de912..e2d096514 100644 --- a/app/assets/javascripts/discourse/templates/queued-posts.hbs +++ b/app/assets/javascripts/discourse/templates/queued-posts.hbs @@ -1,28 +1,44 @@
{{i18n "queue.none"}}
{{/each}} diff --git a/app/assets/stylesheets/desktop/queued-posts.scss b/app/assets/stylesheets/desktop/queued-posts.scss index 50955c997..49cdb433d 100644 --- a/app/assets/stylesheets/desktop/queued-posts.scss +++ b/app/assets/stylesheets/desktop/queued-posts.scss @@ -10,8 +10,10 @@ width: $topic-body-width; float: left; } - h4.title { - margin-bottom: 1em; + + .post-title { + color: darken(scale-color-diff(), 50%); + font-weight: bold; } border-bottom: 1px solid darken(scale-color-diff(), 10%); diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 7ee38765d..146fe5771 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -219,6 +219,7 @@ class ApplicationController < ActionController::Base def render_json_dump(obj, opts=nil) opts ||= {} + obj['__rest_serializer'] = "1" if opts[:rest_serializer] render json: MultiJson.dump(obj), status: opts[:status] || 200 end diff --git a/app/controllers/queued_posts_controller.rb b/app/controllers/queued_posts_controller.rb index e7dd20d79..8ee84538e 100644 --- a/app/controllers/queued_posts_controller.rb +++ b/app/controllers/queued_posts_controller.rb @@ -8,12 +8,20 @@ class QueuedPostsController < ApplicationController state = QueuedPost.states[(params[:state] || 'new').to_sym] state ||= QueuedPost.states[:new] - @queued_posts = QueuedPost.where(state: state) - render_serialized(@queued_posts, QueuedPostSerializer, root: :queued_posts) + @queued_posts = QueuedPost.where(state: state).includes(:topic, :user) + render_serialized(@queued_posts, QueuedPostSerializer, root: :queued_posts, rest_serializer: true) end def update qp = QueuedPost.where(id: params[:id]).first + + state = params[:queued_post][:state] + if state == 'approved' + qp.approve!(current_user) + elsif state == 'rejected' + qp.reject!(current_user) + end + render_serialized(qp, QueuedPostSerializer, root: :queued_posts) end diff --git a/app/serializers/queued_post_serializer.rb b/app/serializers/queued_post_serializer.rb index ed9b9a700..8e2637bce 100644 --- a/app/serializers/queued_post_serializer.rb +++ b/app/serializers/queued_post_serializer.rb @@ -1,4 +1,5 @@ class QueuedPostSerializer < ApplicationSerializer + attributes :id, :queue, :user_id, @@ -11,4 +12,6 @@ class QueuedPostSerializer < ApplicationSerializer :created_at has_one :user, serializer: BasicUserSerializer, embed: :object + has_one :topic, serializer: BasicTopicSerializer + end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index d27df4732..f6a1d4e87 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -226,6 +226,7 @@ en: placeholder: "type the topic title here" queue: + topic: "Topic:" approve: 'Approve Post' reject: 'Reject Post' title: "Needs Approval" diff --git a/spec/controllers/queued_posts_controller_spec.rb b/spec/controllers/queued_posts_controller_spec.rb index 99cbd3834..1e4210797 100644 --- a/spec/controllers/queued_posts_controller_spec.rb +++ b/spec/controllers/queued_posts_controller_spec.rb @@ -1,4 +1,6 @@ require 'spec_helper' +require_dependency 'queued_posts_controller' +require_dependency 'queued_post' describe QueuedPostsController do context 'without authentication' do @@ -24,5 +26,34 @@ describe QueuedPostsController do expect(response).to be_success end end + + + context 'update' do + let!(:user) { log_in(:moderator) } + let(:qp) { Fabricate(:queued_post) } + + context 'approved' do + it 'updates the post to approved' do + + xhr :put, :update, id: qp.id, queued_post: { state: 'approved' } + expect(response).to be_success + + qp.reload + expect(qp.state).to eq(QueuedPost.states[:approved]) + end + end + + context 'rejected' do + it 'updates the post to approved' do + + xhr :put, :update, id: qp.id, queued_post: { state: 'rejected' } + expect(response).to be_success + + qp.reload + expect(qp.state).to eq(QueuedPost.states[:rejected]) + end + end + + end end diff --git a/spec/fabricators/queued_post_fabricator.rb b/spec/fabricators/queued_post_fabricator.rb new file mode 100644 index 000000000..45cae4006 --- /dev/null +++ b/spec/fabricators/queued_post_fabricator.rb @@ -0,0 +1,18 @@ +Fabricator(:queued_post) do + queue 'new_post' + state QueuedPost.states[:new] + user + topic + raw 'This post should be queued up' + post_options do + { reply_to_post_number: 1, + via_email: true, + raw_email: 'store_me', + auto_track: true, + custom_fields: { hello: 'world' }, + cooking_options: { cat: 'hat' }, + cook_method: Post.cook_methods[:raw_html], + image_sizes: {"http://foo.bar/image.png" => {"width" => 0, "height" => 222}} } + end +end + diff --git a/spec/models/queued_post_spec.rb b/spec/models/queued_post_spec.rb index 521819fef..976c3faf1 100644 --- a/spec/models/queued_post_spec.rb +++ b/spec/models/queued_post_spec.rb @@ -7,22 +7,7 @@ describe QueuedPost do let(:topic) { Fabricate(:topic) } let(:user) { Fabricate(:user) } let(:admin) { Fabricate(:admin) } - let(:qp) { QueuedPost.create(queue: 'new_post', - state: QueuedPost.states[:new], - user_id: user.id, - topic_id: topic.id, - raw: 'This post should be queued up', - post_options: { - reply_to_post_number: 1, - via_email: true, - raw_email: 'store_me', - auto_track: true, - custom_fields: { hello: 'world' }, - cooking_options: { cat: 'hat' }, - cook_method: Post.cook_methods[:raw_html], - not_create_option: true, - image_sizes: {"http://foo.bar/image.png" => {"width" => 0, "height" => 222}} - }) } + let(:qp) { Fabricate(:queued_post, topic: topic, user: user) } it "returns the appropriate options for posting" do create_options = qp.create_options diff --git a/test/javascripts/acceptance/topic-test.js.es6 b/test/javascripts/acceptance/topic-test.js.es6 index 53bf0c339..6d4aa67fc 100644 --- a/test/javascripts/acceptance/topic-test.js.es6 +++ b/test/javascripts/acceptance/topic-test.js.es6 @@ -2,7 +2,7 @@ import { acceptance } from "helpers/qunit-helpers"; acceptance("View Topic"); test("Enter a Topic", () => { - visit("/t/internationalization-localization/280"); + visit("/t/internationalization-localization/280/1"); andThen(() => { ok(exists("#topic"), "The topic was rendered"); ok(exists("#topic .post-cloak"), "The topic has cloaked posts"); diff --git a/test/javascripts/helpers/create-pretender.js.es6 b/test/javascripts/helpers/create-pretender.js.es6 index c71f168c4..1a4de17aa 100644 --- a/test/javascripts/helpers/create-pretender.js.es6 +++ b/test/javascripts/helpers/create-pretender.js.es6 @@ -173,6 +173,14 @@ export default function() { }); }); + this.get('/fruits/:id', function() { + return response({ + __rest_serializer: "1", + fruit: {id: 1, name: 'apple', farmer_id: 1}, + farmers: [{id: 1, name: 'Evil Trout'}] + }); + }); + this.get('/widgets/:widget_id', function(request) { const w = _widgets.findBy('id', parseInt(request.params.widget_id)); if (w) { diff --git a/test/javascripts/models/rest-model-test.js.es6 b/test/javascripts/models/rest-model-test.js.es6 index 9aa63c4c3..182532b13 100644 --- a/test/javascripts/models/rest-model-test.js.es6 +++ b/test/javascripts/models/rest-model-test.js.es6 @@ -19,23 +19,51 @@ test('munging', function() { test('update', function() { const store = createStore(); - store.find('widget', 123).then(function(widget) { equal(widget.get('name'), 'Trout Lure'); - widget.update({ name: 'new name' }).then(function() { + + ok(!widget.get('isSaving')); + const promise = widget.update({ name: 'new name' }); + ok(widget.get('isSaving')); + promise.then(function() { + ok(!widget.get('isSaving')); equal(widget.get('name'), 'new name'); }); }); }); +test('updating simultaneously', function() { + expect(2); + + const store = createStore(); + store.find('widget', 123).then(function(widget) { + + const firstPromise = widget.update({ name: 'new name' }); + const secondPromise = widget.update({ name: 'new name' }); + firstPromise.then(function() { + ok(true, 'the first promise succeeeds'); + }); + + secondPromise.catch(function() { + ok(true, 'the second promise fails'); + }); + + }); +}); + test('save new', function() { const store = createStore(); const widget = store.createRecord('widget'); ok(widget.get('isNew'), 'it is a new record'); ok(!widget.get('isCreated'), 'it is not created'); + ok(!widget.get('isSaving')); - widget.save({ name: 'Evil Widget' }).then(function() { + const promise = widget.save({ name: 'Evil Widget' }); + ok(widget.get('isSaving')); + + promise.then(function() { + ok(!widget.get('isSaving')); ok(widget.get('id'), 'it has an id'); ok(widget.get('name'), 'Evil Widget'); ok(widget.get('isCreated'), 'it is created'); @@ -43,6 +71,23 @@ test('save new', function() { }); }); +test('creating simultaneously', function() { + expect(2); + + const store = createStore(); + const widget = store.createRecord('widget'); + + const firstPromise = widget.save({ name: 'Evil Widget' }); + const secondPromise = widget.save({ name: 'Evil Widget' }); + firstPromise.then(function() { + ok(true, 'the first promise succeeeds'); + }); + + secondPromise.catch(function() { + ok(true, 'the second promise fails'); + }); +}); + test('destroyRecord', function() { const store = createStore(); store.find('widget', 123).then(function(widget) { diff --git a/test/javascripts/models/store-test.js.es6 b/test/javascripts/models/store-test.js.es6 index 9b3893cf4..13e2e8d00 100644 --- a/test/javascripts/models/store-test.js.es6 +++ b/test/javascripts/models/store-test.js.es6 @@ -88,3 +88,11 @@ test('destroyRecord', function() { }); }); }); + +test('find embedded', function() { + const store = createStore(); + store.find('fruit', 1).then(function(f) { + ok(f.get('farmer'), 'it has the embedded object'); + }); +}); +