diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index 6eac5176d7c89d9c64b7a92a86d477b8d2cfa673..ae32f3a101dd6534ca05146a5c90e229a696f25f 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -117,15 +117,15 @@ class PostsController < ApplicationController after_tags = if @post_type.has_category? Tag.where(tag_set_id: @post.category.tag_set_id, name: params[:post][:tags_cache]) end - body_rendered = helpers.post_markdown(:post, :body) - new_tags_cache = params[:question][:tags_cache]&.reject(&:empty?) + body_rendered = helpers.post_markdown(:post, :body_markdown) + new_tags_cache = params[:post][:tags_cache]&.reject(&:empty?) - if edit_post_params.all? { |k, v| @post.send(k) == v } + if edit_post_params.to_h.all? { |k, v| @post.send(k) == v } flash[:danger] = "No changes were saved because you didn't edit the post." return redirect_to post_path(@post) end - if current_user.privilege?('edit_posts') + if current_user.privilege?('edit_posts') || current_user.is_moderator || current_user == @post.user if @post.update(edit_post_params.merge(body: body_rendered, last_edited_at: DateTime.now, last_edited_by: current_user, last_activity: DateTime.now, last_activity_by: current_user)) @@ -133,6 +133,7 @@ class PostsController < ApplicationController after: @post.body_markdown, comment: params[:edit_comment], before_title: before[:title], after_title: @post.title, before_tags: before[:tags], after_tags: after_tags) + redirect_to post_path(@post) else render :edit, status: :bad_request end @@ -321,7 +322,8 @@ class PostsController < ApplicationController private def permitted - [:post_type_id, :category_id, :title, :body_markdown, :license_id, :doc_slug, :help_category, :help_ordering] + [:post_type_id, :category_id, :parent_id, :title, :body_markdown, :license_id, + :doc_slug, :help_category, :help_ordering] end def post_params @@ -331,7 +333,8 @@ class PostsController < ApplicationController end def edit_post_params - p = params.require(:post).permit(*(permitted - [:license_id]), tags_cache: []) + p = params.require(:post).permit(*(permitted - [:license_id, :post_type_id, :category_id, :parent_id]), + tags_cache: []) p[:tags_cache] = p[:tags_cache]&.reject { |t| t.empty? } p end diff --git a/app/views/posts/_form.html.erb b/app/views/posts/_form.html.erb index 84ddea1995ec5c27553236b0a7d48af97ab0cdbb..c274a537fc6334d5a4c7c6554b7edbaffbc363ef 100644 --- a/app/views/posts/_form.html.erb +++ b/app/views/posts/_form.html.erb @@ -27,13 +27,15 @@ <%= render 'posts/image_upload' %> <%= form_for post, url: submit_path, html: { class: 'has-margin-top-4' } do |f| %> - <%= f.hidden_field :post_type_id, value: post.post_type_id || post_type.id %> + <% if post.id.nil? %> + <%= f.hidden_field :post_type_id, value: post.post_type_id || post_type.id %> + <% end %> - <% if post_type.has_category? %> + <% if post_type.has_category? && post.id.nil? %> <%= f.hidden_field :category_id, value: post.category_id || category&.id || parent&.category_id %> <% end %> - <% if post_type.has_parent? %> + <% if post_type.has_parent? && post.id.nil? %> <%= f.hidden_field :parent_id, value: post.parent_id || parent&.id %> <% end %> diff --git a/test/controllers/posts_controller_test.rb b/test/controllers/posts_controller_test.rb index 0f83125a33f6b736a0dc2478c11ec676f9eda6ec..fcc735141fb6b06deb889cc4e2208294a72e1494 100644 --- a/test/controllers/posts_controller_test.rb +++ b/test/controllers/posts_controller_test.rb @@ -233,4 +233,73 @@ class PostsControllerTest < ActionController::TestCase assert_response 302 assert_redirected_to new_user_session_path end + + test 'can update post' do + sign_in users(:standard_user) + before_history = PostHistory.where(post: posts(:question_one)).count + patch :update, params: { id: posts(:question_one).id, + post: { title: sample.edit.title, body_markdown: sample.edit.body_markdown, + tags_cache: sample.edit.tags_cache } } + after_history = PostHistory.where(post: posts(:question_one)).count + assert_response 302 + assert_redirected_to post_path(posts(:question_one)) + assert_not_nil assigns(:post) + assert_equal sample.edit.body_markdown, assigns(:post).body_markdown + assert_equal before_history + 1, after_history, 'No PostHistory event created on author update' + end + + test 'moderators can update post' do + sign_in users(:moderator) + before_history = PostHistory.where(post: posts(:question_one)).count + patch :update, params: { id: posts(:question_one).id, + post: { title: sample.edit.title, body_markdown: sample.edit.body_markdown, + tags_cache: sample.edit.tags_cache } } + after_history = PostHistory.where(post: posts(:question_one)).count + assert_response 302 + assert_redirected_to post_path(posts(:question_one)) + assert_not_nil assigns(:post) + assert_equal sample.edit.body_markdown, assigns(:post).body_markdown + assert_equal before_history + 1, after_history, 'No PostHistory event created on moderator update' + end + + test 'update requires authentication' do + patch :update, params: { id: posts(:question_one).id, + post: { title: sample.edit.title, body_markdown: sample.edit.body_markdown, + tags_cache: sample.edit.tags_cache } } + assert_response 302 + assert_redirected_to new_user_session_path + end + + test 'update by unprivileged user generates suggested edit' do + sign_in users(:closer) + before_history = PostHistory.where(post: posts(:question_one)).count + before_edits = SuggestedEdit.where(post: posts(:question_one)).count + before_body = posts(:question_one).body_markdown + patch :update, params: { id: posts(:question_one).id, + post: { title: sample.edit.title, body_markdown: sample.edit.body_markdown, + tags_cache: sample.edit.tags_cache } } + after_history = PostHistory.where(post: posts(:question_one)).count + after_edits = SuggestedEdit.where(post: posts(:question_one)).count + assert_response 302 + assert_redirected_to post_path(posts(:question_one)) + assert_not_nil assigns(:post) + assert_equal before_body, assigns(:post).body_markdown, 'Suggested edit incorrectly applied immediately' + assert_equal before_history, after_history, 'PostHistory event incorrectly created on unprivileged update' + assert_equal before_edits + 1, after_edits, 'No SuggestedEdit created on unprivileged update' + end + + test 'update rejects no change edit' do + sign_in users(:standard_user) + post = posts(:question_one) + before_history = PostHistory.where(post: post).count + patch :update, params: { id: post.id, + post: { title: post.title, body_markdown: post.body_markdown, + tags_cache: post.tags_cache } } + after_history = PostHistory.where(post: posts(:question_one)).count + assert_response 302 + assert_redirected_to post_path(posts(:question_one)) + assert_not_nil assigns(:post) + assert_not_nil flash[:danger] + assert_equal before_history, after_history, 'PostHistory event incorrectly created on no-change update' + end end diff --git a/test/fixtures/user_abilities.yml b/test/fixtures/user_abilities.yml index 0cbcccbe2b9316ab8bac1b363e6b4ef1c6f15eb7..6d4b9dd5ec1b1e896aaf5a0ec2ea92fd31b15781 100644 --- a/test/fixtures/user_abilities.yml +++ b/test/fixtures/user_abilities.yml @@ -1,10 +1,3 @@ -# Read about fixtures at http://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html - -# This model initially had no columns defined. If you add columns to the -# model remove the '{}' from the fixture names and add the columns immediately -# below each fixture, per the syntax in the comments below -# - stu_eo: community_user: sample_standard_user ability: everyone diff --git a/test/test_helper.rb b/test/test_helper.rb index 1295d968b52b86bf75958f2dd598f2471e5c64c3..4e1b5d47b9edc3da8bae83d8497ad7c3580204ae 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -41,7 +41,13 @@ class ActiveSupport::TestCase title: 'This is a sample title', body_markdown: 'This is a sample post with some **Markdown** and [a link](/).', body: '<p>This is a sample post with some <b>Markdown</b> and <a href="/">a link</a></p>', - tags_cache: ['discussion', 'posts', 'tags'] + tags_cache: ['discussion', 'posts', 'tags'], + edit: OpenStruct.new( + title: 'This is another sample title', + body_markdown: 'This is a sample post with some more **Markdown** and [a link](/).', + body: '<p>This is a sample post with some more <b>Markdown</b> and <a href="/">a link</a></p>', + tags_cache: ['discussion', 'posts', 'tags', 'edits'], + ) ) end end