FEATURE: Category setting to allow unlimited first post edits by the owner of the topic (#12690)

This PR adds a new category setting which is a column in the `categories` table, `allow_unlimited_owner_edits_on_first_post`.

What this does is:

* Inside the `can_edit_post?` method of `PostGuardian`, if the current user editing a post is the owner of the post, it is the first post, and the topic's category has `allow_unlimited_owner_edits_on_first_post`, then we bypass the check for `LimitedEdit#edit_time_limit_expired?` on that post.
* Also, similar to wiki topics, in `PostActionNotifier#after_create_post_revision` we send a notification to all users watching a topic when the OP is edited in a topic with the category setting `allow_unlimited_owner_edits_on_first_post` enabled.

This is useful for forums where there is a Marketplace or similar category, where topics are created and then updated indefinitely by the OP rather than the OP making new topics or additional replies. In a way this acts similar to a wiki that only one person can edit.
This commit is contained in:
Martin Brennan 2021-04-14 15:54:09 +10:00 committed by GitHub
parent d56da72fe9
commit eeaecd4fd2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 232 additions and 90 deletions

View File

@ -204,6 +204,8 @@ const Category = RestModel.extend({
custom_fields: this.custom_fields,
topic_template: this.topic_template,
all_topics_wiki: this.all_topics_wiki,
allow_unlimited_owner_edits_on_first_post: this
.allow_unlimited_owner_edits_on_first_post,
allowed_tags: this.allowed_tags,
allowed_tag_groups: this.allowed_tag_groups,
allow_global_tags: this.allow_global_tags,

View File

@ -77,6 +77,13 @@
{{i18n "category.all_topics_wiki"}}
</label>
</section>
<section class="field allow-unlimited-owner-edits-on-first-post">
<label>
{{input type="checkbox" checked=category.allow_unlimited_owner_edits_on_first_post}}
{{i18n "category.allow_unlimited_owner_edits_on_first_post"}}
</label>
</section>
</section>
<section>

View File

@ -296,6 +296,7 @@ class CategoriesController < ApplicationController
:email_in_allow_strangers,
:mailinglist_mirror,
:all_topics_wiki,
:allow_unlimited_owner_edits_on_first_post,
:parent_category_id,
:auto_close_hours,
:auto_close_based_on_last_post,

View File

@ -1011,13 +1011,14 @@ end
# min_tags_from_required_group :integer default(1), not null
# read_only_banner :string
# default_list_filter :string(20) default("all")
# allow_unlimited_owner_edits_on_first_post :boolean default(FALSE)
#
# Indexes
#
# index_categories_on_email_in (email_in) UNIQUE
# index_categories_on_forum_thread_count (topic_count)
# index_categories_on_reviewable_by_group_id (reviewable_by_group_id)
# index_categories_on_search_priority (search_priority)
# index_categories_on_topic_count (topic_count)
# unique_index_categories_on_name (COALESCE(parent_category_id, '-1'::integer), name) UNIQUE
# unique_index_categories_on_slug (COALESCE(parent_category_id, '-1'::integer), slug) UNIQUE WHERE ((slug)::text <> ''::text)
# unique_index_categories_on_slug (COALESCE(parent_category_id, '-1'::integer), lower((slug)::text)) UNIQUE WHERE ((slug)::text <> ''::text)
#

View File

@ -1418,6 +1418,10 @@ class Topic < ActiveRecord::Base
category && category.read_restricted
end
def category_allows_unlimited_owner_edits_on_first_post?
category && category.allow_unlimited_owner_edits_on_first_post?
end
def acting_user
@acting_user || user
end

View File

@ -12,6 +12,7 @@ class CategorySerializer < SiteCategorySerializer
:email_in_allow_strangers,
:mailinglist_mirror,
:all_topics_wiki,
:allow_unlimited_owner_edits_on_first_post,
:can_delete,
:cannot_delete_reason,
:is_special,

View File

@ -110,7 +110,10 @@ class PostActionNotifier
user_ids << post.user_id
end
if post.wiki && post.is_first_post?
# Notify all users watching the topic when the OP of a wiki topic is edited
# or if the topic category allows unlimited owner edits on the OP.
if post.is_first_post? &&
(post.wiki? || post.topic.category_allows_unlimited_owner_edits_on_first_post?)
user_ids.concat(
TopicUser.watching(post.topic_id)
.where.not(user_id: post_revision.user_id)

View File

@ -3171,6 +3171,7 @@ en:
num_featured_topics: "Number of topics shown on the categories page:"
subcategory_num_featured_topics: "Number of featured topics on parent category's page:"
all_topics_wiki: "Make new topics wikis by default"
allow_unlimited_owner_edits_on_first_post: "Allow unlimited owner edits on first post"
subcategory_list_style: "Subcategory List Style:"
sort_order: "Topic List Sort By:"
default_view: "Default Topic List:"

View File

@ -0,0 +1,7 @@
# frozen_string_literal: true
class AddCategorySettingAllowUnlimitedOwnerEditsOp < ActiveRecord::Migration[6.0]
def change
add_column :categories, :allow_unlimited_owner_edits_on_first_post, :boolean, default: false, null: false
end
end

View File

@ -137,6 +137,7 @@ module PostGuardian
return false
end
# Editing a shared draft.
return true if (
can_see_post?(post) &&
can_create_post?(post.topic) &&
@ -165,6 +166,10 @@ module PostGuardian
return true
end
if post.is_first_post? && post.topic.category_allows_unlimited_owner_edits_on_first_post?
return true
end
return !post.edit_time_limit_expired?(@user)
end

View File

@ -455,32 +455,4 @@ describe UserGuardian do
expect(guardian.can_upload_user_card_background?(trust_level_1)).to eq(false)
end
end
describe '#can_edit_post?' do
fab!(:category) { Fabricate(:category) }
let(:topic) { Fabricate(:topic, category: category) }
let(:post_with_draft) { Fabricate(:post, topic: topic) }
before do
SiteSetting.shared_drafts_category = category.id
SiteSetting.shared_drafts_min_trust_level = '2'
Fabricate(:shared_draft, topic: topic)
end
it 'returns true if a shared draft exists' do
expect(Guardian.new(trust_level_2).can_edit_post?(post_with_draft)).to eq(true)
end
it 'returns false if the user has a lower trust level' do
expect(Guardian.new(trust_level_1).can_edit_post?(post_with_draft)).to eq(false)
end
it 'returns false if the draft is from a different category' do
topic.update!(category: Fabricate(:category))
expect(Guardian.new(trust_level_2).can_edit_post?(post_with_draft)).to eq(false)
end
end
end

View File

@ -1492,6 +1492,33 @@ describe Guardian do
expect(Guardian.new(post.user).can_edit?(post)).to be_truthy
end
context "shared drafts" do
fab!(:category) { Fabricate(:category) }
let(:topic) { Fabricate(:topic, category: category) }
let(:post_with_draft) { Fabricate(:post, topic: topic) }
before do
SiteSetting.shared_drafts_category = category.id
SiteSetting.shared_drafts_min_trust_level = '2'
Fabricate(:shared_draft, topic: topic)
end
it 'returns true if a shared draft exists' do
expect(Guardian.new(trust_level_2).can_edit_post?(post_with_draft)).to eq(true)
end
it 'returns false if the user has a lower trust level' do
expect(Guardian.new(trust_level_1).can_edit_post?(post_with_draft)).to eq(false)
end
it 'returns false if the draft is from a different category' do
topic.update!(category: Fabricate(:category))
expect(Guardian.new(trust_level_2).can_edit_post?(post_with_draft)).to eq(false)
end
end
context 'category group moderation is enabled' do
fab!(:cat_mod_user) { Fabricate(:user) }
@ -1511,8 +1538,10 @@ describe Guardian do
end
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) }
let(:topic) { Fabricate(:topic) }
let(:old_post) { Fabricate(:post, topic: topic, user: topic.user, created_at: 6.minutes.ago) }
before do
topic.user.update_columns(trust_level: 1)
@ -1539,6 +1568,31 @@ describe Guardian do
old_post.wiki = true
expect(Guardian.new(coding_horror).can_edit?(old_post)).to be_truthy
end
context "unlimited owner edits on first post" do
let(:owner) { old_post.user }
it "returns true when the post topic's category allow_unlimited_owner_edits_on_first_post" do
old_post.topic.category.update(allow_unlimited_owner_edits_on_first_post: true)
expect(Guardian.new(owner).can_edit?(old_post)).to be_truthy
end
it "returns false when the post topic's category does not allow_unlimited_owner_edits_on_first_post" do
old_post.topic.category.update(allow_unlimited_owner_edits_on_first_post: false)
expect(Guardian.new(owner).can_edit?(old_post)).to be_falsey
end
it "returns false when the post topic's category allow_unlimited_owner_edits_on_first_post but the post is not the first in the topic" do
old_post.topic.category.update(allow_unlimited_owner_edits_on_first_post: true)
new_post = Fabricate(:post, user: owner, topic: old_post.topic, created_at: 6.minutes.ago)
expect(Guardian.new(owner).can_edit?(new_post)).to be_falsey
end
it "returns false when someone other than owner is editing and category allow_unlimited_owner_edits_on_first_post" do
old_post.topic.category.update(allow_unlimited_owner_edits_on_first_post: false)
expect(Guardian.new(coding_horror).can_edit?(old_post)).to be_falsey
end
end
end
context 'post is older than tl2_post_edit_time_limit' do

View File

@ -131,6 +131,15 @@
"min_tags_from_required_group": {
"type": "integer"
},
"allowed_tags": {
"type": "array"
},
"allowed_tag_groups": {
"type": "array"
},
"allow_global_tags": {
"type": "boolean"
},
"required_tag_group_name": {
"type": [
"string",
@ -158,6 +167,9 @@
"auto_close_based_on_last_post": {
"type": "boolean"
},
"allow_unlimited_owner_edits_on_first_post": {
"type": "boolean"
},
"group_permissions": {
"type": "array",
"items": [
@ -261,6 +273,7 @@
"available_groups",
"auto_close_hours",
"auto_close_based_on_last_post",
"allow_unlimited_owner_edits_on_first_post",
"group_permissions",
"email_in",
"email_in_allow_strangers",

View File

@ -46,6 +46,9 @@
"per_page": {
"type": "integer"
},
"top_tags": {
"type": "array"
},
"topics": {
"type": "array",
"items": [

View File

@ -134,6 +134,15 @@
"min_tags_from_required_group": {
"type": "integer"
},
"allowed_tags": {
"type": "array"
},
"allowed_tag_groups": {
"type": "array"
},
"allow_global_tags": {
"type": "boolean"
},
"required_tag_group_name": {
"type": [
"string",
@ -161,6 +170,9 @@
"auto_close_based_on_last_post": {
"type": "boolean"
},
"allow_unlimited_owner_edits_on_first_post": {
"type": "boolean"
},
"group_permissions": {
"type": "array",
"items": [
@ -264,6 +276,7 @@
"available_groups",
"auto_close_hours",
"auto_close_based_on_last_post",
"allow_unlimited_owner_edits_on_first_post",
"group_permissions",
"email_in",
"email_in_allow_strangers",

View File

@ -23,7 +23,7 @@ describe PostActionNotifier do
SiteSetting.editing_grace_period_max_diff = 1
post.update!(wiki: true)
user = post.user
owner = post.user
user2 = Fabricate(:user)
user3 = Fabricate(:user)
@ -42,7 +42,7 @@ describe PostActionNotifier do
edited_notification_type = Notification.types[:edited]
expect(Notification.exists?(
user: user,
user: owner,
notification_type: edited_notification_type
)).to eq(true)
@ -52,7 +52,7 @@ describe PostActionNotifier do
)).to eq(true)
expect do
post.revise(user, raw: "I made some changes to the wiki again!")
post.revise(owner, raw: "I made some changes to the wiki again!")
end.to change {
Notification.where(notification_type: edited_notification_type).count
}.by(1)
@ -69,7 +69,62 @@ describe PostActionNotifier do
}.by(1)
expect(Notification.where(
user: user,
user: owner,
notification_type: edited_notification_type
).count).to eq(2)
end
it 'notifies watching users of revision when topic category allow_unlimited_owner_edits_on_first_post and first post in topic is edited' do
SiteSetting.editing_grace_period_max_diff = 1
post.topic.update(category: Fabricate(:category, allow_unlimited_owner_edits_on_first_post: true))
owner = post.user
user2 = Fabricate(:user)
user3 = Fabricate(:user)
TopicUser.change(user2.id, post.topic,
notification_level: TopicUser.notification_levels[:watching]
)
TopicUser.change(user3.id, post.topic,
notification_level: TopicUser.notification_levels[:tracking]
)
expect do
post.revise(Fabricate(:user), raw: "I made some changes to the first post!")
end.to change { Notification.count }.by(2)
edited_notification_type = Notification.types[:edited]
expect(Notification.exists?(
user: owner,
notification_type: edited_notification_type
)).to eq(true)
expect(Notification.exists?(
user: user2,
notification_type: edited_notification_type
)).to eq(true)
expect do
post.revise(owner, raw: "I made some changes to the first post again!")
end.to change {
Notification.where(notification_type: edited_notification_type).count
}.by(1)
expect(Notification.where(
user: user2,
notification_type: edited_notification_type
).count).to eq(2)
expect do
post.revise(user2, raw: "I changed the first post totally")
end.to change {
Notification.where(notification_type: edited_notification_type).count
}.by(1)
expect(Notification.where(
user: owner,
notification_type: edited_notification_type
).count).to eq(2)
end