Don't autoload PeopleHelper during initialization

this is for future rails versions with zeitwerk autoloader where this
isn't allowed anymore

5 files changed, 26 insertions, 33 deletions

diff --git a/app/helpers/people_helper.rb b/app/helpers/people_helper.rb
index 40c79bb43..87c622503 100644
--- a/app/helpers/people_helper.rb
+++ b/app/helpers/people_helper.rb
@@ -27,10 +27,10 @@ module PeopleHelper
 
   def person_link(person, opts={})
     css_class = person_link_class(person, opts[:class])
-    remote_or_hovercard_link = Rails.application.routes.url_helpers.person_path(person).html_safe
-    "<a data-hovercard='#{remote_or_hovercard_link}' href='#{remote_or_hovercard_link}' class='#{css_class}'>"\
-      "#{html_escape_once(opts[:display_name] || person.name)}</a>"\
-      .html_safe
+    remote_or_hovercard_link = person_path(person)
+    tag.a('data-hovercard': remote_or_hovercard_link, href: remote_or_hovercard_link, class: css_class) do
+      opts[:display_name] || person.name
+    end
   end
 
   def person_image_tag(person, size = :thumb_small)
@@ -44,15 +44,12 @@ module PeopleHelper
     if opts[:to] == :photos
       link_to person_image_tag(person, opts[:size]), person_photos_path(person)
     else
-      css_class = person_link_class(person, opts[:class])
-      remote_or_hovercard_link = Rails.application.routes.url_helpers.person_path(person).html_safe
-      "<a href='#{remote_or_hovercard_link}' class='#{css_class}' #{('target=' + opts[:target]) if opts[:target]}>
-      #{person_image_tag(person, opts[:size])}
-      </a>".html_safe
+      tag.a(href: person_path(person), class: person_link_class(person, opts[:class])) do
+        person_image_tag(person, opts[:size])
+      end
     end
   end
 
-  # Rails.application.routes.url_helpers is needed since this is indirectly called from a model
   def local_or_remote_person_path(person, opts={})
     opts.merge!(:protocol => AppConfig.pod_uri.scheme, :host => AppConfig.pod_uri.authority)
     absolute = opts.delete(:absolute)
@@ -61,19 +58,11 @@ module PeopleHelper
       username = person.username
       unless username.include?('.')
         opts.merge!(:username => username)
-        if absolute
-          return Rails.application.routes.url_helpers.user_profile_url(opts)
-        else
-          return Rails.application.routes.url_helpers.user_profile_path(opts)
-        end
+        return absolute ? user_profile_url(opts) : user_profile_path(opts)
       end
     end
 
-    if absolute
-      return Rails.application.routes.url_helpers.person_url(person, opts)
-    else
-      return Rails.application.routes.url_helpers.person_path(person, opts)
-    end
+    absolute ? person_url(person, opts) : person_path(person, opts)
   end
 
   private
diff --git a/lib/diaspora/mentionable.rb b/lib/diaspora/mentionable.rb
index 59f2da8f0..2b447dc5f 100644
--- a/lib/diaspora/mentionable.rb
+++ b/lib/diaspora/mentionable.rb
@@ -1,4 +1,3 @@
-
 # frozen_string_literal: true
 
 module Diaspora::Mentionable
@@ -93,7 +92,7 @@ module Diaspora::Mentionable
 
   # inline module for namespacing
   module MentionsInternal
-    extend ::PeopleHelper
+    extend ERB::Util
 
     # output a formatted mention link as defined by the given arguments.
     # if the display name is blank, falls back to the person's name.
@@ -105,10 +104,15 @@ module Diaspora::Mentionable
     def self.mention_link(person, display_name, diaspora_id, opts)
       return display_name || diaspora_id unless person.present?
 
+      display_name ||= person.name
       if opts[:plain_text]
-        display_name || person.name
+        display_name
       else
-        person_link(person, class: PERSON_HREF_CLASS, display_name: display_name)
+        # rubocop:disable Rails/OutputSafety
+        remote_or_hovercard_link = Rails.application.routes.url_helpers.person_path(person).html_safe
+        "<a data-hovercard=\"#{remote_or_hovercard_link}\" href=\"#{remote_or_hovercard_link}\" " \
+          "class=\"#{PERSON_HREF_CLASS}\">#{html_escape_once(display_name)}</a>".html_safe
+        # rubocop:enable Rails/OutputSafety
       end
     end
 
