From cf01c98d814631193a8b68c9bdb50950fac1dc59 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Wed, 8 May 2013 13:33:58 -0400 Subject: [PATCH] Experimental: Interface to Move Posts to an Existing Topic --- .../discourse/components/search.js | 25 +++++ .../discourse/controllers/topic_controller.js | 9 +- .../discourse/helpers/application_helpers.js | 20 ++++ .../discourse/mixins/selected_posts_count.js | 18 +++ .../javascripts/discourse/models/topic.js | 6 +- .../templates/choose_topic.js.handlebars | 19 ++++ .../modal/move_selected.js.handlebars | 18 +-- ...move_selected_existing_topic.js.handlebars | 15 +++ .../move_selected_new_topic.js.handlebars | 19 ++++ .../templates/selected_posts.js.handlebars | 4 +- .../discourse/views/choose_topic_view.js | 52 +++++++++ .../move_selected_existing_topic_view.js | 47 ++++++++ .../modal/move_selected_new_topic_view.js | 47 ++++++++ .../views/modal/move_selected_view.js | 52 ++++----- .../javascripts/discourse/views/post_view.js | 4 +- .../discourse/views/search/search_view.js | 31 +++--- .../stylesheets/application/modal.css.scss | 14 +++ .../application/topic-post.css.scss | 8 ++ app/controllers/topics_controller.rb | 14 ++- app/models/topic.rb | 41 +++++-- config/locales/client.en.yml | 24 +++- lib/search.rb | 1 - spec/controllers/topics_controller_spec.rb | 71 ++++++++---- spec/models/topic_spec.rb | 103 +++++++++++++----- 24 files changed, 511 insertions(+), 151 deletions(-) create mode 100644 app/assets/javascripts/discourse/components/search.js create mode 100644 app/assets/javascripts/discourse/mixins/selected_posts_count.js create mode 100644 app/assets/javascripts/discourse/templates/choose_topic.js.handlebars create mode 100644 app/assets/javascripts/discourse/templates/modal/move_selected_existing_topic.js.handlebars create mode 100644 app/assets/javascripts/discourse/templates/modal/move_selected_new_topic.js.handlebars create mode 100644 app/assets/javascripts/discourse/views/choose_topic_view.js create mode 100644 app/assets/javascripts/discourse/views/modal/move_selected_existing_topic_view.js create mode 100644 app/assets/javascripts/discourse/views/modal/move_selected_new_topic_view.js diff --git a/app/assets/javascripts/discourse/components/search.js b/app/assets/javascripts/discourse/components/search.js new file mode 100644 index 00000000000..ae733877119 --- /dev/null +++ b/app/assets/javascripts/discourse/components/search.js @@ -0,0 +1,25 @@ +/** + 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 {String} typeFilter An optional filter to restrict the search by type + @return {Promise} a promise that resolves the search results + **/ + forTerm: function(term, typeFilter) { + return Discourse.ajax('/search', { + data: { term: term, type_filter: typeFilter } + }); + } + +} + diff --git a/app/assets/javascripts/discourse/controllers/topic_controller.js b/app/assets/javascripts/discourse/controllers/topic_controller.js index 685399be9f5..fdd05efc105 100644 --- a/app/assets/javascripts/discourse/controllers/topic_controller.js +++ b/app/assets/javascripts/discourse/controllers/topic_controller.js @@ -6,7 +6,7 @@ @namespace Discourse @module Discourse **/ -Discourse.TopicController = Discourse.ObjectController.extend({ +Discourse.TopicController = Discourse.ObjectController.extend(Discourse.SelectedPostsCount, { userFilters: new Em.Set(), multiSelect: false, bestOf: false, @@ -22,11 +22,6 @@ Discourse.TopicController = Discourse.ObjectController.extend({ return posts.filterProperty('selected'); }.property('content.posts.@each.selected'), - selectedCount: function() { - if (!this.get('selectedPosts')) return 0; - return this.get('selectedPosts').length; - }.property('selectedPosts'), - canMoveSelected: function() { if (!this.get('content.can_move_posts')) return false; // For now, we can move it if we can delete it since the posts need to be deleted. @@ -91,7 +86,7 @@ Discourse.TopicController = Discourse.ObjectController.extend({ deleteSelected: function() { var topicController = this; - return bootbox.confirm(Em.String.i18n("post.delete.confirm", { count: this.get('selectedCount')}), function(result) { + return bootbox.confirm(Em.String.i18n("post.delete.confirm", { count: this.get('selectedPostsCount')}), function(result) { if (result) { var selectedPosts = topicController.get('selectedPosts'); Discourse.Post.deleteMany(selectedPosts); diff --git a/app/assets/javascripts/discourse/helpers/application_helpers.js b/app/assets/javascripts/discourse/helpers/application_helpers.js index 26a37c02de3..2054034e65b 100644 --- a/app/assets/javascripts/discourse/helpers/application_helpers.js +++ b/app/assets/javascripts/discourse/helpers/application_helpers.js @@ -56,6 +56,26 @@ Handlebars.registerHelper('categoryLink', function(property, options) { return new Handlebars.SafeString(Discourse.Utilities.categoryLink(category)); }); +/** + Inserts a Discourse.TextField to allow the user to enter information. + + @method textField + @for Handlebars +**/ +Ember.Handlebars.registerHelper('textField', function(options) { + var hash = options.hash, + types = options.hashTypes; + + for (var prop in hash) { + if (types[prop] === 'ID') { + hash[prop + 'Binding'] = hash[prop]; + delete hash[prop]; + } + } + + return Ember.Handlebars.helpers.view.call(this, Discourse.TextField, options); +}); + /** Produces a bound link to a category diff --git a/app/assets/javascripts/discourse/mixins/selected_posts_count.js b/app/assets/javascripts/discourse/mixins/selected_posts_count.js new file mode 100644 index 00000000000..a42c487564d --- /dev/null +++ b/app/assets/javascripts/discourse/mixins/selected_posts_count.js @@ -0,0 +1,18 @@ +/** + This mixin allows a modal to list a selected posts count nicely. + + @class Discourse.SelectedPostsCount + @extends Ember.Mixin + @namespace Discourse + @module Discourse +**/ +Discourse.SelectedPostsCount = Em.Mixin.create({ + + selectedPostsCount: function() { + if (!this.get('selectedPosts')) return 0; + return this.get('selectedPosts').length; + }.property('selectedPosts') + +}); + + diff --git a/app/assets/javascripts/discourse/models/topic.js b/app/assets/javascripts/discourse/models/topic.js index 460398dc4e8..9a567a9f56f 100644 --- a/app/assets/javascripts/discourse/models/topic.js +++ b/app/assets/javascripts/discourse/models/topic.js @@ -449,12 +449,10 @@ Discourse.Topic.reopenClass({ }); }, - // Create a topic from posts - movePosts: function(topicId, title, postIds) { - + movePosts: function(topicId, opts) { var promise = Discourse.ajax("/t/" + topicId + "/move-posts", { type: 'POST', - data: { title: title, post_ids: postIds } + data: opts }).then(function (result) { if (result.success) return result; promise.reject(); diff --git a/app/assets/javascripts/discourse/templates/choose_topic.js.handlebars b/app/assets/javascripts/discourse/templates/choose_topic.js.handlebars new file mode 100644 index 00000000000..bdbcb493ea4 --- /dev/null +++ b/app/assets/javascripts/discourse/templates/choose_topic.js.handlebars @@ -0,0 +1,19 @@ + + +{{textField value=view.topicTitle placeholderKey="choose_topic.title.placeholder" elementId="choose-topic-title"}} + +{{#if view.loading}} +

{{i18n loading}}

+{{else}} + {{#if view.noResults}} +

{{i18n choose_topic.none_found}}

+ {{else}} + {{#each view.topics}} +
+
+ {{/each}} + + {{/if}} +{{/if}} diff --git a/app/assets/javascripts/discourse/templates/modal/move_selected.js.handlebars b/app/assets/javascripts/discourse/templates/modal/move_selected.js.handlebars index ecba34fe951..4946725cc1a 100644 --- a/app/assets/javascripts/discourse/templates/modal/move_selected.js.handlebars +++ b/app/assets/javascripts/discourse/templates/modal/move_selected.js.handlebars @@ -1,20 +1,8 @@
- {{#if view.error}} -
- - {{i18n topic.invite_reply.error}} -
- {{/if}} - {{{i18n topic.move_selected.instructions count="view.selectedCount"}}} +

{{{i18n topic.move_selected.instructions count="view.selectedPostsCount"}}}

-
- - {{view Discourse.TextField valueBinding="view.topicName" placeholderKey="composer.title_placeholder"}} -
+ +
- -
- -
\ No newline at end of file diff --git a/app/assets/javascripts/discourse/templates/modal/move_selected_existing_topic.js.handlebars b/app/assets/javascripts/discourse/templates/modal/move_selected_existing_topic.js.handlebars new file mode 100644 index 00000000000..8fbc1febe52 --- /dev/null +++ b/app/assets/javascripts/discourse/templates/modal/move_selected_existing_topic.js.handlebars @@ -0,0 +1,15 @@ +
+ {{#if view.error}} +
+ +
+ {{/if}} + +

{{{i18n topic.move_selected.existing_topic.instructions count="view.selectedPostsCount"}}}

+ + {{view Discourse.ChooseTopicView selectedTopicIdBinding="view.selectedTopicId"}} +
+ +
+ +
\ No newline at end of file diff --git a/app/assets/javascripts/discourse/templates/modal/move_selected_new_topic.js.handlebars b/app/assets/javascripts/discourse/templates/modal/move_selected_new_topic.js.handlebars new file mode 100644 index 00000000000..430fa55287c --- /dev/null +++ b/app/assets/javascripts/discourse/templates/modal/move_selected_new_topic.js.handlebars @@ -0,0 +1,19 @@ +
+ {{#if view.error}} +
+ +
+ {{/if}} + + {{{i18n topic.move_selected.new_topic.instructions count="view.selectedPostsCount"}}} + +
+ + {{view Discourse.TextField valueBinding="view.topicName" placeholderKey="composer.title_placeholder"}} +
+ +
+ +
+ +
\ No newline at end of file diff --git a/app/assets/javascripts/discourse/templates/selected_posts.js.handlebars b/app/assets/javascripts/discourse/templates/selected_posts.js.handlebars index 013a4b1a50d..a306dc1a2ee 100644 --- a/app/assets/javascripts/discourse/templates/selected_posts.js.handlebars +++ b/app/assets/javascripts/discourse/templates/selected_posts.js.handlebars @@ -1,4 +1,4 @@ -

{{countI18n topic.multi_select.description countBinding="controller.selectedCount"}}

+

{{countI18n topic.multi_select.description countBinding="controller.selectedPostsCount"}}

{{#if canDeleteSelected}} @@ -6,6 +6,6 @@ {{#if canMoveSelected}} -{{/if}} +{{/if}}

{{i18n topic.multi_select.cancel}}

diff --git a/app/assets/javascripts/discourse/views/choose_topic_view.js b/app/assets/javascripts/discourse/views/choose_topic_view.js new file mode 100644 index 00000000000..31d55821cf6 --- /dev/null +++ b/app/assets/javascripts/discourse/views/choose_topic_view.js @@ -0,0 +1,52 @@ +/** + This view presents the user with a widget to choose a topic. + + @class ChooseTopicView + @extends Discourse.View + @namespace Discourse + @module Discourse +**/ +Discourse.ChooseTopicView = Discourse.View.extend({ + templateName: 'choose_topic', + + topicTitleChanged: function() { + this.set('loading', true); + this.set('noResults', true); + this.set('selectedTopicId', null); + this.search(this.get('topicTitle')); + }.observes('topicTitle'), + + topicsChanged: function() { + var topics = this.get('topics'); + if (topics) { + this.set('noResults', topics.length === 0); + } + this.set('loading', false); + }.observes('topics'), + + search: Discourse.debounce(function(title) { + var chooseTopicView = this; + Discourse.Search.forTerm(title, 'topic').then(function (facets) { + if (facets && facets[0] && facets[0].results) { + chooseTopicView.set('topics', facets[0].results); + } else { + chooseTopicView.set('topics', null); + chooseTopicView.set('loading', false); + } + }); + }, 300), + + chooseTopic: function (topic) { + var topicId = Em.get(topic, 'id'); + this.set('selectedTopicId', topicId); + + Em.run.next(function() { + $('#choose-topic-' + topicId).prop('checked', 'true'); + }); + + return false; + } + +}); + + diff --git a/app/assets/javascripts/discourse/views/modal/move_selected_existing_topic_view.js b/app/assets/javascripts/discourse/views/modal/move_selected_existing_topic_view.js new file mode 100644 index 00000000000..58ef5595bfb --- /dev/null +++ b/app/assets/javascripts/discourse/views/modal/move_selected_existing_topic_view.js @@ -0,0 +1,47 @@ +/** + A modal view for handling moving of posts to an existing topic + + @class MoveSelectedExistingTopicView + @extends Discourse.ModalBodyView + @namespace Discourse + @module Discourse +**/ +Discourse.MoveSelectedExistingTopicView = Discourse.ModalBodyView.extend(Discourse.SelectedPostsCount, { + templateName: 'modal/move_selected_existing_topic', + title: Em.String.i18n('topic.move_selected.existing_topic.title'), + + buttonDisabled: function() { + if (this.get('saving')) return true; + return this.blank('selectedTopicId'); + }.property('selectedTopicId', 'saving'), + + buttonTitle: function() { + if (this.get('saving')) return Em.String.i18n('saving'); + return Em.String.i18n('topic.move_selected.title'); + }.property('saving'), + + movePostsToExistingTopic: function() { + this.set('saving', true); + + var postIds = this.get('selectedPosts').map(function(p) { return p.get('id'); }); + var moveSelectedView = this; + + Discourse.Topic.movePosts(this.get('topic.id'), { + destination_topic_id: this.get('selectedTopicId'), + post_ids: postIds + }).then(function(result) { + // Posts moved + $('#discourse-modal').modal('hide'); + moveSelectedView.get('topicController').toggleMultiSelect(); + Em.run.next(function() { Discourse.URL.routeTo(result.url); }); + }, function() { + // Error moving posts + moveSelectedView.flash(Em.String.i18n('topic.move_selected.error')); + moveSelectedView.set('saving', false); + }); + return false; + } + +}); + + diff --git a/app/assets/javascripts/discourse/views/modal/move_selected_new_topic_view.js b/app/assets/javascripts/discourse/views/modal/move_selected_new_topic_view.js new file mode 100644 index 00000000000..2f2ce0a95f2 --- /dev/null +++ b/app/assets/javascripts/discourse/views/modal/move_selected_new_topic_view.js @@ -0,0 +1,47 @@ +/** + A modal view for handling moving of posts to a new topic + + @class MoveSelectedNewTopicView + @extends Discourse.ModalBodyView + @namespace Discourse + @module Discourse +**/ +Discourse.MoveSelectedNewTopicView = Discourse.ModalBodyView.extend(Discourse.SelectedPostsCount, { + templateName: 'modal/move_selected_new_topic', + title: Em.String.i18n('topic.move_selected.new_topic.title'), + saving: false, + + buttonDisabled: function() { + if (this.get('saving')) return true; + return this.blank('topicName'); + }.property('saving', 'topicName'), + + buttonTitle: function() { + if (this.get('saving')) return Em.String.i18n('saving'); + return Em.String.i18n('topic.move_selected.title'); + }.property('saving'), + + movePostsToNewTopic: function() { + this.set('saving', true); + + var postIds = this.get('selectedPosts').map(function(p) { return p.get('id'); }); + var moveSelectedView = this; + + Discourse.Topic.movePosts(this.get('topic.id'), { + title: this.get('topicName'), + post_ids: postIds + }).then(function(result) { + // Posts moved + $('#discourse-modal').modal('hide'); + moveSelectedView.get('topicController').toggleMultiSelect(); + Em.run.next(function() { Discourse.URL.routeTo(result.url); }); + }, function() { + // Error moving posts + moveSelectedView.flash(Em.String.i18n('topic.move_selected.error')); + moveSelectedView.set('saving', false); + }); + return false; + } +}); + + diff --git a/app/assets/javascripts/discourse/views/modal/move_selected_view.js b/app/assets/javascripts/discourse/views/modal/move_selected_view.js index 6186c0d4909..7c155aa1d19 100644 --- a/app/assets/javascripts/discourse/views/modal/move_selected_view.js +++ b/app/assets/javascripts/discourse/views/modal/move_selected_view.js @@ -1,49 +1,37 @@ /** - A modal view for handling moving of posts to a new topic + A modal view for handling moving of posts. @class MoveSelectedView @extends Discourse.ModalBodyView @namespace Discourse @module Discourse **/ -Discourse.MoveSelectedView = Discourse.ModalBodyView.extend({ +Discourse.MoveSelectedView = Discourse.ModalBodyView.extend(Discourse.SelectedPostsCount, { templateName: 'modal/move_selected', title: Em.String.i18n('topic.move_selected.title'), - saving: false, - selectedCount: function() { - if (!this.get('selectedPosts')) return 0; - return this.get('selectedPosts').length; - }.property('selectedPosts'), + showMoveNewTopic: function() { + var modalController = this.get('controller'); + if (!modalController) return; - buttonDisabled: function() { - if (this.get('saving')) return true; - return this.blank('topicName'); - }.property('saving', 'topicName'), + modalController.show(Discourse.MoveSelectedNewTopicView.create({ + topicController: this.get('topicController'), + topic: this.get('topic'), + selectedPosts: this.get('selectedPosts') + })); + }, - buttonTitle: function() { - if (this.get('saving')) return Em.String.i18n('saving'); - return Em.String.i18n('topic.move_selected.title'); - }.property('saving'), + showMoveExistingTopic: function() { + var modalController = this.get('controller'); + if (!modalController) return; - movePosts: function() { - this.set('saving', true); - - var postIds = this.get('selectedPosts').map(function(p) { return p.get('id'); }); - var moveSelectedView = this; - - Discourse.Topic.movePosts(this.get('topic.id'), this.get('topicName'), postIds).then(function(result) { - // Posts moved - $('#discourse-modal').modal('hide'); - moveSelectedView.get('topicController').toggleMultiSelect(); - Em.run.next(function() { Discourse.URL.routeTo(result.url); }); - }, function() { - // Error moving posts - moveSelectedView.flash(Em.String.i18n('topic.move_selected.error')); - moveSelectedView.set('saving', false); - }); - return false; + modalController.show(Discourse.MoveSelectedExistingTopicView.create({ + topicController: this.get('topicController'), + topic: this.get('topic'), + selectedPosts: this.get('selectedPosts') + })); } + }); diff --git a/app/assets/javascripts/discourse/views/post_view.js b/app/assets/javascripts/discourse/views/post_view.js index c3adb9acf08..f320a61a4e4 100644 --- a/app/assets/javascripts/discourse/views/post_view.js +++ b/app/assets/javascripts/discourse/views/post_view.js @@ -50,8 +50,8 @@ Discourse.PostView = Discourse.View.extend({ }, selectText: function() { - return this.get('post.selected') ? Em.String.i18n('topic.multi_select.selected', { count: this.get('controller.selectedCount') }) : Em.String.i18n('topic.multi_select.select'); - }.property('post.selected', 'controller.selectedCount'), + return this.get('post.selected') ? Em.String.i18n('topic.multi_select.selected', { count: this.get('controller.selectedPostsCount') }) : Em.String.i18n('topic.multi_select.select'); + }.property('post.selected', 'controller.selectedPostsCount'), repliesHidden: function() { return !this.get('repliesShown'); diff --git a/app/assets/javascripts/discourse/views/search/search_view.js b/app/assets/javascripts/discourse/views/search/search_view.js index 0e463d5781f..44ed5350f30 100644 --- a/app/assets/javascripts/discourse/views/search/search_view.js +++ b/app/assets/javascripts/discourse/views/search/search_view.js @@ -46,6 +46,13 @@ Discourse.SearchView = Discourse.View.extend({ return this.set('selectedIndex', 0); }.observes('term', 'typeFilter'), + searchTerm: Discourse.debouncePromise(function(term, typeFilter) { + var searchView = this; + return Discourse.Search.forTerm(term, typeFilter).then(function(results) { + searchView.set('results', results); + }); + }, 300), + showCancelFilter: function() { if (this.get('loading')) return false; return this.present('typeFilter'); @@ -56,7 +63,7 @@ Discourse.SearchView = Discourse.View.extend({ }.observes('term'), // We can re-order them based on the context - content: (function() { + content: function() { var index, order, path, results, results_hashed; if (results = this.get('results')) { // Make it easy to find the results by type @@ -78,29 +85,17 @@ Discourse.SearchView = Discourse.View.extend({ }); } return results; - }).property('results'), + }.property('results'), - updateProgress: (function() { + updateProgress: function() { var results; if (results = this.get('results')) { this.set('noResults', results.length === 0); } return this.set('loading', false); - }).observes('results'), + }.observes('results'), - searchTerm: Discourse.debouncePromise(function(term, typeFilter) { - var searchView = this; - return Discourse.ajax('/search', { - data: { - term: term, - type_filter: typeFilter - } - }).then(function(results) { - searchView.set('results', results); - }); - }, 300), - - resultCount: (function() { + resultCount: function() { var count; if (this.blank('content')) return 0; count = 0; @@ -108,7 +103,7 @@ Discourse.SearchView = Discourse.View.extend({ count += result.results.length; }); return count; - }).property('content'), + }.property('content'), moreOfType: function(type) { this.set('typeFilter', type); diff --git a/app/assets/stylesheets/application/modal.css.scss b/app/assets/stylesheets/application/modal.css.scss index fedebfca265..a27297547fb 100644 --- a/app/assets/stylesheets/application/modal.css.scss +++ b/app/assets/stylesheets/application/modal.css.scss @@ -157,6 +157,20 @@ } #move-selected { + p { + margin-top: 0; + } + + input[type=radio] { + margin-right: 10px; + } + + button { + margin-top: 10px; + display: block; + width: 300px; + } + form { margin-top: 20px; input[type=text] { diff --git a/app/assets/stylesheets/application/topic-post.css.scss b/app/assets/stylesheets/application/topic-post.css.scss index d641e4d4f34..e03e18ef28c 100644 --- a/app/assets/stylesheets/application/topic-post.css.scss +++ b/app/assets/stylesheets/application/topic-post.css.scss @@ -39,6 +39,14 @@ border: 1px solid lighten($blue, 40%); padding: 5px; margin-bottom: 5px; + + button { + width: 160px; + margin: 4px auto; + display: inline-block; + text-align: left; + } + &.hidden { display: none; } diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index ae966f8dc3b..e1ca6a65f0f 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -127,18 +127,22 @@ class TopicsController < ApplicationController end def move_posts - requires_parameters(:title, :post_ids) + requires_parameters(:post_ids) + topic = Topic.where(id: params[:topic_id]).first guardian.ensure_can_move_posts!(topic) - # Move the posts - new_topic = topic.move_posts(current_user, params[:title], params[:post_ids].map {|p| p.to_i}) + args = {} + args[:title] = params[:title] if params[:title].present? + args[:destination_topic_id] = params[:destination_topic_id].to_i if params[:destination_topic_id].present? - if new_topic.present? - render json: {success: true, url: new_topic.relative_url} + dest_topic = topic.move_posts(current_user, params[:post_ids].map {|p| p.to_i}, args) + if dest_topic.present? + render json: {success: true, url: dest_topic.relative_url} else render json: {success: false} end + end def clear_pin diff --git a/app/models/topic.rb b/app/models/topic.rb index 436352ef3c0..fb96ceb6455 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -447,30 +447,53 @@ class Topic < ActiveRecord::Base invite end - def move_posts(moved_by, new_title, post_ids) - topic = nil + def move_posts_to_topic(post_ids, destination_topic) + to_move = posts.where(id: post_ids).order(:created_at) + raise Discourse::InvalidParameters.new(:post_ids) if to_move.blank? + first_post_number = nil Topic.transaction do - topic = Topic.create(user: moved_by, title: new_title, category: category) - - to_move = posts.where(id: post_ids).order(:created_at) - raise Discourse::InvalidParameters.new(:post_ids) if to_move.blank? + # Find the max post number in the topic + max_post_number = destination_topic.posts.maximum(:post_number) || 0 to_move.each_with_index do |post, i| first_post_number ||= post.post_number - row_count = Post.update_all ["post_number = :post_number, topic_id = :topic_id, sort_order = :post_number", post_number: i+1, topic_id: topic.id], id: post.id, topic_id: id + row_count = Post.update_all ["post_number = :post_number, topic_id = :topic_id, sort_order = :post_number", post_number: max_post_number+i+1, topic_id: destination_topic.id], id: post.id, topic_id: id # We raise an error if any of the posts can't be moved raise Discourse::InvalidParameters.new(:post_ids) if row_count == 0 end + end + + + first_post_number + end + + def move_posts(moved_by, post_ids, opts) + + topic = nil + first_post_number = nil + + if opts[:title].present? + # If we're moving to a new topic... + Topic.transaction do + topic = Topic.create(user: moved_by, title: opts[:title], category: category) + first_post_number = move_posts_to_topic(post_ids, topic) + end + + elsif opts[:destination_topic_id].present? + # If we're moving to an existing topic... + + topic = Topic.where(id: opts[:destination_topic_id]).first + Guardian.new(moved_by).ensure_can_see!(topic) + first_post_number = move_posts_to_topic(post_ids, topic) - # Update denormalized values since we've manually moved stuff end # Add a moderator post explaining that the post was moved if topic.present? topic_url = "#{Discourse.base_url}#{topic.relative_url}" - topic_link = "[#{new_title}](#{topic_url})" + topic_link = "[#{topic.title}](#{topic_url})" add_moderator_post(moved_by, I18n.t("move_posts.moderator_post", count: post_ids.size, topic_link: topic_link), post_number: first_post_number) Jobs.enqueue(:notify_moved_posts, post_ids: post_ids, moved_by_id: moved_by.id) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index d0af195b2f9..a67f0d455e0 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -50,6 +50,12 @@ en: saving: "Saving..." saved: "Saved!" + choose_topic: + none_found: "No topics found." + title: + search: "Search for a Topic:" + placeholder: "type the topic title here" + user_action: user_posted_topic: "{{user}} posted the topic" you_posted_topic: "You posted the topic" @@ -573,11 +579,23 @@ en: move_selected: title: "Move Selected Posts" - topic_name: "New Topic Name:" error: "Sorry, there was an error moving those posts." instructions: - one: "You are about to create a new topic and populate it with the post you've selected." - other: "You are about to create a new topic and populate it with the {{count}} posts you've selected." + one: "How would you like to move this post?" + other: "How would you like to move the {{count}} posts you've created?" + + new_topic: + title: "Move Selected Posts to a New Topic" + topic_name: "New Topic Name:" + instructions: + one: "You are about to create a new topic and populate it with the post you've selected." + other: "You are about to create a new topic and populate it with the {{count}} posts you've selected." + + existing_topic: + title: "Move Selected Posts to an Existing Topic" + instructions: + one: "Please choose the topic you'd like to move that post to." + other: "Please choose the topic you'd like to move those {{count}} posts to." multi_select: select: 'select' diff --git a/lib/search.rb b/lib/search.rb index 3d72b1e9d04..42523581ed9 100644 --- a/lib/search.rb +++ b/lib/search.rb @@ -171,7 +171,6 @@ module Search end # Remove attributes when we know they don't matter - row.delete('id') if type == 'user' row['avatar_template'] = User.avatar_template(row['email']) end diff --git a/spec/controllers/topics_controller_spec.rb b/spec/controllers/topics_controller_spec.rb index 450037af437..3c824e7643c 100644 --- a/spec/controllers/topics_controller_spec.rb +++ b/spec/controllers/topics_controller_spec.rb @@ -7,15 +7,11 @@ describe TopicsController do lambda { xhr :post, :move_posts, topic_id: 111, title: 'blah', post_ids: [1,2,3] }.should raise_error(Discourse::NotLoggedIn) end - describe 'when logged in' do + describe 'moving to a new topic' do let!(:user) { log_in(:moderator) } let(:p1) { Fabricate(:post, user: user) } let(:topic) { p1.topic } - it "raises an error without a title" do - lambda { xhr :post, :move_posts, topic_id: topic.id, post_ids: [1,2,3] }.should raise_error(Discourse::InvalidParameters) - end - it "raises an error without postIds" do lambda { xhr :post, :move_posts, topic_id: topic.id, title: 'blah' }.should raise_error(Discourse::InvalidParameters) end @@ -30,20 +26,15 @@ describe TopicsController do let(:p2) { Fabricate(:post, user: user) } before do - Topic.any_instance.expects(:move_posts).with(user, 'blah', [p2.id]).returns(topic) + Topic.any_instance.expects(:move_posts).with(user, [p2.id], title: 'blah').returns(topic) xhr :post, :move_posts, topic_id: topic.id, title: 'blah', post_ids: [p2.id] end it "returns success" do response.should be_success - end - - it "has a JSON response" do - ::JSON.parse(response.body)['success'].should be_true - end - - it "has a url" do - ::JSON.parse(response.body)['url'].should be_present + result = ::JSON.parse(response.body) + result['success'].should be_true + result['url'].should be_present end end @@ -51,24 +42,56 @@ describe TopicsController do let(:p2) { Fabricate(:post, user: user) } before do - Topic.any_instance.expects(:move_posts).with(user, 'blah', [p2.id]).returns(nil) + Topic.any_instance.expects(:move_posts).with(user, [p2.id], title: 'blah').returns(nil) xhr :post, :move_posts, topic_id: topic.id, title: 'blah', post_ids: [p2.id] end + it "returns JSON with a false success" do + response.should be_success + result = ::JSON.parse(response.body) + result['success'].should be_false + result['url'].should be_blank + end + end + end + + describe 'moving to an existing topic' do + let!(:user) { log_in(:moderator) } + let(:p1) { Fabricate(:post, user: user) } + let(:topic) { p1.topic } + let(:dest_topic) { Fabricate(:topic) } + + context 'success' do + let(:p2) { Fabricate(:post, user: user) } + + before do + Topic.any_instance.expects(:move_posts).with(user, [p2.id], destination_topic_id: dest_topic.id).returns(topic) + xhr :post, :move_posts, topic_id: topic.id, post_ids: [p2.id], destination_topic_id: dest_topic.id + end + it "returns success" do response.should be_success + result = ::JSON.parse(response.body) + result['success'].should be_true + result['url'].should be_present end - - it "has success in the JSON" do - ::JSON.parse(response.body)['success'].should be_false - end - - it "has a url" do - ::JSON.parse(response.body)['url'].should be_blank - end - end + context 'failure' do + let(:p2) { Fabricate(:post, user: user) } + + before do + Topic.any_instance.expects(:move_posts).with(user, [p2.id], destination_topic_id: dest_topic.id).returns(nil) + xhr :post, :move_posts, topic_id: topic.id, destination_topic_id: dest_topic.id, post_ids: [p2.id] + end + + it "returns JSON with a false success" do + response.should be_success + result = ::JSON.parse(response.body) + result['success'].should be_false + result['url'].should be_blank + end + end end end diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index 4349ffc184e..90028e25cc3 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -222,12 +222,12 @@ describe Topic do it "enqueues a job to notify users" do topic.stubs(:add_moderator_post) Jobs.expects(:enqueue).with(:notify_moved_posts, post_ids: [p1.id, p4.id], moved_by_id: user.id) - topic.move_posts(user, "new testing topic name", [p1.id, p4.id]) + topic.move_posts(user, [p1.id, p4.id], title: "new testing topic name") end it "adds a moderator post at the location of the first moved post" do topic.expects(:add_moderator_post).with(user, instance_of(String), has_entries(post_number: 2)) - topic.move_posts(user, "new testing topic name", [p2.id, p4.id]) + topic.move_posts(user, [p2.id, p4.id], title: "new testing topic name") end end @@ -235,52 +235,97 @@ describe Topic do context "errors" do it "raises an error when one of the posts doesn't exist" do - lambda { topic.move_posts(user, "new testing topic name", [1003]) }.should raise_error(Discourse::InvalidParameters) + lambda { topic.move_posts(user, [1003], title: "new testing topic name") }.should raise_error(Discourse::InvalidParameters) end it "raises an error if no posts were moved" do - lambda { topic.move_posts(user, "new testing topic name", []) }.should raise_error(Discourse::InvalidParameters) + lambda { topic.move_posts(user, [], title: "new testing topic name") }.should raise_error(Discourse::InvalidParameters) end end - context "afterwards" do + context "successfully moved" do before do topic.expects(:add_moderator_post) TopicUser.update_last_read(user, topic.id, p4.post_number, 0) end - let!(:new_topic) { topic.move_posts(user, "new testing topic name", [p2.id, p4.id]) } + context "to a new topic" do + let!(:new_topic) { topic.move_posts(user, [p2.id, p4.id], title: "new testing topic name") } - it "moved correctly" do - TopicUser.where(user_id: user.id, topic_id: topic.id).first.last_read_post_number.should == p3.post_number + it "moved correctly" do + TopicUser.where(user_id: user.id, topic_id: topic.id).first.last_read_post_number.should == p3.post_number - new_topic.should be_present - new_topic.featured_user1_id.should == another_user.id - new_topic.like_count.should == 1 - new_topic.category.should == category - topic.featured_user1_id.should be_blank - new_topic.posts.should =~ [p2, p4] + new_topic.should be_present + new_topic.featured_user1_id.should == another_user.id + new_topic.like_count.should == 1 + new_topic.category.should == category + topic.featured_user1_id.should be_blank + new_topic.posts.should =~ [p2, p4] - new_topic.reload - new_topic.posts_count.should == 2 - new_topic.highest_post_number.should == 2 + new_topic.reload + new_topic.posts_count.should == 2 + new_topic.highest_post_number.should == 2 - p2.reload - p2.sort_order.should == 1 - p2.post_number.should == 1 + p2.reload + p2.sort_order.should == 1 + p2.post_number.should == 1 - p4.reload - p4.post_number.should == 2 - p4.sort_order.should == 2 + p4.reload + p4.post_number.should == 2 + p4.sort_order.should == 2 - topic.reload - topic.featured_user1_id.should be_blank - topic.like_count.should == 0 - topic.posts_count.should == 2 - topic.posts.should =~ [p1, p3] - topic.highest_post_number.should == p3.post_number + topic.reload + topic.featured_user1_id.should be_blank + topic.like_count.should == 0 + topic.posts_count.should == 2 + topic.posts.should =~ [p1, p3] + topic.highest_post_number.should == p3.post_number + end end + + context "to an existing topic" do + + let!(:destination_topic) { Fabricate(:topic, user: user ) } + let!(:destination_op) { Fabricate(:post, topic: destination_topic, user: user) } + let!(:moved_to) { topic.move_posts(user, [p2.id, p4.id], destination_topic_id: destination_topic.id )} + + it "moved correctly" do + moved_to.should == destination_topic + + # Check out new topic + moved_to.reload + moved_to.posts_count.should == 3 + moved_to.highest_post_number.should == 3 + moved_to.featured_user1_id.should == another_user.id + moved_to.like_count.should == 1 + moved_to.category.should be_blank + + # Posts should be re-ordered + p2.reload + p2.sort_order.should == 2 + p2.post_number.should == 2 + + p4.reload + p4.post_number.should == 3 + p4.sort_order.should == 3 + + # Check out the original topic + topic.reload + topic.posts_count.should == 2 + topic.highest_post_number.should == 3 + topic.featured_user1_id.should be_blank + topic.like_count.should == 0 + topic.posts_count.should == 2 + topic.posts.should =~ [p1, p3] + topic.highest_post_number.should == p3.post_number + + # Should update last reads + TopicUser.where(user_id: user.id, topic_id: topic.id).first.last_read_post_number.should == p3.post_number + end + + end + end end