Skip to content

Commit 2ced927

Browse files
authored
Merge pull request #1773 from codidact/0valt/1731/thread-title-validation
Thread title validation
2 parents 7ab0270 + 154867f commit 2ced927

File tree

14 files changed

+150
-22
lines changed

14 files changed

+150
-22
lines changed

app/assets/javascripts/character_count.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,18 @@ $(() => {
6262
const $button = $form.find('input[type="submit"],.js-suggested-edit-approve');
6363
const $count = $counter.find('.js-character-count__count');
6464
const $icon = $counter.find('.js-character-count__icon');
65+
const $info = $counter.find('.js-character-count__info');
66+
const omitMarkdown = $tgt.data('markdown') === 'strip';
67+
68+
const fullCount = $tgt.val().trim().length;
69+
const count = omitMarkdown ?
70+
QPixel.MD.stripMarkdown($tgt.val().trim(), { removeLeadingQuote: true }).length :
71+
fullCount;
72+
73+
$info.toggleClass('hide', fullCount === count)
74+
.attr('title', 'Markdown will be stripped away and does not count towards the limit');
75+
$icon.toggleClass('hide', fullCount !== count);
6576

66-
const count = $tgt.val().length;
6777
const max = parseInt($counter.attr('data-max'), 10);
6878
const min = parseInt($counter.attr('data-min'), 10);
6979
const threshold = parseFloat($counter.attr('data-threshold'));
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
window.QPixel = window.QPixel || {};
2+
3+
QPixel.MD = {
4+
stripMarkdown: (content, options = {}) => {
5+
const stripped = content
6+
.replace(/(?:^#+ +|^-{3,}|^\[[^\]]+\]: ?.+$|^!\[[^\]]+\](?:\([^)]+\)|\[[^\]]+\])$|<[^>]+>)/g, '')
7+
.replace(/[*_~]+/g, '')
8+
.replace(/!?\[([^\]]+)\](?:\([^)]+\)|\[[^\]]+\])/g, '$1');
9+
10+
if (options.removeLeadingQuote ?? false) {
11+
return stripped.replace(/^>.+?$/g, '');
12+
}
13+
14+
return stripped;
15+
},
16+
};

app/controllers/comments_controller.rb

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ def create_thread
2626

2727
body = params[:body]
2828

29-
@comment_thread = CommentThread.new(title: title, post: @post)
29+
@comment_thread = CommentThread.new(title: helpers.strip_markdown(title, strip_leading_quote: true), post: @post)
3030
@comment = Comment.new(post: @post, content: body, user: current_user, comment_thread: @comment_thread)
3131

3232
pings = check_for_pings(@comment_thread, body)
@@ -208,7 +208,13 @@ def thread_rename
208208
return
209209
end
210210

211-
@comment_thread.update title: params[:title]
211+
title = helpers.strip_markdown(params[:title], strip_leading_quote: true)
212+
status = @comment_thread.update(title: title)
213+
214+
unless status
215+
flash[:danger] = I18n.t('comments.errors.rename_thread_generic')
216+
end
217+
212218
redirect_to comment_thread_path(@comment_thread.id)
213219
end
214220

app/helpers/application_helper.rb

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,16 +127,20 @@ def render_markdown(markdown)
127127
# This isn't a perfect way to strip out Markdown, so it should only be used for non-critical things like
128128
# page descriptions - things that will later be supplemented by the full formatted content.
129129
# @param markdown [String] The Markdown string to strip.
130+
# @param strip_leading_quote [Boolean] whether to remove the leading blockquote if present
130131
# @return [String] The plain-text equivalent.
131-
def strip_markdown(markdown)
132+
def strip_markdown(markdown, strip_leading_quote: false)
132133
# Remove block-level formatting: headers, hr, references, images, HTML tags
133134
markdown = markdown.gsub(/(?:^#+ +|^-{3,}|^\[[^\]]+\]: ?.+$|^!\[[^\]]+\](?:\([^)]+\)|\[[^\]]+\])$|<[^>]+>)/, '')
134135

135136
# Remove inline formatting: bold, italic, strike etc.
136137
markdown = markdown.gsub(/[*_~]+/, '')
137138

138139
# Remove links and inline images but replace them with their text/alt text.
139-
markdown.gsub(/!?\[([^\]]+)\](?:\([^)]+\)|\[[^\]]+\])/, '\1')
140+
markdown = markdown.gsub(/!?\[([^\]]+)\](?:\([^)]+\)|\[[^\]]+\])/, '\1')
141+
142+
# Optionally remove the leading blockquote
143+
strip_leading_quote ? markdown.gsub(/^>.+?$/, '') : markdown
140144
end
141145

142146
##

app/helpers/comments_helper.rb

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,7 @@ module CommentsHelper
44
# @param body [String] coment thread body
55
# @return [String] generated title
66
def generate_thread_title(body)
7-
body = strip_markdown(body)
8-
body = body.gsub(/^>.+?$/, '') # also remove leading blockquotes
7+
body = strip_markdown(body, strip_leading_quote: true)
98

109
if body.length > 100
1110
"#{body[0..100]}..."
@@ -184,6 +183,12 @@ def comment_rate_limited?(user, post, create_audit_log: true)
184183

185184
[true, message]
186185
end
186+
187+
# Get the maximum comment thread title length for the current community, with a maximum of 255.
188+
# @return [Integer]
189+
def maximum_thread_title_length
190+
[SiteSetting['MaxThreadTitleLength'] || 255, 255].min
191+
end
187192
end
188193

