Skip to content

Commit ea496be

Browse files
OaphiOaphi
authored
Merge pull request #1713 from codidact/0valt/1708/general
Use correct host when rendering comment pings
2 parents d0be299 + d510a7d commit ea496be

4 files changed

Lines changed: 63 additions & 41 deletions

File tree

‎app/helpers/comments_helper.rb‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ def pinged_users(content)
4747
# @param content [String] content to convert ping-strings for
4848
# @param pingable [Array<Integer>, nil] A list of user IDs. Any user ID not present will be displayed as 'unpingable'.
4949
# @return [ActiveSupport::SafeBuffer]
50-
def render_pings(content, pingable: nil)
50+
def render_pings(content, pingable: nil, host: nil)
5151
users = pinged_users(content)
5252

5353
content.gsub(/@#(\d+)/) do |ping|
@@ -57,8 +57,10 @@ def render_pings(content, pingable: nil)
5757
else
5858
was_pung = pingable.present? && pingable.include?(user.id)
5959
classes = "ping #{'me' if user.same_as?(current_user)} #{'unpingable' unless was_pung}"
60-
user_link user, class: classes, dir: 'ltr',
61-
title: was_pung ? '' : I18n.t('comments.warnings.unrelated_user_not_pinged')
60+
user_link(user, { host: host },
61+
class: classes,
62+
dir: 'ltr',
63+
title: was_pung ? '' : I18n.t('comments.warnings.unrelated_user_not_pinged'))
6264
end
6365
end.html_safe
6466
end

‎app/views/comments/_comment.html.erb‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,8 @@
7777
</div>
7878
</div>
7979
<div class="comment--body" dir="ltr">
80-
<%= render_pings(raw(sanitize(render_comment_helpers(render_markdown(comment.content)), scrubber: CommentScrubber.new)),
81-
pingable: pingable) %>
80+
<% rendered = render_comment_helpers(render_markdown(comment.content)) %>
81+
<% sanitized = sanitize(rendered, scrubber: CommentScrubber.new) %>
82+
<%= render_pings(raw(sanitized), pingable: pingable, host: comment.community.host) %>
8283
</div>
8384
</div>

‎app/views/layouts/application.html.erb‎

Lines changed: 37 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,50 +1,51 @@
11
<!DOCTYPE html>
22
<html lang="en" dir="ltr">
3-
<head>
4-
<%= render 'layouts/head' %>
5-
</head>
6-
<body class="<%= Rails.env.development? ? 'development' : '' %>"
3+
<head>
4+
<%= render 'layouts/head' %>
5+
</head>
6+
<body class="<%= Rails.env.development? ? 'development' : '' %>"
77
data-user-id="<%= user_signed_in? ? current_user.id : 'none' %>"
88
data-mathjax="<%= SiteSetting['MathJaxEnabled'] %>">
9-
<%= render 'layouts/header' %>
10-
11-
<main class="container">
12-
<div class="grid">
13-
<div class="grid--cell is-8-lg is-12">
14-
<div class="has-padding-4">
15-
<% {notice: :info, alert: :danger, danger: :danger, success: :success, info: :info, warning: :warning}.each do |mt, cc| %>
16-
<% if flash[mt].present? %>
17-
<div class="notice is-<%= cc.to_s %>">
18-
<%= flash[mt] %>
19-
</div>
9+
<%= render 'layouts/header' %>
10+
11+
<main class="container">
12+
<div class="grid">
13+
<div class="grid--cell is-8-lg is-12">
14+
<div class="has-padding-4">
15+
<% {notice: :info, alert: :danger, danger: :danger, success: :success, info: :info, warning: :warning}.each do |mt, cc| %>
16+
<% if flash[mt].present? %>
17+
<div class="notice is-<%= cc.to_s %>">
18+
<%= flash[mt] %>
19+
</div>
20+
<% end %>
2021
<% end %>
21-
<% end %>
22-
2322

24-
<% if @first_visit_notice %>
25-
<% notice = SiteSetting['FirstVisitGuidance'] %>
26-
<% if notice.present? %>
27-
<div class="notice js-first-visit-notice" id="fvn">
28-
<button type="button" class="button is-close-button" data-dismiss="#fvn" aria-label="Close" id="fvn-dismiss">
29-
<span aria-hidden="true">&times;</span>
30-
</button>
31-
<%= raw(sanitize(notice, scrubber: scrubber)) %>
32-
</div>
23+
<% if @first_visit_notice %>
24+
<% notice = SiteSetting['FirstVisitGuidance'] %>
25+
<% if notice.present? %>
26+
<div class="notice js-first-visit-notice" id="fvn">
27+
<button type="button" class="button is-close-button" data-dismiss="#fvn" aria-label="Close" id="fvn-dismiss">
28+
<span aria-hidden="true">&times;</span>
29+
</button>
30+
<%= raw(sanitize(notice, scrubber: scrubber)) %>
31+
</div>
32+
<% end %>
3333
<% end %>
34-
<% end %>
3534

36-
<%= yield %>
35+
<%= yield %>
36+
</div>
3737
</div>
38-
</div>
3938

40-
<%= render 'layouts/sidebar' %>
41-
</div>
42-
</main>
39+
<%= render 'layouts/sidebar' %>
40+
</div>
41+
</main>
4342

44-
<%= render 'layouts/footer' %>
43+
<%= render 'layouts/footer' %>
4544

46-
<%= render 'layouts/matomo' %>
45+
<%= render 'layouts/matomo' %>
4746

48-
<script src="https://7zb04r9ckbwg.statuspage.io/embed/script.js"></script>
49-
</body>
47+
<% if Rails.env.production? %>
48+
<script src="https://7zb04r9ckbwg.statuspage.io/embed/script.js"></script>
49+
<% end %>
50+
</body>
5051
</html>

‎test/helpers/comments_helper_test.rb‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
require 'test_helper'
22

33
class CommentsHelperTest < ActionView::TestCase
4+
include ApplicationHelper
5+
include UsersHelper
46
include Devise::Test::ControllerHelpers
57

68
test '[help center] and [help] substitution' do
@@ -108,4 +110,20 @@ class CommentsHelperTest < ActionView::TestCase
108110
assert_nil limit_message
109111
end
110112
end
113+
114+
test 'render_pings should correctly render comment pings' do
115+
std = users(:standard_user)
116+
mod = users(:moderator)
117+
118+
sign_in std
119+
120+
host = RequestContext.community.host
121+
122+
rendered = render_pings("this is not correct, @##{std.id}. @##{mod.id}'s answer is better. @#123, thoughs?",
123+
host: host)
124+
125+
assert Regexp.new("http:\\/\\/#{Regexp.escape(host)}\\/users\\/#{std.id}").match?(rendered)
126+
assert Regexp.new("http:\\/\\/#{Regexp.escape(host)}\\/users\\/#{mod.id}").match?(rendered)
127+
assert(/@#123/.match?(rendered))
128+
end
111129
end

0 commit comments

Comments
 (0)