Allow changing ownwership of posts by admins

This commit is contained in:
riking 2014-03-27 18:28:14 -07:00
parent 7a843d2ac2
commit 1540a3d5e5
22 changed files with 308 additions and 20 deletions

View File

@ -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;
}
}
});

View File

@ -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(),

View File

@ -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')
});

View File

@ -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',

View File

@ -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

View File

@ -0,0 +1,16 @@
<div class="modal-body">
{{{i18n topic.change_owner.instructions count="selectedPostsCount" old_user="selectedPostsUsername"}}}
<p>
{{{i18n topic.change_owner.instructions_warn}}}
</p>
<form>
<label>{{i18n topic.change_owner.label}}</label>
{{userSelector single=true usernames=new_user include_groups="false" placeholderKey="topic.change_owner.placeholder"}}
</form>
</div>
<div class="modal-footer">
<button class='btn btn-primary' {{bind-attr disabled="buttonDisabled"}} {{action changeOwnershipOfPosts}}>{{buttonTitle}}</button>
</div>

View File

@ -30,6 +30,11 @@
{{{category_diff}}}
</div>
{{/if}}
{{#if user_changes}}
<div class="row">
{{boundAvatar user_changes.previous imageSize="small"}} {{user_changes.previous.username}}{{boundAvatar user_changes.current imageSize="small"}} {{user_changes.current.username}}
</div>
{{/if}}
{{{body_diff}}}
</div>
</div>

View File

@ -18,5 +18,8 @@
{{#if canMergeTopic}}
<button class='btn' {{action mergeTopic}}><i class='fa fa-sign-out'></i> {{i18n topic.merge_topic.action}}</button>
{{/if}}
{{#if canChangeOwner}}
<button class='btn' {{action changeOwner}}><i class='fa fa-user'></i> {{i18n topic.change_owner.action}}</button>
{{/if}}
<p class='cancel'><a href='#' {{action toggleMultiSelect}}>{{i18n topic.multi_select.cancel}}</a></p>

View File

@ -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')
});

View File

@ -10,5 +10,3 @@ Discourse.MergeTopicView = Discourse.ModalBodyView.extend({
templateName: 'modal/merge_topic',
title: I18n.t('topic.merge_topic.title')
});

View File

@ -35,6 +35,8 @@
word-wrap: break-word;
.row, table {
margin-top: 10px;
padding-bottom: 10px;
border-bottom: 1px dotted;
}
}
img {

View File

@ -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;

View File

@ -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)

View File

@ -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!(

View File

@ -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

View File

@ -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

View File

@ -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 <b>{{count}}</b> 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 <b>{{old_user}}</b>."
other: "Please choose the new owner of the {{count}} posts by <b>{{old_user}}</b>."
instructions_warn: "Note that any notifications about this post will not be transferred to the new user retroactively.<br>Warning: Currently, no post-dependent data is transferred over to the new user. Use with caution."
multi_select:
select: 'select'
selected: 'selected ({{count}})'

View File

@ -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."

View File

@ -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+/}

View File

@ -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

View File

@ -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

View File

@ -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' }