@@ -121,7 +125,7 @@ module Diaspora::Mentionable
     def self.profile_link(person, display_name, diaspora_id)
       return display_name || diaspora_id unless person.present?
 
-      "[#{display_name || person.name}](#{local_or_remote_person_path(person)})"
+      "[#{display_name || person.name}](#{Rails.application.routes.url_helpers.person_path(person)})"
     end
   end
 end
diff --git a/spec/helpers/people_helper_spec.rb b/spec/helpers/people_helper_spec.rb
index 4b4140918..1a7d15f85 100644
--- a/spec/helpers/people_helper_spec.rb
+++ b/spec/helpers/people_helper_spec.rb
@@ -75,7 +75,7 @@ describe PeopleHelper, :type => :helper do
     end
 
     it 'links by id for a local user' do
-      expect(person_link(@user.person)).to include "href='#{person_path(@user.person)}'"
+      expect(person_link(@user.person)).to include "href=\"#{person_path(@user.person)}\""
     end
 
     it "recognizes the 'display_name' option" do
diff --git a/spec/lib/diaspora/mentionable_spec.rb b/spec/lib/diaspora/mentionable_spec.rb
index 25926d898..78e590b75 100644
--- a/spec/lib/diaspora/mentionable_spec.rb
+++ b/spec/lib/diaspora/mentionable_spec.rb
@@ -1,8 +1,6 @@
 # frozen_string_literal: true
 
 describe Diaspora::Mentionable do
-  include PeopleHelper
-
   let(:people) { [alice, bob, eve].map(&:person) }
   let(:names) { %w(Alice\ A Bob\ B "Eve>\ E) }
 
@@ -41,7 +39,9 @@ STR
   end
 
   describe ".format" do
-    context "html output" do
+    context "html output", type: :helper do
+      include PeopleHelper
+
       it "adds the links to the formatted message" do
         fmt_msg = Diaspora::Mentionable.format(test_text_with_names, people)
 
@@ -75,7 +75,7 @@ STR
         fmt_msg = Diaspora::Mentionable.format(test_txt, people)
 
         expect(fmt_msg).not_to include(name)
-        expect(fmt_msg).to include("&gt;", "&lt;", "&#39;") # ">", "<", "'"
+        expect(fmt_msg).to include("&lt;/a&gt;&lt;script&gt;alert(&#39;h&#39;)&lt;/script&gt;")
       end
     end
 
@@ -184,7 +184,7 @@ STR
         user_a.aspects.where(name: "generic").first.contacts.map(&:person_id)
       )
 
-      expect(txt).to include("@[user C](#{local_or_remote_person_path(user_c.person)}")
+      expect(txt).to include("@[user C](#{Rails.application.routes.url_helpers.person_path(user_c.person)}")
       expect(txt).not_to include("href")
       expect(txt).not_to include(mention)
     end
diff --git a/spec/lib/diaspora/message_renderer_spec.rb b/spec/lib/diaspora/message_renderer_spec.rb
index 248563ed8..11bffbbcb 100644
--- a/spec/lib/diaspora/message_renderer_spec.rb
+++ b/spec/lib/diaspora/message_renderer_spec.rb
@@ -81,7 +81,7 @@ describe Diaspora::MessageRenderer do
         it 'makes plain links for people not in the post aspects' do
           message = message("@{Bob; #{bob.person.diaspora_handle}}", link_all_mentions: true).html
           expect(message).to_not include 'hovercard'
-          expect(message).to include '/u/bob'
+          expect(message).to include "/people/#{bob.person.guid}"
         end
 
         it "makes no hovercards if they're disabled" do
@@ -91,7 +91,7 @@ describe Diaspora::MessageRenderer do
             disable_hovercards: true
           ).html
           expect(message).to_not include 'hovercard'
-          expect(message).to include '/u/bob'
+          expect(message).to include "/people/#{bob.person.guid}"
         end
       end
     end