From e816a284939d66c9ba12dc2f4d4df1f4f291240d Mon Sep 17 00:00:00 2001 From: Max Goodman Date: Tue, 9 Jul 2013 01:09:38 -0700 Subject: [PATCH] Add REST API for descriptions; copy API call; cleanup. This adds a bona fide special copy operation to simplify API use and handle copying sub-documents like descriptions. --- r2/r2/config/routing.py | 2 + r2/r2/controllers/multi.py | 101 +++++++++++++++++++++++++++---- r2/r2/lib/jsontemplates.py | 17 +++++- r2/r2/lib/strings.py | 1 - r2/r2/lib/validator/validator.py | 4 +- r2/r2/models/subreddit.py | 1 + r2/r2/public/static/js/multi.js | 69 ++++++++++++--------- 7 files changed, 149 insertions(+), 46 deletions(-) diff --git a/r2/r2/config/routing.py b/r2/r2/config/routing.py index d30891d01..8ef83c01c 100644 --- a/r2/r2/config/routing.py +++ b/r2/r2/config/routing.py @@ -321,8 +321,10 @@ def make_map(): mc('/api/:action', controller='api') mc("/api/multi/mine", controller="multiapi", action="my_multis") + mc("/api/multi/*multipath/copy", controller="multiapi", action="multi_copy") mc("/api/multi/*multipath/rename", controller="multiapi", action="multi_rename") mc("/api/multi/*multipath/r/:srname", controller="multiapi", action="multi_subreddit") + mc("/api/multi/*multipath/description", controller="multiapi", action="multi_description") mc("/api/multi/*multipath", controller="multiapi", action="multi") mc("/api/v1/:action", controller="oauth2frontend", diff --git a/r2/r2/controllers/multi.py b/r2/r2/controllers/multi.py index cf6975e80..a08ff5f0f 100644 --- a/r2/r2/controllers/multi.py +++ b/r2/r2/controllers/multi.py @@ -21,6 +21,7 @@ ############################################################################### from pylons import c, request, response +from pylons.i18n import _ from r2.config.extensions import set_extension from r2.controllers.api_docs import api_doc, api_section @@ -50,7 +51,10 @@ from r2.lib.validator import ( VMultiByPath, ) from r2.lib.pages.things import wrap_things -from r2.lib.jsontemplates import LabeledMultiJsonTemplate +from r2.lib.jsontemplates import ( + LabeledMultiJsonTemplate, + LabeledMultiDescriptionJsonTemplate, +) from r2.lib.errors import errors, reddit_http_error, RedditError from r2.lib.base import abort @@ -61,6 +65,11 @@ multi_json_spec = { } +multi_description_json_spec = { + 'body_md': VMarkdown('body_md', empty_error=None), +} + + class MultiApiController(RedditController, OAuth2ResourceController): def pre(self): set_extension(request.environ, "json") @@ -193,6 +202,52 @@ class MultiApiController(RedditController, OAuth2ResourceController): """Delete a multi.""" multi.delete() + def _copy_multi(self, from_multi, to_path_info): + self._check_new_multi_path(to_path_info) + + try: + LabeledMulti._byID(to_path_info['path']) + except tdb_cassandra.NotFound: + to_multi = LabeledMulti.copy(to_path_info['path'], from_multi) + else: + raise RedditError('MULTI_EXISTS', code=409, fields='multipath') + + return to_multi + + @require_oauth2_scope("subscribe") + @api_doc( + api_section.multis, + uri="/api/multi/{multipath}/copy", + ) + @validate( + VUser(), + VModhash(), + from_multi=VMultiByPath("multipath", require_view=True), + to_path_info=VMultiPath("to", + docs={"to": "destination multireddit url path"}, + ), + ) + def POST_multi_copy(self, from_multi, to_path_info): + """Copy a multi. + + Responds with 409 Conflict if the target already exists. + + A "copied from ..." line will automatically be appended to the + description. + + """ + to_multi = self._copy_multi(from_multi, to_path_info) + from_path = from_multi.path + to_multi.copied_from = from_path + if to_multi.description_md: + to_multi.description_md += '\n\n' + to_multi.description_md += _('copied from %(source)s') % { + # force markdown linking since /user/foo is not autolinked + 'source': '[%s](%s)' % (from_path, from_path) + } + to_multi._commit() + return self._format_multi(to_multi) + @require_oauth2_scope("subscribe") @api_doc( api_section.multis, @@ -206,19 +261,11 @@ class MultiApiController(RedditController, OAuth2ResourceController): docs={"to": "destination multireddit url path"}, ), ) - def POST_multi_rename(self, multi, to_path_info): + def POST_multi_rename(self, from_multi, to_path_info): """Rename a multi.""" - self._check_new_multi_path(to_path_info) - - try: - LabeledMulti._byID(to_path_info['path']) - except tdb_cassandra.NotFound: - to_multi = LabeledMulti.copy(to_path_info['path'], multi) - else: - raise RedditError('MULTI_EXISTS', code=409, fields='multipath') - - multi.delete() + to_multi = self._copy_multi(from_multi, to_path_info) + from_multi.delete() return self._format_multi(to_multi) def _get_multi_subreddit(self, multi, sr): @@ -281,3 +328,33 @@ class MultiApiController(RedditController, OAuth2ResourceController): """Remove a subreddit from a multi.""" multi.del_srs(sr) multi._commit() + + def _format_multi_description(self, multi): + resp = LabeledMultiDescriptionJsonTemplate().render(multi).finalize() + return self.api_wrapper(resp) + + @require_oauth2_scope("read") + @api_doc( + api_section.multis, + uri="/api/multi/{multipath}/description", + ) + @validate( + VUser(), + multi=VMultiByPath("multipath", require_view=True), + ) + def GET_multi_description(self, multi): + """Get a multi's description.""" + return self._format_multi_description(multi) + + @require_oauth2_scope("read") + @api_doc(api_section.multis, extends=GET_multi_description) + @validate( + VUser(), + multi=VMultiByPath("multipath", require_edit=True), + data=VValidatedJSON('model', multi_description_json_spec), + ) + def PUT_multi_description(self, multi, data): + """Change a multi's markdown description.""" + multi.description_md = data['body_md'] + multi._commit() + return self._format_multi_description(multi) diff --git a/r2/r2/lib/jsontemplates.py b/r2/r2/lib/jsontemplates.py index 508c5ae27..69446d907 100755 --- a/r2/r2/lib/jsontemplates.py +++ b/r2/r2/lib/jsontemplates.py @@ -257,8 +257,6 @@ class LabeledMultiJsonTemplate(ThingJsonTemplate): subreddits="srs", visibility="visibility", can_edit="can_edit", - description_md="description_md", - description_html="description_html", ) del _data_attrs_["id"] @@ -275,7 +273,20 @@ class LabeledMultiJsonTemplate(ThingJsonTemplate): return self.sr_props(thing, thing.srs) elif attr == "can_edit": return c.user_is_loggedin and thing.can_edit(c.user) - elif attr == "description_html": + else: + return ThingJsonTemplate.thing_attr(self, thing, attr) + +class LabeledMultiDescriptionJsonTemplate(ThingJsonTemplate): + _data_attrs_ = dict( + body_md="description_md", + body_html="description_html", + ) + + def kind(self, wrapped): + return "LabeledMultiDescription" + + def thing_attr(self, thing, attr): + if attr == "description_html": # if safemarkdown is passed a falsy string it returns None :/ description_html = safemarkdown(thing.description_md) or '' return description_html diff --git a/r2/r2/lib/strings.py b/r2/r2/lib/strings.py index ee0d824a1..70f52d4c1 100644 --- a/r2/r2/lib/strings.py +++ b/r2/r2/lib/strings.py @@ -217,7 +217,6 @@ Note: there are a couple of places outside of your subreddit where someone can c awesomeness_goes_here = _('awesomeness goes here'), add_multi_sr = _('add a subreddit to your multi.'), open_multi = _('open this multi'), - copied_from = _('copied from %(source)s'), ) class StringHandler(object): diff --git a/r2/r2/lib/validator/validator.py b/r2/r2/lib/validator/validator.py index 488cde76e..e2c39e1d8 100644 --- a/r2/r2/lib/validator/validator.py +++ b/r2/r2/lib/validator/validator.py @@ -539,9 +539,9 @@ class VLength(Validator): def run(self, text, text2 = ''): text = text or text2 if self.empty_error and (not text or self.only_whitespace.match(text)): - self.set_error(self.empty_error) + self.set_error(self.empty_error, code=400) elif len(text) > self.max_length: - self.set_error(self.length_error, {'max_length': self.max_length}) + self.set_error(self.length_error, {'max_length': self.max_length}, code=400) else: return text diff --git a/r2/r2/models/subreddit.py b/r2/r2/models/subreddit.py index 897236396..9652135c9 100644 --- a/r2/r2/models/subreddit.py +++ b/r2/r2/models/subreddit.py @@ -1289,6 +1289,7 @@ class LabeledMulti(tdb_cassandra.Thing, MultiReddit): _defaults = dict(MultiReddit._defaults, visibility='private', description_md='', + copied_from=None, # for internal analysis/bookkeeping purposes ) _extra_schema_creation_args = { "key_validation_class": tdb_cassandra.UTF8_TYPE, diff --git a/r2/r2/public/static/js/multi.js b/r2/r2/public/static/js/multi.js index 048f852ac..7f244b5ff 100644 --- a/r2/r2/public/static/js/multi.js +++ b/r2/r2/public/static/js/multi.js @@ -51,6 +51,16 @@ r.multi.MultiRedditList = Backbone.Collection.extend({ } }) +r.multi.MultiRedditDescription = Backbone.Model.extend({ + parse: function(response) { + return response.data + }, + + isNew: function() { + return false + } +}) + r.multi.MultiReddit = Backbone.Model.extend({ idAttribute: 'path', url: function() { @@ -63,8 +73,13 @@ r.multi.MultiReddit = Backbone.Model.extend({ initialize: function(attributes, options) { this.uncreated = options && !!options.isNew - this.subreddits = new r.multi.MultiRedditList(this.get('subreddits'), {parse: true}) - this.subreddits.url = this.url() + '/r/' + this.subreddits = new r.multi.MultiRedditList(this.get('subreddits'), { + url: this.url() + '/r/', + parse: true + }) + this.description = new r.multi.MultiRedditDescription(null, { + url: this.url() + '/description' + }) this.on('change:subreddits', function(model, value) { this.subreddits.set(value, {parse: true}) }, this) @@ -128,19 +143,20 @@ r.multi.MultiReddit = Backbone.Model.extend({ this.subreddits.getByName(name).destroy(options) }, - rename: function(newPath) { + _copyOp: function(op, newCollection, newName) { var deferred = new $.Deferred Backbone.ajax({ type: 'POST', - url: this.url() + '/rename', + url: this.url() + '/' + op, data: { - to: newPath + to: newCollection.pathByName(newName) }, success: _.bind(function(resp) { - var collection = this.collection - this.trigger('destroy', this, this.collection) + if (op == 'rename') { + this.trigger('destroy', this, this.collection) + } var multi = r.multi.multis.reify(resp) - r.multi.mine.add(multi) + newCollection.add(multi) deferred.resolve(multi) }, this), error: _.bind(deferred.reject, deferred) @@ -148,16 +164,12 @@ r.multi.MultiReddit = Backbone.Model.extend({ return deferred }, - copyTo: function(newMulti) { - var attrs = _.clone(this.attributes) - delete attrs.path - attrs.visibility = 'private' - attrs.description_md = this.get('description_md') + '\n\n' + r.strings('copied_from', { - // ensure that linking happens (currently /user/foo is not autolinked) - source: '[' + this.get('path') + '](' + this.get('path') + ')' - }) - newMulti.set(attrs) - return newMulti + copyTo: function(newCollection, name) { + return this._copyOp('copy', newCollection, name) + }, + + renameTo: function(newCollection, name) { + return this._copyOp('rename', newCollection, name) } }) @@ -252,6 +264,7 @@ r.multi.MultiDetails = Backbone.View.extend({ initialize: function() { this.listenTo(this.model, 'change', this.render) + this.listenTo(this.model.description, 'change', this.render) this.listenTo(this.model.subreddits, 'add', this.addOne) this.listenTo(this.model.subreddits, 'remove', this.removeOne) this.listenTo(this.model.subreddits, 'sort', this.resort) @@ -289,9 +302,11 @@ r.multi.MultiDetails = Backbone.View.extend({ this.$el.toggleClass('readonly', !canEdit) - this.$('.description .usertext-body').html( - _.unescape(this.model.get('description_html')) - ) + if (this.model.description.has('body_html')) { + this.$('.description .usertext-body').html( + _.unescape(this.model.description.get('body_html')) + ) + } this.$('.count').text(this.model.subreddits.length) @@ -408,8 +423,8 @@ r.multi.MultiDetails = Backbone.View.extend({ saveDescription: function(ev) { ev.preventDefault() - this.model.save({ - 'description_md': this.$('.description textarea').val() + this.model.description.save({ + 'body_md': this.$('.description textarea').val() }, { success: _.bind(function() { hide_edit_usertext(this.$el) @@ -551,8 +566,6 @@ r.multi.MultiCreateForm = Backbone.View.extend({ path: r.multi.mine.pathByName(name) }, {isNew: true}) - this._alterMulti(newMulti) - var deferred = new $.Deferred r.multi.mine.create(newMulti, { wait: true, @@ -575,14 +588,14 @@ r.multi.MultiCreateForm = Backbone.View.extend({ }) r.multi.MultiCopyForm = r.multi.MultiCreateForm.extend({ - _alterMulti: function(multi) { - this.options.sourceMulti.copyTo(multi) + _createMulti: function(name) { + return this.options.sourceMulti.copyTo(r.multi.mine, name) } }) r.multi.MultiRenameForm = r.multi.MultiCopyForm.extend({ _createMulti: function(name) { - return this.options.sourceMulti.rename(r.multi.mine.pathByName(name)) + return this.options.sourceMulti.renameTo(r.multi.mine, name) } })