pagure

Clone

#4981 Fix for #4978
Merged 3 years ago by pingou. Opened 3 years ago by misc.
misc/pagure fix_4978  into  master

file modified
+5 -1 
@@ -35,6 +35,7 @@ 
 

  import pagure.lib.tasks_services
 

  from pagure.config import config as pagure_config
 

  from pagure.pfmarkdown import MENTION_RE
 

+ from markdown.extensions.fenced_code import FencedBlockPreprocessor
 

  

  

  _log = logging.getLogger(__name__)
 
@@ -234,7 +235,10 @@ 
 

      """ Check the comment to see if an user is mentioned in it and if
 

      so add this user to the list of people to notify.
 

      """
 

-     for username in re.findall(MENTION_RE, comment):
 

+     filtered_comment = re.sub(
 

+         FencedBlockPreprocessor.FENCED_BLOCK_RE, "", comment
 

+     )
 

+     for username in re.findall(MENTION_RE, filtered_comment):
 

          user = pagure.lib.query.search_user(flask.g.session, username=username)
 

          if user:
 

              emails.add(user.default_email)

file modified
+26 
@@ -25,6 +25,7 @@ 
 

  import pagure.lib.notify
 

  import pagure.lib.query
 

  import tests
 

+ import munch
 

  

  

  class PagureLibNotifytests(tests.Modeltests):
 
@@ -543,6 +544,31 @@ 
 

  """
 

          self.assertEqual(email.as_string(), exp)
 

  

+     def test_notification_mention(self):
 

+         g = munch.Munch()
 

+         g.session = self.session
 

+         with patch("flask.g", g):
 

+ 

+             def _check_mention(comment, exp):
 

+                 emails = set([])
 

+                 emails = pagure.lib.notify._add_mentioned_users(
 

+                     emails, comment
 

+                 )
 

+ 

+                 self.assertEqual(emails, exp)
 

+ 

+             exp = set(["bar@pingou.com"])
 

+             comment = "I think we should ask @pingou how to pronounce pagure"
 

+             _check_mention(comment, exp)
 

+ 

+             exp = set([])
 

+             comment = """Let me quote him:
 

+ ~~~~

Pagure also support block quotes with 4 * ` ie:

````
foo bar
````

Adding a test for that seems to fail :/

 
+  @pingou> Pagure is pronounced 'pa-gure', not 'pagu-re'
 

+ ~~~~
 

+ """
 

+             _check_mention(comment, exp)
 

+ 

  

  if __name__ == "__main__":
 

      unittest.main(verbosity=2)

We need to first replace the fenced code block before
looking for mention.

rebased onto 599363c23df955b5c8b41c755f861a4161205876
3 years ago

rebased onto 0ecd97de5c15d8d6936e2c886fa61ee93d53180d
3 years ago

pretty please pagure-ci rebuild
3 years ago

Code style failure:

08:21:56  stdout: 
08:21:56  --- /pagure/pagure/lib/notify.py  2020-09-04 07:57:46.040486 +0000
08:21:56  +++ /pagure/pagure/lib/notify.py  2020-09-04 08:21:17.155764 +0000
08:21:56  @@ -233,12 +233,13 @@
08:21:56   
08:21:56   def _add_mentioned_users(emails, comment):
08:21:56       """ Check the comment to see if an user is mentioned in it and if
08:21:56       so add this user to the list of people to notify.
08:21:56       """
08:21:56  -    filtered_comment = re.sub(FencedBlockPreprocessor.FENCED_BLOCK_RE,
08:21:56  -                              "", comment)
08:21:56  +    filtered_comment = re.sub(
08:21:56  +        FencedBlockPreprocessor.FENCED_BLOCK_RE, "", comment
08:21:56  +    )
08:21:56       for username in re.findall(MENTION_RE, filtered_comment):
08:21:56           user = pagure.lib.query.search_user(flask.g.session, username=username)
08:21:56           if user:
08:21:56               emails.add(user.default_email)
08:21:56       return emails
08:21:56  --- /pagure/tests/test_pagure_lib_notify.py   2020-09-04 07:57:46.040486 +0000
08:21:56  +++ /pagure/tests/test_pagure_lib_notify.py   2020-09-04 08:21:39.417395 +0000
08:21:56  @@ -546,13 +546,16 @@
08:21:56   
08:21:56       def test_notification_mention(self):
08:21:56           g = munch.Munch()
08:21:56           g.session = self.session
08:21:56           with patch("flask.g", g):
08:21:56  +
08:21:56               def _check_mention(comment, exp):
08:21:56                   emails = set([])
08:21:56  -                emails = pagure.lib.notify._add_mentioned_users(emails, comment)
08:21:56  +                emails = pagure.lib.notify._add_mentioned_users(
08:21:56  +                    emails, comment
08:21:56  +                )
08:21:56   
08:21:56                   self.assertEqual(emails, exp)
08:21:56   
08:21:56               exp = set(["bar@pingou.com"])
08:21:56               comment = "I think we should ask @pingou how to pronounce pagure"
08:21:56  
08:21:56  stderr: 
08:21:56  would reformat /pagure/pagure/lib/notify.py
08:21:56  would reformat /pagure/tests/test_pagure_lib_notify.py
08:21:56  Oh no! 💥 💔 💥
08:21:56  2 files would be reformatted, 197 files would be left unchanged.

mhh, I tought I fixed it, maybe I didn't push it

rebased onto cd2792458c87acd16e998b2569089b9fe065f9de
3 years ago

Indeed, I guess I mixed up between laptops and/or branches, I pushed it now.

rebased onto 49d9543
3 years ago

:thumbsup:

Pagure also support block quotes with 4 * ` ie:

````
foo bar
````

Adding a test for that seems to fail :/

This are the tests I had added which are similar to yours and thought would be complementary and help me test the PR (and led me to discovering the ~ vs ` issue):

diff --git a/ tests/test_pagure_lib_notify_email.py b/ tests/test_pagure_lib_notify_email.py
index b0d9ea4e9..da211ebfd 100644
--- a/tests/test_pagure_lib_notify_email.py    
+++ b/tests/test_pagure_lib_notify_email.py    
@@ -255,6 +255,106 @@ http://localhost.localdomain/test/issue/1
         # Mail should be from user1 (who wrote the comment).
         self.assertEqual(kwargs["user_from"], self.user1.fullname)

+    @mock.patch("pagure.lib.notify.send_email")
+    def test_user_notified_new_comment_not_in_code_block(self, fakemail):
+        """Ensures that @mention doesn't over-reach in code-blocks."""
+        self.comment1.comment = "So apparently they said ``@foo.com is awesome`` :)"
+        g = munch.Munch()
+        g.fas_user = tests.FakeUser(username="pingou")
+        g.authenticated = True
+        g.session = self.session
+        with mock.patch("flask.g", g):
+            pagure.lib.notify.notify_new_comment(self.comment1)
+
+        (_, args, kwargs) = fakemail.mock_calls[0]
+
+        # Mail should be sent to both users
+        self.assertEqual(args[2], "bar@pingou.com")
+
+        # Mail ID should be comment #1's mail ID...
+        self.assertEqual(kwargs["mail_id"], self.comment1.mail_id)
+
+        # In reply to issue #1's mail ID.
+        self.assertEqual(kwargs["in_reply_to"], self.issue1.mail_id)
+
+        # Project name should be...project (full) name.
+        self.assertEqual(kwargs["project_name"], self.project1.fullname)
+
+        # Mail should be from user1 (who wrote the comment).
+        self.assertEqual(kwargs["user_from"], self.user1.fullname)
+
+    @mock.patch("pagure.lib.notify.send_email")
+    def test_user_notified_new_comment_not_in_code_block2(self, fakemail):
+        """Ensures that @mention doesn't over-reach in code-blocks."""
+        self.comment1.comment = """
+So apparently they said
+````
+@foo.com is awesome and @foo is great
+
+We all love @foo !
+```
+:)
+"""
+        g = munch.Munch()
+        g.fas_user = tests.FakeUser(username="pingou")
+        g.authenticated = True
+        g.session = self.session
+        with mock.patch("flask.g", g):
+            pagure.lib.notify.notify_new_comment(self.comment1)
+
+        (_, args, kwargs) = fakemail.mock_calls[0]
+
+        # Mail should be sent to both users
+        self.assertEqual(args[2], "bar@pingou.com")
+
+        # Mail ID should be comment #1's mail ID...
+        self.assertEqual(kwargs["mail_id"], self.comment1.mail_id)
+
+        # In reply to issue #1's mail ID.
+        self.assertEqual(kwargs["in_reply_to"], self.issue1.mail_id)
+
+        # Project name should be...project (full) name.
+        self.assertEqual(kwargs["project_name"], self.project1.fullname)
+
+        # Mail should be from user1 (who wrote the comment).
+        self.assertEqual(kwargs["user_from"], self.user1.fullname)
+
+    @mock.patch("pagure.lib.notify.send_email")
+    def test_user_notified_new_comment_not_in_code_block3(self, fakemail):
+        """Ensures that @mention doesn't over-reach in code-blocks."""
+        self.comment1.comment = """
+So apparently they said
+~~~~
+@foo.com is awesome and @foo is great
+
+We all love @foo !
+~~~~
+:)
+"""
+        g = munch.Munch()
+        g.fas_user = tests.FakeUser(username="pingou")
+        g.authenticated = True
+        g.session = self.session
+        with mock.patch("flask.g", g):
+            pagure.lib.notify.notify_new_comment(self.comment1)
+
+        (_, args, kwargs) = fakemail.mock_calls[0]
+
+        # Mail should be sent to both users
+        self.assertEqual(args[2], "bar@pingou.com")
+
+        # Mail ID should be comment #1's mail ID...
+        self.assertEqual(kwargs["mail_id"], self.comment1.mail_id)
+
+        # In reply to issue #1's mail ID.
+        self.assertEqual(kwargs["in_reply_to"], self.issue1.mail_id)
+
+        # Project name should be...project (full) name.
+        self.assertEqual(kwargs["project_name"], self.project1.fullname)
+
+        # Mail should be from user1 (who wrote the comment).
+        self.assertEqual(kwargs["user_from"], self.user1.fullname)
+
     @mock.patch("pagure.lib.notify.send_email")
     def test_notify_new_issue_namespaced(
         self, fakemail

Feel free to add them here if you like them

Oh, sorry!

Trying to debug this, I found out that my comment was invalid, the quotes are unbalanced (4 to open and only 3 to close the section). Fixing the block to be 4 to open and close fixes the test.

Let's get this in then and I'll add my tests in another PR (yes and I am belt and suspender person with these questions :))

Thanks for the PR! :)

Pull-Request has been merged by pingou
3 years ago
Changes Summary 2
+5 -1
file changed
pagure/lib/notify.py
+26 -0
file changed
tests/test_pagure_lib_notify.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.