189194
# HTML sanitizer for use with comments.

app/models/comment_thread.rb

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ class CommentThread < ApplicationRecord
1111
scope :publicly_available, -> { where(deleted: false).where('reply_count > 0') }
1212
scope :archived, -> { where(archived: true) }
1313

14+
validate :maximum_title_length
15+
validates :title, presence: { message: I18n.t('comments.errors.title_presence') }
16+
1417
after_create :create_follower
1518

1619
def self.post_followed?(post, user)
@@ -21,10 +24,16 @@ def read_only?
2124
locked? || archived? || deleted?
2225
end
2326

27+
# Is a given user a follower of the thread?
28+
# @param user [User] user to check
29+
# @return [Boolean] check result
2430
def followed_by?(user)
2531
ThreadFollower.where(comment_thread: self, user: user).any?
2632
end
2733

34+
# Does a given user have access to the thread?
35+
# @param user [User] user to check access for
36+
# @return [Boolean] check result
2837
def can_access?(user)
2938
(!deleted? || user&.privilege?('flag_curate') || user&.post_privilege?('flag_curate', post)) &&
3039
post.can_access?(user)
@@ -53,6 +62,13 @@ def pingable
5362
ActiveRecord::Base.connection.execute(query).to_a.flatten
5463
end
5564

65+
def maximum_title_length
66+
max_len = SiteSetting['MaxThreadTitleLength'] || 255
67+
if title.length > [max_len, 255].min
68+
errors.add(:title, "can't be more than #{max_len} characters")
69+
end
70+
end
71+
5672
private
5773

5874
# Comment author and post author are automatically followed to the thread. Question author is NOT

app/views/comments/_new_thread_modal.html.erb

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<%#
2-
Start new comment thread dialog.
3-
%>
2+
"Start new comment thread dialog.
3+
"%>
44

55
<%
66
# TODO: make configurable
@@ -43,10 +43,17 @@
4343
class: 'form-element js-thread-title-field',
4444
data: { post: post.id,
4545
thread: '-1',
46-
character_count: ".js-character-count-thread-title" } %>
47-
<%= render 'shared/char_count', type: 'thread-title' %>
46+
character_count: '.js-character-count-thread-title',
47+
markdown: 'strip' } %>
48+
<%= render 'shared/char_count',
49+
type: 'thread-title',
50+
min: 1,
51+
max: maximum_thread_title_length %>
4852

49-
<%= submit_tag 'Create thread', class: 'button is-filled', id: "create_thread_button_#{post.id}", disabled: true %>
53+
<%= submit_tag 'Create thread',
54+
class: 'button is-filled',
55+
id: "create_thread_button_#{post.id}",
56+
disabled: true %>
5057
<% end %>
5158
</div>
5259
</div>
Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
<%#
2-
Helper for rendering comment thread rename action modal
2+
"Helper for rendering comment thread rename action modal
33
44
Variables:
55
thread : Comment thread to create the modal for
6-
%>
6+
"%>
77

88
<div class="modal is-with-backdrop is-small js--rename-thread-<%= thread.id %>">
99
<%= form_tag rename_comment_thread_path(thread.id), class: 'modal--container' do %>
@@ -12,18 +12,28 @@
1212
class="button is-close-button modal--header-button"
1313
data-modal=".js--rename-thread-<%= thread.id %>">&times;
1414
</button>
15-
Rename...
15+
Rename thread
1616
</div>
1717
<div class="modal--body">
1818
<%= label_tag :title, 'Title', class: 'form-element' %>
1919
<%= text_field_tag :title,
2020
thread.title,
2121
class: 'form-element js-thread-title-field',
22-
data: { post: thread.post.id, thread: thread.id } %>
22+
data: { post: thread.post.id,
23+
thread: thread.id,
24+
character_count: ".js-character-count-thread-title-#{thread.id}",
25+
markdown: 'strip' } %>
26+
<%= render 'shared/char_count',
27+
type: "thread-title-#{thread.id}",
28+
cur: thread.title.length,
29+
min: 1,
30+
max: maximum_thread_title_length %>
2331
</div>
2432
<div class="modal--footer">
2533
<%= submit_tag 'Update thread', class: 'button is-muted is-filled' %>
26-
<button type="button" class="button is-muted" data-modal=".js--rename-thread-<%= thread.id %>">Cancel</button>
34+
<button type="button"
35+
class="button is-muted"
36+
data-modal=".js--rename-thread-<%= thread.id %>">Cancel</button>
2737
</div>
2838
<% end %>
2939
</div>

app/views/shared/_char_count.html.erb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,9 @@
2121
data-max="<%= max %>"
2222
data-min="<%= min %>"
2323
data-threshold="<%= threshold %>">
24+
<i class="fas fa-info-circle hide js-character-count__info"></i>
2425
<i class="fas fa-ellipsis-h js-character-count__icon"></i>
2526
<span class="js-character-count__count">
2627
<%= cur %> / <%= cur < min ? min : max %>
2728
</span>
28-
</span>
29+
</span>

config/locales/strings/en.comments.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,10 @@ en:
2929
Something went wrong when trying to delete the comment.
3030
undelete_comment_server_error: >
3131
Something went wrong when trying to undelete the comment.
32+
rename_thread_generic: >
33+
Failed to rename the thread.
34+
title_presence: >
35+
can't be empty after Markdown is removed.
3236
labels:
3337
create_new_thread: >
3438
Start new comment thread

0 commit comments

Comments
 (0)