Project

General

Profile

Actions
Copy link

Defect #39553

closed

Mention notification is not sent (MENTION_PATTERN / LINKS_RE inconsistency)

Added by Thomas Löber 11 months ago. Updated 11 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Marius BĂLTEANU
Category:
Email notifications
Target version:
5.0.7
Start date:
Due date:
% Done:

0%

Estimated time:
Resolution:
Fixed
Affected version:
5.0.6

Description

Hi,

When I add an issue comment

Hello @login!

where login is a valid login and there is no newline at the end of the comment, the mention is highlighted in the issue history, but no notification is sent to the user.

This is because both, Redmine::Acts::Mentionable::InstanceMethods::MENTION_PATTERN and ApplicationHelper::LINKS_RE, require a character after the exclamation mark, but only LINKS_RE (used in ApplicationHelper#parse_redmine_links) finds additional characters, namely the </p>, which is added earlier when the comment is converted to HTML.

MENTION_PATTERN should handle the case that [[:punct:]] is the at end of the string.

Thanks,
Thomas

Actions
Copy link
 #1

Updated by Marius BĂLTEANU 11 months ago

  • Status changed from New to Confirmed
  • Assignee set to Marius BĂLTEANU
  • Target version set to 5.0.7
Actions
Copy link
 #2

Updated by Marius BĂLTEANU 11 months ago

Indeed, the MENTION_PATTERN and LINKS_RE are not consistent right now, but I think in order to fix this more changes are needed.

Strict to the case reported by you, the following change should fix this:

diff --git a/lib/redmine/acts/mentionable.rb b/lib/redmine/acts/mentionable.rb
index e561e558f..10365ea86 100644
--- a/lib/redmine/acts/mentionable.rb
+++ b/lib/redmine/acts/mentionable.rb
@@ -100,6 +100,8 @@ module Redmine
           (?=
             (?=[[:punct:]][^A-Za-z0-9_\/])|
             ,|
+            !|
+            \?|
             \.+$|
             \s|
             \]|

What do you think? If you have a better fix, please post it.

Actions
Copy link
 #3

Updated by Thomas Löber 11 months ago

I'd allow any optional punctuation character before the end of the string. This solves the problem also for characters different from ! and ?:

diff --git a/lib/redmine/acts/mentionable.rb b/lib/redmine/acts/mentionable.rb
--- a/lib/redmine/acts/mentionable.rb
+++ b/lib/redmine/acts/mentionable.rb
@@ -103,7 +103,7 @@
             \s|
             \]|
             <|
-            $)
+            [[:punct:]]?$)
         /ix
       end
     end
Actions
Copy link
 #4

Updated by Marius BĂLTEANU 11 months ago

  • Status changed from Confirmed to Resolved
  • Resolution set to Fixed

Thomas Löber wrote in #note-3:

I'd allow any optional punctuation character before the end of the string. This solves the problem also for characters different from ! and ?:

[...]

Fix committed with udpated tests to cover these scenarios. Thanks for posting the patch.

Actions
Copy link
 #5

Updated by Marius BĂLTEANU 11 months ago

  • Status changed from Resolved to Closed

Fixes merged to stable branches, sorry for the noise with the commits.

Actions
Copy link

Also available in: Atom PDF