From 6bbd83067d67a115af31cf4b2fc4a907a7870911 Mon Sep 17 00:00:00 2001 From: Rimian Perkins Date: Fri, 6 Sep 2019 21:44:12 +1000 Subject: [PATCH] FEATURE: New post editing period for >= tl2 users (#8070) * FEATURE: Add tl2 threshold for editing new posts * Adds a new setting and for tl2 editing posts (30 days same as old value) * Sets the tl0/tl1 editing period as 1 day * FIX: Spec uses wrong setting * Fix site setting on guardian spec * FIX: post editing period specs * Avoid shared examples * Use update_columns to avoid callbacks on user during tests --- app/controllers/posts_controller.rb | 2 +- app/models/concerns/limited_edit.rb | 17 ++++-- config/locales/server.en.yml | 3 +- config/site_settings.yml | 3 ++ lib/guardian/post_guardian.rb | 4 +- lib/guardian/topic_guardian.rb | 2 +- spec/components/guardian_spec.rb | 73 +++++++++++++++++++------- spec/requests/posts_controller_spec.rb | 20 ++++++- 8 files changed, 95 insertions(+), 29 deletions(-) diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index 03221e65ca1..d5a9a12c0da 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -204,7 +204,7 @@ class PostsController < ApplicationController if !guardian.public_send("can_edit?", post) && post.user_id == current_user.id && - post.edit_time_limit_expired? + post.edit_time_limit_expired?(current_user) return render_json_error(I18n.t('too_late_to_edit')) end diff --git a/app/models/concerns/limited_edit.rb b/app/models/concerns/limited_edit.rb index cbe15543870..bfd21392b5b 100644 --- a/app/models/concerns/limited_edit.rb +++ b/app/models/concerns/limited_edit.rb @@ -3,11 +3,22 @@ module LimitedEdit extend ActiveSupport::Concern - 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 + def edit_time_limit_expired?(user) + time_limit = user_time_limit(user) + if created_at && time_limit > 0 + created_at < time_limit.minutes.ago else false end end + + private + + def user_time_limit(user) + if user.trust_level < 2 + SiteSetting.post_edit_time_limit.to_i + else + SiteSetting.tl2_post_edit_time_limit.to_i + end + end end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 60f20e54c07..ff522b004ec 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1361,7 +1361,8 @@ en: editing_grace_period_max_diff: "Maximum number of character changes allowed in editing grace period, if more changed store another post revision (trust level 0 and 1)" editing_grace_period_max_diff_high_trust: "Maximum number of character changes allowed in editing grace period, if more changed store another post revision (trust level 2 and up)" staff_edit_locks_post: "Posts will be locked from editing if they are edited by staff members" - post_edit_time_limit: "The author can edit their post for (n) minutes after posting. Set to 0 for forever." + post_edit_time_limit: "A tl0 or tl1 author can edit their post for (n) minutes after posting. Set to 0 for forever." + tl2_post_edit_time_limit: "A tl2 author can edit their post for (n) minutes after posting. Set to 0 for forever." edit_history_visible_to_public: "Allow everyone to see previous versions of an edited post. When disabled, only staff members can view." delete_removed_posts_after: "Posts removed by the author will be automatically deleted after (n) hours. If set to 0, posts will be deleted immediately." max_image_width: "Maximum thumbnail width of images in a post" diff --git a/config/site_settings.yml b/config/site_settings.yml index 67683bccef5..c1a6cbfd74f 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -700,6 +700,9 @@ posting: type: category default: "" post_edit_time_limit: + default: 1440 + max: 10080 + tl2_post_edit_time_limit: default: 43200 max: 525600 edit_history_visible_to_public: diff --git a/lib/guardian/post_guardian.rb b/lib/guardian/post_guardian.rb index a8344e9fa83..25c46de3c5d 100644 --- a/lib/guardian/post_guardian.rb +++ b/lib/guardian/post_guardian.rb @@ -158,7 +158,7 @@ module PostGuardian return true end - return !post.edit_time_limit_expired? + return !post.edit_time_limit_expired?(@user) end false @@ -238,7 +238,7 @@ module PostGuardian if @user.has_trust_level?(SiteSetting.min_trust_to_allow_self_wiki) && is_my_own?(post) return false if post.hidden? - return !post.edit_time_limit_expired? + return !post.edit_time_limit_expired?(@user) end false diff --git a/lib/guardian/topic_guardian.rb b/lib/guardian/topic_guardian.rb index e171d32d90f..89f5fe6e54a 100644 --- a/lib/guardian/topic_guardian.rb +++ b/lib/guardian/topic_guardian.rb @@ -106,7 +106,7 @@ module TopicGuardian return false if topic.archived is_my_own?(topic) && - !topic.edit_time_limit_expired? && + !topic.edit_time_limit_expired?(user) && !Post.where(topic_id: topic.id, post_number: 1).where.not(locked_by_id: nil).exists? end diff --git a/spec/components/guardian_spec.rb b/spec/components/guardian_spec.rb index 926bf310e83..0dddab72819 100644 --- a/spec/components/guardian_spec.rb +++ b/spec/components/guardian_spec.rb @@ -1389,32 +1389,65 @@ describe Guardian do expect(Guardian.new(post.user).can_edit?(post)).to be_truthy 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) } + describe 'post edit time limits' do + 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.post_edit_time_limit = 5 + before do + topic.user.update_columns(trust_level: 1) + SiteSetting.post_edit_time_limit = 5 + end + + it 'returns false to the author of the post' do + expect(Guardian.new(old_post.user).can_edit?(old_post)).to be_falsey + end + + it 'returns true as a moderator' do + expect(Guardian.new(moderator).can_edit?(old_post)).to eq(true) + end + + it 'returns true as an admin' do + expect(Guardian.new(admin).can_edit?(old_post)).to eq(true) + end + + it 'returns false for another regular user trying to edit your post' do + expect(Guardian.new(coding_horror).can_edit?(old_post)).to be_falsey + end + + it 'returns true for another regular user trying to edit a wiki post' do + old_post.wiki = true + expect(Guardian.new(coding_horror).can_edit?(old_post)).to be_truthy + end end - it 'returns false to the author of the post' do - expect(Guardian.new(old_post.user).can_edit?(old_post)).to be_falsey - end + context 'post is older than tl2_post_edit_time_limit' do + let(:old_post) { build(:post, topic: topic, user: topic.user, created_at: 12.minutes.ago) } - it 'returns true as a moderator' do - expect(Guardian.new(moderator).can_edit?(old_post)).to eq(true) - end + before do + topic.user.update_columns(trust_level: 2) + SiteSetting.tl2_post_edit_time_limit = 10 + end - it 'returns true as an admin' do - expect(Guardian.new(admin).can_edit?(old_post)).to eq(true) - end + it 'returns false to the author of the post' do + expect(Guardian.new(old_post.user).can_edit?(old_post)).to be_falsey + end - it 'returns false for another regular user trying to edit your post' do - expect(Guardian.new(coding_horror).can_edit?(old_post)).to be_falsey - end + it 'returns true as a moderator' do + expect(Guardian.new(moderator).can_edit?(old_post)).to eq(true) + end - it 'returns true for another regular user trying to edit a wiki post' do - old_post.wiki = true - expect(Guardian.new(coding_horror).can_edit?(old_post)).to be_truthy + it 'returns true as an admin' do + expect(Guardian.new(admin).can_edit?(old_post)).to eq(true) + end + + it 'returns false for another regular user trying to edit your post' do + expect(Guardian.new(coding_horror).can_edit?(old_post)).to be_falsey + end + + it 'returns true for another regular user trying to edit a wiki post' do + old_post.wiki = true + expect(Guardian.new(coding_horror).can_edit?(old_post)).to be_truthy + end end end @@ -2854,7 +2887,7 @@ describe Guardian do let(:old_post) { build(:post, user: trust_level_2, created_at: 6.minutes.ago) } before do SiteSetting.min_trust_to_allow_self_wiki = 2 - SiteSetting.post_edit_time_limit = 5 + SiteSetting.tl2_post_edit_time_limit = 5 end it 'returns false when user satisfies trust level and owns the post' do diff --git a/spec/requests/posts_controller_spec.rb b/spec/requests/posts_controller_spec.rb index 7a8c83f97c4..7e10e1a4ae0 100644 --- a/spec/requests/posts_controller_spec.rb +++ b/spec/requests/posts_controller_spec.rb @@ -313,8 +313,26 @@ describe PostsController do sign_in(user) end - it 'does not allow to update when edit time limit expired' do + it 'does not allow TL0 or TL1 to update when edit time limit expired' do SiteSetting.post_edit_time_limit = 5 + SiteSetting.tl2_post_edit_time_limit = 30 + + post = Fabricate(:post, created_at: 10.minutes.ago, user: user) + + user.update_columns(trust_level: 1) + + put "/posts/#{post.id}.json", params: update_params + + expect(response.status).to eq(422) + expect(JSON.parse(response.body)['errors']).to include(I18n.t('too_late_to_edit')) + end + + it 'does not allow TL2 to update when edit time limit expired' do + SiteSetting.post_edit_time_limit = 12 + SiteSetting.tl2_post_edit_time_limit = 8 + + user.update_columns(trust_level: 2) + post = Fabricate(:post, created_at: 10.minutes.ago, user: user) put "/posts/#{post.id}.json", params: update_params