Hashtags parser fixes and improvements

CHANGELOG.md

@@ -59,6 +59,7 @@ end
 
 **Fixed**:
 
+- **decidim-core**: Hashtags with unicode characters are now parsed correctly [\#4473](https://github.com/decidim/decidim/pull/4473)
 - **decidim-conferences**: Check participatory spaces manifest exists when relating conferences to other spaces [\#4446](https://github.com/decidim/decidim/pull/4446)
 - **decidim-proposals**: Allow admins to edit proposals even if creation is not enabled [\#4390](https://github.com/decidim/decidim/pull/4390)
 - **decidim-core**: Fix events for polymorphic authors [\#4387](https://github.com/decidim/decidim/pull/4387)

decidim-core/lib/decidim/content_parsers/hashtag_parser.rb

@@ -2,34 +2,59 @@
 
 module Decidim
   module ContentParsers
-    # A parser that searches user mentions in content.
+    # A parser that searches hashtags in content.
     #
-    # A word starting with `#` will be considered as a possible hashtagging if
-    # they only contains letters, numbers or underscores. If the `#` is
+    # A word starting with `#` will be considered as a hashtag if
+    # it only contains letters, numbers or underscores. If the `#` is
     # followed with an underscore, then it is not considered.
     #
     # @see BaseParser Examples of how to use a content parser
     class HashtagParser < BaseParser
+      # Class used as a container for metadata
+      #
+      # @!attribute hashtags
+      #   @return [Array] an array of Decidim::Hashtag mentioned in content
       Metadata = Struct.new(:hashtags)
 
-      # Replaces found hashtags matching a name of an existing
-      # hashtag with a global id.
+      # Matches a hashtag if it starts with a letter or number
+      # and only contains letters, numbers or underscores.
+      HASHTAG_REGEX = /\B#([[:alnum:]](?:[[:alnum:]]|_)*)\b/i
+
+      # Replaces hashtags name with new or existing hashtags models global ids.
       #
-      # @return [String] the content with the hashtags replaced by a global id
+      # @return [String] the content with the hashtags replaced by global ids
       def rewrite
-        content.gsub(Decidim::Hashtag::HASHTAG_REGEX) do |match|
-          if (hashtag = Decidim::Hashtag.find_or_create_by(organization: context[:current_organization], name: Regexp.last_match[2].downcase))
-            Regexp.last_match[1] + hashtag.to_global_id.to_s
-          else
-            match
-          end
+        content.gsub(HASHTAG_REGEX) do |match|
+          hashtags[match[1..-1]].to_global_id.to_s
         end
       end
 
       def metadata
-        Metadata.new(
-          Decidim::Hashtag.where(organization: context[:current_organization], name: content.scan(Decidim::Hashtag::HASHTAG_REGEX).flatten)
-        )
+        Metadata.new(content_hashtags.map { |content_hashtag| hashtags[content_hashtag] })
+      end
+
+      private
+
+      def hashtags
+        @hashtags ||= Hash.new do |hash, name|
+          hash[name] = Decidim::Hashtag.create(organization: current_organization, name: name.downcase)
+        end .merge(Hash[
+          existing_hashtags.map do |hashtag|
+            [hashtag.name, hashtag]
+          end
+        ])
+      end
+
+      def existing_hashtags
+        @existing_hashtags ||= Decidim::Hashtag.where(organization: current_organization, name: content_hashtags)
+      end
+
+      def content_hashtags
+        @content_hashtags ||= content.scan(HASHTAG_REGEX).flatten.uniq
+      end
+
+      def current_organization
+        @current_organization ||= context[:current_organization]
       end
     end
   end

decidim-core/lib/decidim/content_parsers/user_parser.rb

@@ -16,9 +16,9 @@ class UserParser < BaseParser
       #   @return [Array] an array of Decidim::User mentioned in content
       Metadata = Struct.new(:users)
 
-      # Matches a nickname if they start with a letter or number
+      # Matches a nickname if it starts with a letter or number
       # and only contains letters, numbers or underscores.
-      MENTION_REGEX = /(^|\s)@([a-zA-Z0-9]\w*)/
+      MENTION_REGEX = /\B@([a-zA-Z0-9]\w*)\b/
 
       # Replaces found mentions matching a nickname of an existing
       # user in the current organization with a global id. Other
@@ -28,19 +28,35 @@ class UserParser < BaseParser
       # @return [String] the content with the valid mentions replaced by a global id
       def rewrite
         content.gsub(MENTION_REGEX) do |match|
-          if (user = Decidim::User.find_by(nickname: Regexp.last_match[2], organization: context[:current_organization]))
-            Regexp.last_match[1] + user.to_global_id.to_s
-          else
-            match
-          end
+          users[match[1..-1]]&.to_global_id&.to_s || match
         end
       end
 
       # (see BaseParser#metadata)
       def metadata
-        Metadata.new(
-          Decidim::User.where(nickname: content.scan(MENTION_REGEX).flatten, organization: context[:current_organization])
-        )
+        Metadata.new(existing_users)
+      end
+
+      private
+
+      def users
+        @users ||= Hash[
+          existing_users.map do |user|
+            [user.nickname, user]
+          end
+        ]
+      end
+
+      def existing_users
+        @existing_users ||= Decidim::User.where(organization: current_organization, nickname: content_nicknames)
+      end
+
+      def content_nicknames
+        @content_nicknames ||= content.scan(MENTION_REGEX).flatten.uniq
+      end
+
+      def current_organization
+        @current_organization ||= context[:current_organization]
       end
     end
   end

decidim-core/lib/decidim/hashtag.rb

@@ -9,7 +9,5 @@ class Hashtag < ApplicationRecord
 
     validates :name, presence: true
     validates :name, uniqueness: { scope: [:decidim_organization_id] }
-
-    HASHTAG_REGEX = /(^|\s)#([a-zA-Z0-9]\w*)/i
   end
 end

decidim-core/spec/content_parsers/decidim/hashtag_parser_spec.rb

@@ -5,35 +5,65 @@
 module Decidim
   describe ContentParsers::HashtagParser do
     let(:organization) { create(:organization) }
-    let(:hashtag) { create(:hashtag, organization: organization) }
+    let(:hashtag) { create(:hashtag, organization: organization, name: name) }
+    let(:name) { "a_hashtag" }
     let(:context) { { current_organization: organization } }
     let(:parser) { described_class.new(content, context) }
 
-    context "when hashtagging a hashtag present" do
-      let(:content) { "This text contains a hashtag present on DB: ##{hashtag.name}" }
+    let(:content) { "This text contains a hashtag present on DB: ##{hashtag.name}" }
+    let(:parsed_content) { "This text contains a hashtag present on DB: #{hashtag.to_global_id}" }
+    let(:metadata_hashtags) { [hashtag] }
 
+    shared_examples "find and stores the hashtags references" do
       it "rewrites the hashtag" do
-        expect(parser.rewrite).to eq("This text contains a hashtag present on DB: #{hashtag.to_global_id}")
+        expect(parser.rewrite).to eq(parsed_content)
       end
 
       it "returns the correct metadata" do
         expect(parser.metadata).to be_a(Decidim::ContentParsers::HashtagParser::Metadata)
-        expect(parser.metadata.hashtags).to eq([hashtag])
+        expect(parser.metadata.hashtags).to eq(metadata_hashtags)
       end
     end
 
-    context "when hashtagging multiple hashtag presents" do
+    it_behaves_like "find and stores the hashtags references"
+
+    context "when contents has a new hashtag" do
+      let(:hashtag) { Decidim::Hashtag.find_by(organization: organization, name: name) }
+      let(:content) { "This text contains a hashtag not present on DB: ##{name}" }
+      let(:parsed_content) { "This text contains a hashtag not present on DB: #{hashtag.to_global_id}" }
+
+      it_behaves_like "find and stores the hashtags references"
+    end
+
+    context "when hashtagging multiple hashtags" do
+      let(:new_hashtag) { Decidim::Hashtag.find_by(organization: organization, name: "a_new_one") }
       let(:hashtag2) { create(:hashtag, organization: organization) }
-      let(:content) { "This text contains multiple hashtag presents: ##{hashtag.name} and ##{hashtag2.name}" }
+      let(:content) { "This text contains multiple hashtag presents: #a_new_one, ##{hashtag.name} and ##{hashtag2.name}" }
+      let(:parsed_content) { "This text contains multiple hashtag presents: #{new_hashtag.to_global_id}, #{hashtag.to_global_id} and #{hashtag2.to_global_id}" }
+      let(:metadata_hashtags) { [new_hashtag, hashtag, hashtag2] }
 
-      it "rewrites all hashtags" do
-        expect(parser.rewrite).to include("This text contains multiple hashtag presents: #{hashtag.to_global_id} and #{hashtag2.to_global_id}")
-      end
+      it_behaves_like "find and stores the hashtags references"
+    end
 
-      it "returns the correct metadata" do
-        expect(parser.metadata).to be_a(Decidim::ContentParsers::HashtagParser::Metadata)
-        expect(parser.metadata.hashtags).to eq([hashtag, hashtag2])
-      end
+    context "when hashtags name contains unicode characters" do
+      let(:hashtag) { create(:hashtag, organization: organization, name: "acción_mutante") }
+
+      it_behaves_like "find and stores the hashtags references"
+    end
+
+    context "when content contains the same new hashtag twice" do
+      let(:hashtag) { Decidim::Hashtag.find_by(organization: organization, name: name) }
+      let(:content) { "This text contains a hashtag present on DB twice: ##{name} and ##{name}" }
+      let(:parsed_content) { "This text contains a hashtag present on DB twice: #{hashtag.to_global_id} and #{hashtag.to_global_id}" }
+
+      it_behaves_like "find and stores the hashtags references"
+    end
+
+    context "when content contains non-hash characters next to the hashtag name" do
+      let(:content) { "You can't add some characters to hashtags: ##{hashtag.name}+extra" }
+      let(:parsed_content) { "You can't add some characters to hashtags: #{hashtag.to_global_id}+extra" }
+
+      it_behaves_like "find and stores the hashtags references"
     end
   end
 end