WIP - changing ownership of posts

(Kane York) #1

PR: Change post owner functionality by riking 路 Pull Request #2203 路 discourse/discourse 路 GitHub

@lightyear code review time :wink:

The UI is pretty much done, now I need to do the server side:


Please go through the diff and point out any coding errors. I鈥檒l go through it for style and the JSDoc before making a PR. Also, be prepared for force-pushes to that branch.

I鈥檝e imposed these restrictions on reposessing posts:

  • All selected posts must be by the same author
  • If you click 鈥榮elect with replies鈥 it won鈥檛 work
  • Staff Admin only (unimplemented - TL4 will see the change ownership dialog but fail)

The first two restrictions came about because I was having trouble with the select with replies, then I realized that it wouldn鈥檛 be very useful to change ownership of an entire reply chain anyways.

The third restriction is up for debate.

Screenshot (I鈥檓 pretty sure this includes everything I changed in layout and CSS):

Allow mods to change post ownership
Support deleting a user but keeping their posts
(Sam Saffron) #2

Quick one, before a deeper review, change ownership is admin only, not a staff function. We also need to 鈥渟trip鈥 ownership but I guess we can just give the posts to System user.

Finally, this MUST be logged in post_revisions.

Allow mods to change post ownership
(Kane York) #3

I鈥檓 getting an error thrown on the way through user_action_observer:

Discourse::InvalidParameters (user_id):
  app/models/user_action.rb:300:in `block in require_parameters'
  app/models/user_action.rb:299:in `each'
  app/models/user_action.rb:299:in `require_parameters'
  app/models/user_action.rb:134:in `log_action!'
  app/models/user_action_observer.rb:94:in `block in log_post'
  app/models/user_action_observer.rb:92:in `each'
  app/models/user_action_observer.rb:92:in `log_post'
  app/models/user_action_observer.rb:11:in `after_save'
  lib/post_revisor.rb:108:in `update_post'
  lib/post_revisor.rb:74:in `block in revise_and_create_new_version'
  lib/post_revisor.rb:71:in `revise_and_create_new_version'
  lib/post_revisor.rb:54:in `revise_post'
  lib/post_revisor.rb:23:in `revise!'
  app/models/post.rb:314:in `revise'
  app/models/post.rb:318:in `set_owner'
  app/controllers/topics_controller.rb:268:in `block in change_post_owners'
  app/controllers/topics_controller.rb:263:in `each'
  app/controllers/topics_controller.rb:263:in `change_post_owners'
  config/initializers/quiet_logger.rb:10:in `call_with_quiet_assets'
  config/initializers/silence_logger.rb:19:in `call'
  lib/middleware/missing_avatars.rb:21:in `call'
  lib/middleware/turbo_dev.rb:32:in `call'

I鈥檓 changing @post.user_id in the update_post method of PostRevisor.

EDIT: Pushed my code. Look at this commit: https://github.com/riking/discourse/commit/279a685827adaab6635be91c98050f45ee805dcb

Hmm, this error has rather destructive effects鈥 I鈥檓 going to wrap the whole thing in a transaction

(Kane York) #4

Figured out鈥 something.

Normally, a line in user_action.rb bails out if the record already exists. This is happening every time someone edits a post.

And because Post.user_id is invalid when I鈥檓 changing the user id, being set to null because I鈥檓 confusing a User and their ID it鈥檚 nil to the observer and the check throws. Kaboom

discourse/user_action.rb at 6373de550f5d9175de3afb2a815e568c1d268d25 路 discourse/discourse 路 GitHub

UPDATE: It works! :smiley:

(Kane York) #5

And I now have the actual change details being shipped to the client, with a proper fallback to the System user if the user is deleted:

And crappy revision UI done:

(Kane York) #6

PR sent, blast away at my code: Change post owner functionality by riking 路 Pull Request #2203 路 discourse/discourse 路 GitHub

(Allen - Watchman Monitoring) #7

Thanks for this @riking! It worked great once I found the control in the Wrench at the top of the topic.

(Jeff Atwood) #8