FEATURE: Disable minimum post length check when in PM with non human users.

https://meta.discourse.org/t/discourse-narrative-bot-beta-feedback/58621/65?u=tgxworld
This commit is contained in:
Guo Xiang Tan 2017-04-27 11:53:53 +08:00
parent b755279cf0
commit 59b906ab0d
11 changed files with 138 additions and 24 deletions

View File

@ -284,13 +284,14 @@ const Composer = RestModel.extend({
@property minimumTitleLength
**/
minimumTitleLength: function() {
if (this.get('privateMessage')) {
@computed('privateMessage')
minimumTitleLength(privateMessage) {
if (privateMessage) {
return this.siteSettings.min_private_message_title_length;
} else {
return this.siteSettings.min_topic_title_length;
}
}.property('privateMessage'),
},
@computed('minimumPostLength', 'replyLength', 'canEditTopicFeaturedLink')
missingReplyCharacters(minimumPostLength, replyLength, canEditTopicFeaturedLink) {
@ -304,16 +305,19 @@ const Composer = RestModel.extend({
@property minimumPostLength
**/
minimumPostLength: function() {
if( this.get('privateMessage') ) {
@computed('privateMessage', 'topicFirstPost', 'topic.pm_with_non_human_user')
minimumPostLength(privateMessage, topicFirstPost, pmWithNonHumanUser) {
if (pmWithNonHumanUser) {
return 1;
} else if (privateMessage) {
return this.siteSettings.min_private_message_post_length;
} else if (this.get('topicFirstPost')) {
} else if (topicFirstPost) {
// first post (topic body)
return this.siteSettings.min_first_post_length;
} else {
return this.siteSettings.min_post_length;
}
}.property('privateMessage', 'topicFirstPost'),
},
/**
Computes the length of the title minus non-significant whitespaces

View File

@ -1187,6 +1187,15 @@ SQL
private_topic
end
def pm_with_non_human_user?
Topic.private_messages
.joins("LEFT JOIN topic_allowed_groups ON topics.id = topic_allowed_groups.topic_id")
.where("topic_allowed_groups.topic_id IS NULL")
.where("topics.id = ?", self.id)
.where("(SELECT COUNT(*) FROM topic_allowed_users WHERE topic_allowed_users.topic_id = ? AND topic_allowed_users.user_id > 0) = 1", self.id)
.exists?
end
private
def update_category_topic_count_by(num)

View File

@ -34,7 +34,8 @@ class TopicViewSerializer < ApplicationSerializer
:word_count,
:deleted_at,
:pending_posts_count,
:user_id
:user_id,
:pm_with_non_human_user?
attributes :draft,
:draft_key,
@ -71,7 +72,7 @@ class TopicViewSerializer < ApplicationSerializer
last_poster: BasicUserSerializer.new(topic.last_poster, scope: scope, root: false)
}
if object.topic.private_message?
if private_message?(topic)
allowed_user_ids = Set.new
result[:allowed_groups] = object.topic.allowed_groups.map do |group|
@ -129,7 +130,7 @@ class TopicViewSerializer < ApplicationSerializer
end
def is_warning
object.topic.private_message? && object.topic.subtype == TopicSubtype.moderator_warning
private_message?(object.topic) && object.topic.subtype == TopicSubtype.moderator_warning
end
def include_is_warning?
@ -149,7 +150,7 @@ class TopicViewSerializer < ApplicationSerializer
end
def include_message_archived?
object.topic.private_message?
private_message?(object.topic)
end
def message_archived
@ -274,4 +275,14 @@ class TopicViewSerializer < ApplicationSerializer
gsub_emoji_to_unicode(object.topic.title)
end
def include_pm_with_non_human_user?
private_message?(object.topic)
end
private
def private_message?(topic)
@private_message ||= topic.private_message?
end
end

View File

@ -30,7 +30,7 @@ class Validators::PostValidator < ActiveModel::Validator
end
def post_body_validator(post)
return if options[:skip_post_body]
return if options[:skip_post_body] || post.topic&.pm_with_non_human_user?
stripped_length(post)
raw_quality(post)
end

View File

@ -5,13 +5,41 @@ describe Validators::PostValidator do
let(:post) { build(:post) }
let(:validator) { Validators::PostValidator.new({}) }
context "when empty raw can bypass post body validation" do
let(:validator) { Validators::PostValidator.new(skip_post_body: true) }
it "should be allowed for empty raw based on site setting" do
context "#post_body_validator" do
it 'should not allow a post with an empty raw' do
post.raw = ""
validator.post_body_validator(post)
expect(post.errors).to be_empty
expect(post.errors).to_not be_empty
end
context "when empty raw can bypass validation" do
let(:validator) { Validators::PostValidator.new(skip_post_body: true) }
it "should be allowed for empty raw based on site setting" do
post.raw = ""
validator.post_body_validator(post)
expect(post.errors).to be_empty
end
end
describe "when post's topic is a PM between a human and a non human user" do
let(:robot) { Fabricate(:user, id: -3) }
let(:user) { Fabricate(:user) }
let(:topic) do
Fabricate(:private_message_topic, topic_allowed_users: [
Fabricate.build(:topic_allowed_user, user: robot),
Fabricate.build(:topic_allowed_user, user: user)
])
end
it 'should allow a post with an empty raw' do
post = Fabricate.build(:post, topic: topic)
post.raw = ""
validator.post_body_validator(post)
expect(post.errors).to be_empty
end
end
end

View File

@ -126,8 +126,8 @@ Fabricator(:private_message_post, from: :post) do
created_at: attrs[:created_at],
subtype: TopicSubtype.user_to_user,
topic_allowed_users: [
Fabricate.build(:topic_allowed_user, user_id: attrs[:user].id),
Fabricate.build(:topic_allowed_user, user_id: Fabricate(:user).id)
Fabricate.build(:topic_allowed_user, user: attrs[:user]),
Fabricate.build(:topic_allowed_user, user: Fabricate(:user))
]
)
end

View File

@ -0,0 +1,4 @@
Fabricator(:topic_allowed_group) do
topic
group
end

View File

@ -0,0 +1,3 @@
Fabricator(:topic_allowed_user) do
user
end

View File

@ -12,9 +12,6 @@ Fabricator(:closed_topic, from: :topic) do
closed true
end
Fabricator(:topic_allowed_user) do
end
Fabricator(:banner_topic, from: :topic) do
archetype Archetype.banner
end
@ -25,7 +22,7 @@ Fabricator(:private_message_topic, from: :topic) do
title { sequence(:title) { |i| "This is a private message #{i}" } }
archetype "private_message"
topic_allowed_users{|t| [
Fabricate.build(:topic_allowed_user, user_id: t[:user].id),
Fabricate.build(:topic_allowed_user, user_id: Fabricate(:coding_horror).id)
Fabricate.build(:topic_allowed_user, user: t[:user]),
Fabricate.build(:topic_allowed_user, user: Fabricate(:coding_horror))
]}
end

View File

@ -1867,4 +1867,54 @@ describe Topic do
expect(Topic.with_no_response_total).to eq(1)
end
end
describe '#pm_with_non_human_user?' do
let(:robot) { Fabricate(:user, id: -3) }
let(:user) { Fabricate(:user) }
let(:topic) do
Fabricate(:private_message_topic, topic_allowed_users: [
Fabricate.build(:topic_allowed_user, user: robot),
Fabricate.build(:topic_allowed_user, user: user)
])
end
describe 'when PM is between a human and a non human user' do
it 'should return true' do
expect(topic.pm_with_non_human_user?).to be(true)
end
end
describe 'when PM contains 2 human users and a non human user' do
it 'should return false' do
Fabricate(:topic_allowed_user, topic: topic, user: Fabricate(:user))
expect(topic.pm_with_non_human_user?).to be(false)
end
end
describe 'when PM only contains a user' do
it 'should return true' do
topic.topic_allowed_users.first.destroy!
expect(topic.reload.pm_with_non_human_user?).to be(true)
end
end
describe 'when PM contains a group' do
it 'should return false' do
Fabricate(:topic_allowed_group, topic: topic)
expect(topic.pm_with_non_human_user?).to be(false)
end
end
describe 'when topic is not a PM' do
it 'should return false' do
topic.convert_to_public_topic(Fabricate(:admin))
expect(topic.pm_with_non_human_user?).to be(false)
end
end
end
end

View File

@ -160,6 +160,14 @@ test("Title length for private messages", function() {
ok(composer.get('titleLengthValid'), "in the range is okay");
});
test("Post length for private messages with non human users", function() {
const composer = createComposer({
topic: Ember.Object.create({ pm_with_non_human_user: true })
});
equal(composer.get('minimumPostLength'), 1);
});
test('editingFirstPost', function() {
const composer = createComposer();
ok(!composer.get('editingFirstPost'), "it's false by default");