diff --git a/app/assets/javascripts/discourse/controllers/change_owner_controller.js b/app/assets/javascripts/discourse/controllers/change_owner_controller.js new file mode 100644 index 00000000000..865a5c91065 --- /dev/null +++ b/app/assets/javascripts/discourse/controllers/change_owner_controller.js @@ -0,0 +1,57 @@ +/** + Modal related to changing the ownership of posts + + @class ChangeOwnerController + @extends Discourse.ObjectController + @namespace Discourse + @uses Discourse.ModalFunctionality + @module Discourse + **/ +Discourse.ChangeOwnerController = Discourse.ObjectController.extend(Discourse.SelectedPostsCount, Discourse.ModalFunctionality, { + needs: ['topic'], + + topicController: Em.computed.alias('controllers.topic'), + selectedPosts: Em.computed.alias('topicController.selectedPosts'), + + buttonDisabled: function() { + if (this.get('saving')) return true; + return this.blank('new_user'); + }.property('saving', 'new_user'), + + buttonTitle: function() { + if (this.get('saving')) return I18n.t('saving'); + return I18n.t('topic.change_owner.action'); + }.property('saving'), + + onShow: function() { + this.setProperties({ + saving: false, + new_user: '' + }); + }, + + actions: { + changeOwnershipOfPosts: function() { + this.set('saving', true); + + var postIds = this.get('selectedPosts').map(function(p) { return p.get('id'); }), + self = this, + saveOpts = { + post_ids: postIds, + username: this.get('new_user') + }; + + Discourse.Topic.changeOwners(this.get('id'), saveOpts).then(function(result) { + // success + self.send('closeModal'); + self.get('topicController').send('toggleMultiSelect'); + Em.run.next(function() { Discourse.URL.routeTo(result.url); }); + }, function() { + // failure + self.flash(I18n.t('topic.change_owner.error'), 'alert-error'); + self.set('saving', false); + }); + return false; + } + } +}); diff --git a/app/assets/javascripts/discourse/controllers/topic_controller.js b/app/assets/javascripts/discourse/controllers/topic_controller.js index 6ed6aa8df8d..fc67fce07d4 100644 --- a/app/assets/javascripts/discourse/controllers/topic_controller.js +++ b/app/assets/javascripts/discourse/controllers/topic_controller.js @@ -265,6 +265,11 @@ Discourse.TopicController = Discourse.ObjectController.extend(Discourse.Selected return (this.get('selectedPostsCount') > 0); }.property('selectedPostsCount'), + canChangeOwner: function() { + if (!Discourse.User.current().admin) return false; + return !!this.get('selectedPostsUsername'); + }.property('selectedPostsUsername'), + categories: function() { return Discourse.Category.list(); }.property(), diff --git a/app/assets/javascripts/discourse/mixins/selected_posts_count.js b/app/assets/javascripts/discourse/mixins/selected_posts_count.js index e30cfdf40fe..66be20271e7 100644 --- a/app/assets/javascripts/discourse/mixins/selected_posts_count.js +++ b/app/assets/javascripts/discourse/mixins/selected_posts_count.js @@ -19,8 +19,27 @@ Discourse.SelectedPostsCount = Em.Mixin.create({ } return sum; - }.property('selectedPosts.length', 'allPostsSelected', 'selectedReplies.length') + }.property('selectedPosts.length', 'allPostsSelected', 'selectedReplies.length'), + /** + The username that owns every selected post, or undefined if no selection or if + ownership is mixed. + + @returns {String|undefined} username that owns all selected posts + **/ + selectedPostsUsername: function() { + // Don't proceed if replies are selected or usernames are mixed + // Changing ownership in those cases normally doesn't make sense + if (this.get('selectedReplies') && this.get('selectedReplies').length > 0) return; + if (this.get('selectedPosts').length <= 0) return; + + var selectedPosts = this.get('selectedPosts'), + username = selectedPosts[0].username; + + if (selectedPosts.every(function(post) { return post.username === username; })) { + return username; + } + }.property('selectedPosts.length', 'selectedReplies.length') }); diff --git a/app/assets/javascripts/discourse/models/topic.js b/app/assets/javascripts/discourse/models/topic.js index bb7b60fe6e5..a68b5155a3e 100644 --- a/app/assets/javascripts/discourse/models/topic.js +++ b/app/assets/javascripts/discourse/models/topic.js @@ -409,6 +409,17 @@ Discourse.Topic.reopenClass({ return promise; }, + changeOwners: function(topicId, opts) { + var promise = Discourse.ajax("/t/" + topicId + "/change-owner", { + type: 'POST', + data: opts + }).then(function (result) { + if (result.success) return result; + promise.reject(new Error("error changing ownership of posts")); + }); + return promise; + }, + bulkOperation: function(topics, operation) { return Discourse.ajax("/topics/bulk", { type: 'PUT', diff --git a/app/assets/javascripts/discourse/routes/topic_route.js b/app/assets/javascripts/discourse/routes/topic_route.js index 2cf7d60e1f9..122dca61453 100644 --- a/app/assets/javascripts/discourse/routes/topic_route.js +++ b/app/assets/javascripts/discourse/routes/topic_route.js @@ -67,6 +67,10 @@ Discourse.TopicRoute = Discourse.Route.extend({ Discourse.Route.showModal(this, 'splitTopic', this.modelFor('topic')); }, + changeOwner: function() { + Discourse.Route.showModal(this, 'changeOwner', this.modelFor('topic')); + }, + // Use replaceState to update the URL once it changes postChangedRoute: Discourse.debounce(function(currentPost) { // do nothing if we are transitioning to another route diff --git a/app/assets/javascripts/discourse/templates/modal/change_owner.js.handlebars b/app/assets/javascripts/discourse/templates/modal/change_owner.js.handlebars new file mode 100644 index 00000000000..bcf7469d082 --- /dev/null +++ b/app/assets/javascripts/discourse/templates/modal/change_owner.js.handlebars @@ -0,0 +1,16 @@ + + + diff --git a/app/assets/javascripts/discourse/templates/modal/history.js.handlebars b/app/assets/javascripts/discourse/templates/modal/history.js.handlebars index e5d1762f965..81d75ffdf01 100644 --- a/app/assets/javascripts/discourse/templates/modal/history.js.handlebars +++ b/app/assets/javascripts/discourse/templates/modal/history.js.handlebars @@ -30,6 +30,11 @@ {{{category_diff}}} {{/if}} + {{#if user_changes}} +
+ {{boundAvatar user_changes.previous imageSize="small"}} {{user_changes.previous.username}} → {{boundAvatar user_changes.current imageSize="small"}} {{user_changes.current.username}} +
+ {{/if}} {{{body_diff}}} diff --git a/app/assets/javascripts/discourse/templates/selected_posts.js.handlebars b/app/assets/javascripts/discourse/templates/selected_posts.js.handlebars index 1452cc17c2b..95a56ff78f4 100644 --- a/app/assets/javascripts/discourse/templates/selected_posts.js.handlebars +++ b/app/assets/javascripts/discourse/templates/selected_posts.js.handlebars @@ -18,5 +18,8 @@ {{#if canMergeTopic}} {{/if}} +{{#if canChangeOwner}} + +{{/if}}

{{i18n topic.multi_select.cancel}}

diff --git a/app/assets/javascripts/discourse/views/modal/change_owner_view.js b/app/assets/javascripts/discourse/views/modal/change_owner_view.js new file mode 100644 index 00000000000..adfe3c17c0b --- /dev/null +++ b/app/assets/javascripts/discourse/views/modal/change_owner_view.js @@ -0,0 +1,12 @@ +/** + A modal view for handling changing the owner of various posts + + @class ChangeOwnerView + @extends Discourse.ModalBodyView + @namespace Discourse + @module Discourse + **/ +Discourse.ChangeOwnerView = Discourse.ModalBodyView.extend({ + templateName: 'modal/change_owner', + title: I18n.t('topic.change_owner.title') +}); diff --git a/app/assets/javascripts/discourse/views/modal/merge_topic_view.js b/app/assets/javascripts/discourse/views/modal/merge_topic_view.js index ed4658f48f3..24c3b1cb2ed 100644 --- a/app/assets/javascripts/discourse/views/modal/merge_topic_view.js +++ b/app/assets/javascripts/discourse/views/modal/merge_topic_view.js @@ -10,5 +10,3 @@ Discourse.MergeTopicView = Discourse.ModalBodyView.extend({ templateName: 'modal/merge_topic', title: I18n.t('topic.merge_topic.title') }); - - diff --git a/app/assets/stylesheets/desktop/history.scss b/app/assets/stylesheets/desktop/history.scss index 91f3b0947c6..b6a332a5722 100644 --- a/app/assets/stylesheets/desktop/history.scss +++ b/app/assets/stylesheets/desktop/history.scss @@ -35,6 +35,8 @@ word-wrap: break-word; .row, table { margin-top: 10px; + padding-bottom: 10px; + border-bottom: 1px dotted; } } img { diff --git a/app/assets/stylesheets/desktop/topic-post.scss b/app/assets/stylesheets/desktop/topic-post.scss index 6b08d471ae0..df932ff4ef9 100644 --- a/app/assets/stylesheets/desktop/topic-post.scss +++ b/app/assets/stylesheets/desktop/topic-post.scss @@ -622,13 +622,6 @@ iframe { } } -#selected-posts { - padding-left: 20px; - .btn { - margin-bottom: 10px; - } -} - .post-select { float: right; margin-right: 20px; @@ -914,8 +907,9 @@ blockquote { /* solo quotes */ #selected-posts { + padding-left: 20px; margin-left: 330px; - width: 12%; + width: 200px; position: fixed; z-index: 1000; left: 50%; @@ -927,7 +921,7 @@ blockquote { /* solo quotes */ button { - width: 160px; + width: 180px; margin: 4px auto; display: inline-block; text-align: left; @@ -958,9 +952,8 @@ blockquote { /* solo quotes */ border: none; color: $tertiary_text_color; font-weight: normal; - - color: $tertiary_text_color; - background: $btn-primary-background-color; + margin-bottom: 10px; + background: $btn-primary-background-color; &[href] { color: $tertiary_text_color; diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index f31fe6f96e6..0f1c5885911 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -22,7 +22,8 @@ class TopicsController < ApplicationController :clear_pin, :autoclose, :bulk, - :reset_new] + :reset_new, + :change_post_owners] before_filter :consider_user_for_promotion, only: :show @@ -242,6 +243,37 @@ class TopicsController < ApplicationController render_topic_changes(dest_topic) end + def change_post_owners + params.require(:post_ids) + params.require(:topic_id) + params.require(:username) + + guardian.ensure_can_change_post_owner! + + topic = Topic.find(params[:topic_id].to_i) + new_user = User.find_by_username(params[:username]) + ids = params[:post_ids].to_a + + unless new_user && topic && ids + render json: failed_json, status: 422 + return + end + + ActiveRecord::Base.transaction do + ids.each do |id| + post = Post.find(id) + if post.is_first_post? + topic.user = new_user # Update topic owner (first avatar) + end + post.set_owner(new_user, current_user) + end + end + + topic.update_statistics + + render json: success_json + end + def clear_pin topic = Topic.where(id: params[:topic_id].to_i).first guardian.ensure_can_see!(topic) diff --git a/app/models/post.rb b/app/models/post.rb index 297c910a9b5..d015f5327e5 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -314,6 +314,16 @@ class Post < ActiveRecord::Base PostRevisor.new(self).revise!(updated_by, new_raw, opts) end + def set_owner(new_user, actor) + revise(actor, self.raw, { + new_user: new_user, + changed_owner: true, + edit_reason: I18n.t('change_owner.post_revision_text', + old_user: self.user.username_lower, + new_user: new_user.username_lower) + }) + end + before_create do PostCreator.before_create_tasks(self) end @@ -472,7 +482,7 @@ class Post < ActiveRecord::Base end def save_revision - modifications = changes.extract!(:raw, :cooked, :edit_reason) + modifications = changes.extract!(:raw, :cooked, :edit_reason, :user_id) # make sure cooked is always present (oneboxes might not change the cooked post) modifications["cooked"] = [self.cooked, self.cooked] unless modifications["cooked"].present? PostRevision.create!( diff --git a/app/models/post_revision.rb b/app/models/post_revision.rb index 58bca9e96b5..990705289ee 100644 --- a/app/models/post_revision.rb +++ b/app/models/post_revision.rb @@ -41,6 +41,17 @@ class PostRevision < ActiveRecord::Base } end + def user_changes + prev = previous("user_id") + cur = current("user_id") + return if prev == cur + + { + previous_user: User.where(id: prev).first, + current_user: User.where(id: cur).first + } + end + def previous(field) lookup_with_fallback(field, 0) end diff --git a/app/serializers/post_revision_serializer.rb b/app/serializers/post_revision_serializer.rb index e2280f12be6..149865502d0 100644 --- a/app/serializers/post_revision_serializer.rb +++ b/app/serializers/post_revision_serializer.rb @@ -9,7 +9,8 @@ class PostRevisionSerializer < ApplicationSerializer :edit_reason, :body_changes, :title_changes, - :category_changes + :category_changes, + :user_changes def include_title_changes? object.has_topic_data? @@ -43,6 +44,26 @@ class PostRevisionSerializer < ApplicationSerializer object.lookup("edit_reason", 1) end + def user_changes + obj = object.user_changes + return unless obj + # same as below - if stuff is messed up, default to system + prev = obj[:previous_user] || Discourse.system_user + new = obj[:current_user] || Discourse.system_user + { + previous: { + username: prev.username_lower, + display_username: prev.username, + avatar_template: prev.avatar_template + }, + current: { + username: new.username_lower, + display_username: new.username, + avatar_template: new.avatar_template + } + } + end + def user # if stuff goes pear shape attribute to system object.user || Discourse.system_user diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index b17a204fd80..89ef0ee58ca 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -802,7 +802,7 @@ en: invisible: "Make Invisible" visible: "Make Visible" reset_read: "Reset Read Data" - multi_select: "Select Posts to Move" + multi_select: "Select Posts" convert_to_topic: "Convert to Regular Topic" reply: @@ -868,6 +868,17 @@ en: 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." + change_owner: + title: "Change Owner of Posts" + action: "change ownership" + error: "There was an error changing the ownership of the posts." + label: "New Owner of Posts" + placeholder: "username of new owner" + instructions: + one: "Please choose the new owner of the post by {{old_user}}." + other: "Please choose the new owner of the {{count}} posts by {{old_user}}." + instructions_warn: "Note that any notifications about this post will not be transferred to the new user retroactively.
Warning: Currently, no post-dependent data is transferred over to the new user. Use with caution." + multi_select: select: 'select' selected: 'selected ({{count}})' diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index fd985bdef1c..a71cd00b39e 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -910,6 +910,9 @@ en: one: "I moved a post to an existing topic: %{topic_link}" other: "I moved %{count} posts to an existing topic: %{topic_link}" + change_owner: + post_revision_text: "Ownership transferred from %{old_user} to %{new_user}" + topic_statuses: archived_enabled: "This topic is now archived. It is frozen and cannot be changed in any way." archived_disabled: "This topic is now unarchived. It is no longer frozen, and can be changed." diff --git a/config/routes.rb b/config/routes.rb index 684080876ee..243b7028872 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -331,6 +331,7 @@ Discourse::Application.routes.draw do post "t/:topic_id/invite" => "topics#invite", constraints: {topic_id: /\d+/} post "t/:topic_id/move-posts" => "topics#move_posts", constraints: {topic_id: /\d+/} post "t/:topic_id/merge-topic" => "topics#merge_topic", constraints: {topic_id: /\d+/} + post "t/:topic_id/change-owner" => "topics#change_post_owners", constraints: {topic_id: /\d+/} delete "t/:topic_id/timings" => "topics#destroy_timings", constraints: {topic_id: /\d+/} post "t/:topic_id/notifications" => "topics#set_notifications" , constraints: {topic_id: /\d+/} diff --git a/lib/guardian/post_guardian.rb b/lib/guardian/post_guardian.rb index 27d55c6d305..0357f08e3a8 100644 --- a/lib/guardian/post_guardian.rb +++ b/lib/guardian/post_guardian.rb @@ -125,4 +125,8 @@ module PostGuardain def can_vote?(post, opts={}) post_can_act?(post,:vote, opts) end + + def can_change_post_owner? + is_admin? + end end diff --git a/lib/post_revisor.rb b/lib/post_revisor.rb index bf796428a6f..edb9a1c296c 100644 --- a/lib/post_revisor.rb +++ b/lib/post_revisor.rb @@ -10,6 +10,7 @@ class PostRevisor # Recognized options: # :edit_reason User-supplied edit reason + # :new_user New owner of the post # :revised_at changes the date of the revision # :force_new_version bypass ninja-edit window # :bypass_bump do not bump the topic, even if last post @@ -45,7 +46,7 @@ class PostRevisor end def should_revise? - @post.raw != @new_raw + @post.raw != @new_raw || @opts[:changed_owner] end def revise_post @@ -63,6 +64,7 @@ class PostRevisor def should_create_new_version? @post.last_editor_id != @editor.id || get_revised_at - @post.last_version_at > SiteSetting.ninja_edit_window.to_i || + @opts[:changed_owner] == true || @opts[:force_new_version] == true end @@ -93,6 +95,7 @@ class PostRevisor @post.word_count = @new_raw.scan(/\w+/).size @post.last_editor_id = @editor.id @post.edit_reason = @opts[:edit_reason] if @opts[:edit_reason] + @post.user_id = @opts[:new_user].id if @opts[:new_user] if @editor == @post.user && @post.hidden && @post.hidden_reason_id == Post.hidden_reasons[:flag_threshold_reached] @post.hidden = false diff --git a/spec/controllers/topics_controller_spec.rb b/spec/controllers/topics_controller_spec.rb index a4a59bb93e3..2f1eae5b6ba 100644 --- a/spec/controllers/topics_controller_spec.rb +++ b/spec/controllers/topics_controller_spec.rb @@ -195,6 +195,73 @@ describe TopicsController do end + context 'change_post_owners' do + it 'needs you to be logged in' do + lambda { xhr :post, :change_post_owners, topic_id: 111, username: 'user_a', post_ids: [1,2,3] }.should raise_error(Discourse::NotLoggedIn) + end + + describe 'forbidden to moderators' do + let!(:moderator) { log_in(:moderator) } + it 'correctly denies' do + xhr :post, :change_post_owners, topic_id: 111, username: 'user_a', post_ids: [1,2,3] + response.should be_forbidden + end + end + + describe 'forbidden to elders' do + let!(:elder) { log_in(:elder) } + + it 'correctly denies' do + xhr :post, :change_post_owners, topic_id: 111, username: 'user_a', post_ids: [1,2,3] + response.should be_forbidden + end + end + + describe 'changing ownership' do + let!(:editor) { log_in(:admin) } + let(:topic) { Fabricate(:topic) } + let(:user_a) { Fabricate(:user) } + let(:p1) { Fabricate(:post, topic_id: topic.id) } + + it "raises an error with a parameter missing" do + lambda { xhr :post, :change_post_owners, topic_id: 111, post_ids: [1,2,3] }.should raise_error(ActionController::ParameterMissing) + lambda { xhr :post, :change_post_owners, topic_id: 111, username: 'user_a' }.should raise_error(ActionController::ParameterMissing) + end + + it "calls PostRevisor" do + PostRevisor.any_instance.expects(:revise!) + xhr :post, :change_post_owners, topic_id: topic.id, username: user_a.username_lower, post_ids: [p1.id] + response.should be_success + end + + it "changes the user" do + old_user = p1.user + xhr :post, :change_post_owners, topic_id: topic.id, username: user_a.username_lower, post_ids: [p1.id] + p1.reload + old_user.should_not == p1.user + end + + # Make sure that p1.reload isn't changing the user for us + it "is not an artifact of the framework" do + old_user = p1.user + # xhr :post, :change_post_owners, topic_id: topic.id, username: user_a.username_lower, post_ids: [p1.id] + p1.reload + p1.user.should_not == nil + old_user.should == p1.user + end + + let(:p2) { Fabricate(:post, topic_id: topic.id) } + + it "changes multiple posts" do + xhr :post, :change_post_owners, topic_id: topic.id, username: user_a.username_lower, post_ids: [p1.id, p2.id] + p1.reload + p2.reload + p1.user.should_not == nil + p1.user.should == p2.user + end + end + end + context 'similar_to' do let(:title) { 'this title is long enough to search for' }