diff --git a/app/assets/javascripts/discourse/controllers/topic_controller.js b/app/assets/javascripts/discourse/controllers/topic_controller.js index b02d367cce1..ee4c8636825 100644 --- a/app/assets/javascripts/discourse/controllers/topic_controller.js +++ b/app/assets/javascripts/discourse/controllers/topic_controller.js @@ -459,7 +459,17 @@ Discourse.TopicController = Discourse.ObjectController.extend(Discourse.Selected } ]); } else { - post.destroy(user); + post.destroy(user).then(null, function(e) { + console.log('Error case?'); + console.log(e); + post.undoDeleteState(); + var response = $.parseJSON(e.responseText); + if (response && response.errors) { + bootbox.alert(response.errors[0]); + } else { + bootbox.alert(I18n.t('generic_error')); + } + }); } }, diff --git a/app/assets/javascripts/discourse/models/post.js b/app/assets/javascripts/discourse/models/post.js index 948b088f610..c2091061715 100644 --- a/app/assets/javascripts/discourse/models/post.js +++ b/app/assets/javascripts/discourse/models/post.js @@ -153,6 +153,7 @@ Discourse.Post = Discourse.Model.extend({ // We're updating a post return Discourse.ajax("/posts/" + (this.get('id')), { type: 'PUT', + dataType: 'json', data: { post: { raw: this.get('raw'), edit_reason: this.get('editReason') }, image_sizes: this.get('imageSizes') @@ -236,6 +237,8 @@ Discourse.Post = Discourse.Model.extend({ @param {Discourse.User} deletedBy The user deleting the post **/ setDeletedState: function(deletedBy) { + this.set('oldCooked', this.get('cooked')); + // Moderators can delete posts. Regular users can only trigger a deleted at message. if (deletedBy.get('staff')) { this.setProperties({ @@ -255,6 +258,26 @@ Discourse.Post = Discourse.Model.extend({ } }, + /** + Changes the state of the post to NOT be deleted. Does not call the server. + This can only be called after setDeletedState was called, but the delete + failed on the server. + + @method undoDeletedState + **/ + undoDeleteState: function() { + if (this.get('oldCooked')) { + this.setProperties({ + deleted_at: null, + deleted_by: null, + cooked: this.get('oldCooked'), + version: this.get('version') - 1, + can_recover: false, + user_deleted: false + }); + } + }, + /** Deletes a post diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 6a02ed89791..e6d43f5d46d 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -80,15 +80,18 @@ class ApplicationController < ActionController::Base end rescue_from Discourse::NotFound do - rescue_discourse_actions("[error: 'not found']", 404) + rescue_discourse_actions("[error: 'not found']", 404) # TODO: this breaks json responses end rescue_from Discourse::InvalidAccess do - rescue_discourse_actions("[error: 'invalid access']", 403) + rescue_discourse_actions("[error: 'invalid access']", 403) # TODO: this breaks json responses end def rescue_discourse_actions(message, error) if request.format && request.format.json? + # TODO: this doesn't make sense. Stuffing an html page into a json response will cause + # $.parseJSON to fail in the browser. Also returning text like "[error: 'invalid access']" + # from the above rescue_from blocks will fail because that isn't valid json. render status: error, layout: false, text: (error == 404) ? build_not_found_page(error) : message else render text: build_not_found_page(error, 'no_js') diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index 75004399c26..58039f27aa9 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -63,6 +63,12 @@ class PostsController < ApplicationController post = post.with_deleted if guardian.is_staff? post = post.first post.image_sizes = params[:image_sizes] if params[:image_sizes].present? + + if !guardian.can_edit?(post) && post.user_id == current_user.id && post.edit_time_limit_expired? + render json: {errors: [I18n.t('too_late_to_edit')]}, status: 422 + return + end + guardian.ensure_can_edit!(post) # to stay consistent with the create api, @@ -127,6 +133,12 @@ class PostsController < ApplicationController def destroy post = find_post_from_params + + if !guardian.can_delete_post?(post) && post.user_id == current_user.id && post.edit_time_limit_expired? + render json: {errors: [I18n.t('too_late_to_edit')]}, status: 422 + return + end + guardian.ensure_can_delete!(post) destroyer = PostDestroyer.new(current_user, post) diff --git a/app/models/post.rb b/app/models/post.rb index 97e1af5ccf5..e8d336980c3 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -413,6 +413,14 @@ class Post < ActiveRecord::Base end end + def edit_time_limit_expired? + if created_at && SiteSetting.post_edit_time_limit.to_i > 0 + created_at < SiteSetting.post_edit_time_limit.to_i.minutes.ago + else + false + end + end + private def parse_quote_into_arguments(quote) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index fda9b980b7b..3ff9db1b2b7 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -85,6 +85,7 @@ en: rss_description: latest: "Latest topics" hot: "Hot topics" + too_late_to_edit: "That post was created too long ago. It can no longer be edited or deleted." groups: errors: @@ -544,6 +545,7 @@ en: download_remote_images_to_local: "Download a copy of remote images hotlinked in posts" download_remote_images_threshold: "Amount of minimum available disk space required to download remote images locally (in percent)" ninja_edit_window: "Number of seconds after posting where edits do not create a new version" + post_edit_time_limit: "Amount of time in minutes in which posts can be edited and deleted by the author. Set to 0 to allow editing and deleting posts at any time." edit_history_visible_to_public: "Allow everyone to see previous versions of an edited post. When disabled, only staff members can view edit history." delete_removed_posts_after: "Number of hours after which posts removed by the author will be deleted." max_image_width: "Maximum allowed width of images in a post" diff --git a/config/site_settings.yml b/config/site_settings.yml index 29d45b54e61..0ead7d8128f 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -164,6 +164,7 @@ posting: default: 15 enable_private_messages: true ninja_edit_window: 300 + post_edit_time_limit: 525600 edit_history_visible_to_public: client: true default: true diff --git a/lib/guardian.rb b/lib/guardian.rb index d59459ed68b..56083c457ca 100644 --- a/lib/guardian.rb +++ b/lib/guardian.rb @@ -276,7 +276,7 @@ class Guardian end def can_edit_post?(post) - is_staff? || (!post.topic.archived? && is_my_own?(post) && !post.user_deleted &&!post.deleted_at) + is_staff? || (!post.topic.archived? && is_my_own?(post) && !post.user_deleted && !post.deleted_at && !post.edit_time_limit_expired?) end def can_edit_user?(user) @@ -304,6 +304,9 @@ class Guardian # Can't delete the first post return false if post.post_number == 1 + # Can't delete after post_edit_time_limit minutes have passed + return false if !is_staff? && post.edit_time_limit_expired? + # You can delete your own posts return !post.user_deleted? if is_my_own?(post) diff --git a/spec/components/guardian_spec.rb b/spec/components/guardian_spec.rb index 7334f743363..7dfe1ac4c35 100644 --- a/spec/components/guardian_spec.rb +++ b/spec/components/guardian_spec.rb @@ -544,6 +544,29 @@ describe Guardian do it 'returns true as an admin' do Guardian.new(admin).can_edit?(post).should be_true end + + context 'post is older than post_edit_time_limit' do + let(:old_post) { build(:post, topic: topic, user: topic.user, created_at: 6.minutes.ago) } + before do + SiteSetting.stubs(:post_edit_time_limit).returns(5) + end + + it 'returns false to the author of the post' do + Guardian.new(old_post.user).can_edit?(old_post).should eq(false) + end + + it 'returns true as a moderator' do + Guardian.new(moderator).can_edit?(old_post).should eq(true) + end + + it 'returns true as an admin' do + Guardian.new(admin).can_edit?(old_post).should eq(true) + end + + it 'returns false for another regular user trying to edit your post' do + Guardian.new(coding_horror).can_edit?(old_post).should eq(false) + end + end end describe 'a Topic' do @@ -773,6 +796,34 @@ describe Guardian do it 'returns true when an admin' do Guardian.new(admin).can_delete?(post).should be_true end + + context 'post is older than post_edit_time_limit' do + let(:old_post) { build(:post, topic: topic, user: topic.user, post_number: 2, created_at: 6.minutes.ago) } + before do + SiteSetting.stubs(:post_edit_time_limit).returns(5) + end + + it 'returns false to the author of the post' do + Guardian.new(old_post.user).can_delete?(old_post).should eq(false) + end + + it 'returns true as a moderator' do + Guardian.new(moderator).can_delete?(old_post).should eq(true) + end + + it 'returns true as an admin' do + Guardian.new(admin).can_delete?(old_post).should eq(true) + end + + it "returns false when it's the OP, even as a moderator" do + old_post.post_number = 1 + Guardian.new(moderator).can_delete?(old_post).should eq(false) + end + + it 'returns false for another regular user trying to delete your post' do + Guardian.new(coding_horror).can_delete?(old_post).should eq(false) + end + end end context 'a Category' do diff --git a/spec/controllers/posts_controller_spec.rb b/spec/controllers/posts_controller_spec.rb index 5731ec1755f..745f51b32ad 100644 --- a/spec/controllers/posts_controller_spec.rb +++ b/spec/controllers/posts_controller_spec.rb @@ -214,7 +214,7 @@ describe PostsController do end it "raises an error when the user doesn't have permission to see the post" do - Guardian.any_instance.expects(:can_edit?).with(post).returns(false) + Guardian.any_instance.expects(:can_edit?).with(post).at_least_once.returns(false) xhr :put, :update, update_params response.should be_forbidden end