[scm_request_processor] Make bot more interactive

This commits adds separate processing for comments in
scm_request_processor and adds following commands the bot will react on.

help - prints this help message
retry - retries the failed request processing
valid - can be used by authorized user to mark the request as valid

docs/scm_request_processor.md

@@ -10,6 +10,16 @@ The SCM Request processor accepts few topics emitted by pagure. These topics all
 * org.fedoraproject.*.pagure.issue.edit - Issue on pagure is edited
 * org.fedoraporject.*.pagure.issue.comment.added - New comment is added to issue
 
+## Supported commands
+The SCM Request processor now supports few commands that could be invoked by sending comment
+addressed to bot user (releng-bot) in issue opened on https://pagure.io/releng/fedora-scm-requests/
+
+Supported commands:
+
+* help - prints help for commands
+* retry - retries the failed request
+* valid - validate the request as authorized user
+
 ## Flowchart diagram
 
 [Flowchart diagram](./scm_request_processor.png) illustrates the code flow of the SCM Request Processor, what is being validated and what reaction is done if the validation fails.

tests/plugins/test_scm_request_processor.py

@@ -122,11 +122,11 @@ class TestProcess:
         msg = IssueCommentAddedV1(body=body)
 
         with patch(
-            "toddlers.plugins.scm_request_processor.SCMRequestProcessor.process_ticket"
+            "toddlers.plugins.scm_request_processor.SCMRequestProcessor.process_comment"
-        ) as mock_process_ticket:
+        ) as mock_process_comment:
             toddler.process(config, msg)
 
-            mock_process_ticket.assert_not_called()
+            mock_process_comment.assert_not_called()
 
         assert caplog.records[
             -1
@@ -231,6 +231,39 @@ class TestProcess:
         mock_fasjson.assert_called_with(config)
         mock_bugzilla.assert_called_with(config)
 
+    @patch("toddlers.utils.pagure.set_pagure")
+    @patch("toddlers.utils.fedora_account.set_fasjson")
+    @patch("toddlers.utils.bugzilla_system.set_bz")
+    def test_process_comment(self, mock_bugzilla, mock_fasjson, mock_pagure, toddler):
+        """
+        Assert that toddler will handle comments correctly.
+        """
+        pagure_user = "pagure_user"
+        config = {
+            "pagure_user": pagure_user,
+            "branch_slas": {},
+            "monitoring_choices": [],
+            "pagure_namespace_to_component": {},
+            "pagure_namespace_to_product": {},
+            "temp_dir": "",
+            "dist_git_url": "https://src.fedoraproject.org",
+            "dist_git_token": "Private API Key",
+        }
+        issue = {"status": "Open", "user": {"name": "zlopez"}}
+        body = {
+            "project": {"fullname": scm_request_processor.PROJECT_NAMESPACE},
+            "issue": {"status": "Open", "user": {"name": "zlopez"}},
+            "agent": "zlopez",
+        }
+        msg = IssueCommentAddedV1(body=body)
+
+        with patch(
+            "toddlers.plugins.scm_request_processor.SCMRequestProcessor.process_comment"
+        ) as mock_process_comment:
+            toddler.process(config, msg)
+
+            mock_process_comment.assert_called_with(issue)
+
 
 class TestProcessTicket:
     """
@@ -416,7 +449,7 @@ class TestProcessTicket:
         self.toddler.branch_slas = {"branch": "SLA"}
 
         with patch(
-            "toddlers.plugins.scm_request_processor.SCMRequestProcessor.create_new_repo"
+            "toddlers.plugins.scm_request_processor.SCMRequestProcessor.process_new_repo"
         ) as mock_new_repo:
             self.toddler.process_ticket(issue)
 
@@ -563,9 +596,194 @@ class TestVerifySLAs:
             self.toddler.verify_slas("", sla)
 
 
-class TestCreateNewRepo:
+class TestProcessComment:
     """
-    Test class for `toddlers.plugins.scm_request_processor.SCMRequestProcessor.create_new_repo`
+    Test class for `toddlers.plugins.scm_request_processor.SCMRequestProcessor.process_comment`.
+    """
+
+    def setup_method(self):
+        """
+        Initialize toddler.
+        """
+        self.toddler = scm_request_processor.SCMRequestProcessor()
+        self.toddler.pagure_io = Mock()
+        self.toddler.pagure_user = "pagure_user"
+
+    def test_process_comment_notification(self, caplog):
+        """
+        Assert that notification will be ignored.
+        """
+        issue = {
+            "comments": [{"notification": True}],
+            "full_url": "https://blacklibrary.wh40k",
+        }
+        caplog.set_level(logging.INFO)
+
+        self.toddler.process_comment(issue)
+
+        assert caplog.records[-1].message == "Comment is notification. Ignoring."
+
+    def test_process_comment_missing_bot_mention(self, caplog):
+        """
+        Assert that comment will be ignored if it doesn't mention bot.
+        """
+        issue = {
+            "comments": [{"notification": False, "comment": ""}],
+            "full_url": "https://blacklibrary.wh40k",
+        }
+        caplog.set_level(logging.INFO)
+
+        self.toddler.process_comment(issue)
+
+        assert caplog.records[
+            -1
+        ].message == "Comment is not for '{}'. Ignoring.".format(
+            self.toddler.pagure_user
+        )
+
+    def test_process_comment_help_command(self):
+        """
+        Assert that help command will be processed correctly.
+        """
+        issue = {
+            "id": 100,
+            "comments": [
+                {
+                    "notification": False,
+                    "comment": "@{0} help".format(self.toddler.pagure_user),
+                }
+            ],
+            "full_url": "https://blacklibrary.wh40k",
+        }
+
+        self.toddler.process_comment(issue)
+
+        self.toddler.pagure_io.add_comment_to_issue.assert_called_with(
+            issue["id"],
+            namespace=scm_request_processor.PROJECT_NAMESPACE,
+            comment=(
+                "{0} recognizes following commands on {1}.\n"
+                "help - prints this help message\n"
+                "retry - retries the failed request processing\n"
+                "valid - can be used by authorized user to mark the request as valid\n"
+            ).format(self.toddler.pagure_user, scm_request_processor.PROJECT_NAMESPACE),
+        )
+
+    def test_process_comment_retry_command(self):
+        """
+        Assert that retry command will be processed correctly.
+        """
+        issue = {
+            "id": 100,
+            "comments": [
+                {
+                    "notification": False,
+                    "comment": "@{0} retry".format(self.toddler.pagure_user),
+                }
+            ],
+            "full_url": "https://blacklibrary.wh40k",
+        }
+
+        with patch(
+            "toddlers.plugins.scm_request_processor.SCMRequestProcessor.process_ticket"
+        ) as mock_process_ticket:
+            self.toddler.process_comment(issue)
+
+            mock_process_ticket.assert_called_with(issue)
+
+    def test_process_comment_valid_command(self):
+        """
+        Assert that valid command from authorized user
+        will resume repo creation.
+        """
+        user = "zlopez"
+        issue = {
+            "id": 100,
+            "comments": [
+                {
+                    "user": {"name": user},
+                    "notification": False,
+                    "comment": "@{0} valid".format(self.toddler.pagure_user),
+                }
+            ],
+            "full_url": "https://blacklibrary.wh40k",
+            "content": '{"action": "new_repo"}',
+        }
+        self.toddler.pagure_io.get_project_contributors.return_value = {
+            "users": {"admin": [user], "commit": [], "ticket": []}
+        }
+
+        with patch(
+            "toddlers.plugins.scm_request_processor.SCMRequestProcessor._create_new_repo"
+        ) as mock_create_new_repo:
+            self.toddler.process_comment(issue)
+
+            mock_create_new_repo.assert_called_with(issue, {"action": "new_repo"})
+
+    def test_process_comment_valid_command_wrong_action(self):
+        """
+        Assert that valid command will fail for invalid action.
+        """
+        user = "zlopez"
+        issue = {
+            "id": 100,
+            "comments": [
+                {
+                    "user": {"name": user},
+                    "notification": False,
+                    "comment": "@{0} valid".format(self.toddler.pagure_user),
+                }
+            ],
+            "full_url": "https://blacklibrary.wh40k",
+            "content": '{"action": "new_branch"}',
+        }
+        self.toddler.pagure_io.get_project_contributors.return_value = {
+            "users": {"admin": [user], "commit": [], "ticket": []}
+        }
+
+        self.toddler.process_comment(issue)
+
+        self.toddler.pagure_io.add_comment_to_issue.assert_called_with(
+            issue["id"],
+            namespace=scm_request_processor.PROJECT_NAMESPACE,
+            comment="Validation can be done for 'new_repo' requests only.",
+        )
+
+    def test_process_comment_valid_command_unauthorized_user(self):
+        """
+        Assert that valid command will fail for unathorized user.
+        """
+        user = "zlopez"
+        issue = {
+            "id": 100,
+            "comments": [
+                {
+                    "user": {"name": user},
+                    "notification": False,
+                    "comment": "@{0} valid".format(self.toddler.pagure_user),
+                }
+            ],
+            "full_url": "https://blacklibrary.wh40k",
+            "content": '{"action": "new_repo"}',
+        }
+        self.toddler.pagure_io.get_project_contributors.return_value = {
+            "users": {"admin": ["Lucius"], "commit": [], "ticket": []}
+        }
+
+        self.toddler.process_comment(issue)
+
+        self.toddler.pagure_io.add_comment_to_issue.assert_called_with(
+            issue["id"],
+            namespace=scm_request_processor.PROJECT_NAMESPACE,
+            comment=(
+                "User {0} is not a maintainer of {1}." "Can't validate this request."
+            ).format(user, scm_request_processor.PROJECT_NAMESPACE),
+        )
+
+
+class TestProcessNewRepo:
+    """
+    Test class for `toddlers.plugins.scm_request_processor.SCMRequestProcessor.process_new_repo`
     method.
     """
 
@@ -576,15 +794,16 @@ class TestCreateNewRepo:
         self.toddler = scm_request_processor.SCMRequestProcessor()
         self.toddler.pagure_io = Mock()
         self.toddler.dist_git = Mock()
+        self.toddler.ping_comment = "{maintainers}"
 
-    def test_create_new_repo_missing_required_key(self):
+    def test_process_new_repo_missing_required_key(self):
         """
         Assert that ticket will be closed if required key is missing in request.
         """
         issue = {
             "id": 100,
         }
-        self.toddler.create_new_repo(issue, {})
+        self.toddler.process_new_repo(issue, {})
 
         self.toddler.pagure_io.close_issue.assert_called_with(
             100,
@@ -593,7 +812,7 @@ class TestCreateNewRepo:
             reason="Invalid",
         )
 
-    def test_create_new_repo_invalid_repo_name(self):
+    def test_process_new_repo_invalid_repo_name(self):
         """
         Assert that ticket will be closed if provided repository name is invalid.
         """
@@ -609,7 +828,7 @@ class TestCreateNewRepo:
             "monitor": "monitor",
         }
 
-        self.toddler.create_new_repo(issue, json)
+        self.toddler.process_new_repo(issue, json)
 
         error = (
             "The repository name is invalid. It must be at least two "
@@ -625,196 +844,7 @@ class TestCreateNewRepo:
             reason="Invalid",
         )
 
-    def test_create_new_repo_exception_validated_by_invalid_user(self):
+    def test_process_new_repo_missing_bug_id(self):
-        """
-        Assert that processing will be interrupted if the ticket is validated by
-        wrong user.
-        """
-        # Preparation
-        user = "zlopez"
-        invalid_user = "Tzeentch"
-        issue = {
-            "id": 100,
-            "user": {"name": user},
-            "comments": [
-                {
-                    "comment": "valid",
-                    "user": {"name": invalid_user},
-                    "notification": False,
-                }
-            ],
-        }
-
-        repo = "repo"
-        branch = "main"
-        namespace = "tests"
-        bug_id = ""
-        action = "new_repo"
-        sls = {branch: "2050-06-01"}
-        monitor = "monitor"
-        exception = False
-        json = {
-            "repo": repo,
-            "branch": branch,
-            "namespace": namespace,
-            "bug_id": bug_id,
-            "action": action,
-            "sls": sls,
-            "monitor": monitor,
-            "exception": exception,
-        }
-        self.toddler.pagure_io.get_project_contributors.return_value = {
-            "users": {"admin": [user], "commit": [], "ticket": []}
-        }
-        self.toddler.validation_comment = "valid"
-
-        self.toddler.create_new_repo(issue, json)
-
-        # asserts
-        self.toddler.pagure_io.get_project_contributors.assert_called_with(
-            scm_request_processor.PROJECT_NAMESPACE.split("/")[0],
-            scm_request_processor.PROJECT_NAMESPACE.split("/")[1],
-        )
-
-    def test_create_new_repo_exception_no_validation_comment(self):
-        """
-        Assert that processing will be interrupted if the ticket validation comment
-        is not found.
-        """
-        # Preparation
-        user = "zlopez"
-        issue = {
-            "id": 100,
-            "user": {"name": user},
-            "comments": [
-                {"comment": "comment", "user": {"name": user}, "notification": False}
-            ],
-        }
-
-        repo = "repo"
-        branch = "main"
-        namespace = "tests"
-        bug_id = ""
-        action = "new_repo"
-        sls = {branch: "2050-06-01"}
-        monitor = "monitor"
-        exception = False
-        json = {
-            "repo": repo,
-            "branch": branch,
-            "namespace": namespace,
-            "bug_id": bug_id,
-            "action": action,
-            "sls": sls,
-            "monitor": monitor,
-            "exception": exception,
-        }
-        self.toddler.pagure_io.get_project_contributors.return_value = {
-            "users": {"admin": [user], "commit": [], "ticket": []}
-        }
-        self.toddler.validation_comment = "valid"
-
-        self.toddler.create_new_repo(issue, json)
-
-        # asserts
-        self.toddler.pagure_io.get_project_contributors.assert_called_with(
-            scm_request_processor.PROJECT_NAMESPACE.split("/")[0],
-            scm_request_processor.PROJECT_NAMESPACE.split("/")[1],
-        )
-
-    def test_create_new_repo_exception_notify_maintainers(self):
-        """
-        Assert that comment will be added when the ticket needs manual
-        validation.
-        """
-        # Preparation
-        user = "zlopez"
-        issue = {"id": 100, "user": {"name": user}, "comments": []}
-
-        repo = "repo"
-        branch = "main"
-        namespace = "tests"
-        bug_id = ""
-        action = "new_repo"
-        sls = {branch: "2050-06-01"}
-        monitor = "monitor"
-        exception = False
-        json = {
-            "repo": repo,
-            "branch": branch,
-            "namespace": namespace,
-            "bug_id": bug_id,
-            "action": action,
-            "sls": sls,
-            "monitor": monitor,
-            "exception": exception,
-        }
-        self.toddler.pagure_io.get_project_contributors.return_value = {
-            "users": {"admin": [user, "Tzeentch"], "commit": [], "ticket": []}
-        }
-        self.toddler.ping_comment = "Look at this comment {maintainers}"
-        self.toddler.create_new_repo(issue, json)
-
-        # asserts
-        self.toddler.pagure_io.get_project_contributors.assert_called_with(
-            scm_request_processor.PROJECT_NAMESPACE.split("/")[0],
-            scm_request_processor.PROJECT_NAMESPACE.split("/")[1],
-        )
-
-        message = "Look at this comment @Tzeentch @{}".format(user)
-
-        self.toddler.pagure_io.add_comment_to_issue.assert_called_with(
-            100, namespace=scm_request_processor.PROJECT_NAMESPACE, comment=message
-        )
-
-    def test_create_new_repo_exception_not_valid_notification_comment_present(self):
-        """
-        Assert that comment will not be added if the toddler already commented on the
-        ticket.
-        """
-        # Preparation
-        user = "zlopez"
-        issue = {
-            "id": 100,
-            "user": {"name": user},
-            "comments": [
-                {"comment": "comment", "user": {"name": user}, "notification": False}
-            ],
-        }
-
-        repo = "repo"
-        branch = "main"
-        namespace = "tests"
-        bug_id = ""
-        action = "new_repo"
-        sls = {branch: "2050-06-01"}
-        monitor = "monitor"
-        exception = False
-        json = {
-            "repo": repo,
-            "branch": branch,
-            "namespace": namespace,
-            "bug_id": bug_id,
-            "action": action,
-            "sls": sls,
-            "monitor": monitor,
-            "exception": exception,
-        }
-        self.toddler.pagure_io.get_project_contributors.return_value = {
-            "users": {"admin": ["Tzeentch"], "commit": [], "ticket": []}
-        }
-        self.toddler.pagure_user = user
-        self.toddler.create_new_repo(issue, json)
-
-        # asserts
-        self.toddler.pagure_io.get_project_contributors.assert_called_with(
-            scm_request_processor.PROJECT_NAMESPACE.split("/")[0],
-            scm_request_processor.PROJECT_NAMESPACE.split("/")[1],
-        )
-
-        self.toddler.pagure_io.add_comment_to_issue.assert_not_called()
-
-    def test_create_new_repo_missing_bug_id(self):
         """
         Assert that ticket will be closed if Bugzilla bug id is not provided.
         """
@@ -830,7 +860,8 @@ class TestCreateNewRepo:
             "monitor": "monitor",
         }
 
-        self.toddler.create_new_repo(issue, json)
+        self.toddler.dist_git.get_project.return_value = None
+        self.toddler.process_new_repo(issue, json)
 
         self.toddler.pagure_io.close_issue.assert_called_with(
             100,
@@ -842,7 +873,7 @@ class TestCreateNewRepo:
     @patch(
         "toddlers.plugins.scm_request_processor.SCMRequestProcessor.validate_review_bug"
     )
-    def test_create_new_repo_invalid_review_bug(self, mock_validate_review_bug):
+    def test_process_new_repo_invalid_review_bug(self, mock_validate_review_bug):
         """
         Assert that ticket will be closed if Bugzilla bug is not valid.
         """
@@ -858,9 +889,10 @@ class TestCreateNewRepo:
             "monitor": "monitor",
         }
 
+        self.toddler.dist_git.get_project.return_value = None
         mock_validate_review_bug.side_effect = ValidationError("error")
 
-        self.toddler.create_new_repo(issue, json)
+        self.toddler.process_new_repo(issue, json)
 
         mock_validate_review_bug.assert_called_with(
             "123", "repo", "rawhide", namespace="rpms", pagure_user="zlopez"
@@ -876,12 +908,7 @@ class TestCreateNewRepo:
     @patch(
         "toddlers.plugins.scm_request_processor.SCMRequestProcessor.valid_epel_package"
     )
-    @patch(
+    def test_process_new_repo_invalid_epel(self, mock_valid_epel_package):
-        "toddlers.plugins.scm_request_processor.SCMRequestProcessor.validate_review_bug"
-    )
-    def test_create_new_repo_invalid_epel(
-        self, mock_validate_review_bug, mock_valid_epel_package
-    ):
         """
         Assert that ticket will be closed if repo is invalid EPEL repo.
         """
@@ -908,14 +935,10 @@ class TestCreateNewRepo:
 
         mock_valid_epel_package.return_value = False
 
-        self.toddler.create_new_repo(issue, json)
+        self.toddler.process_new_repo(issue, json)
 
         mock_valid_epel_package.assert_called_with(repo, branch)
 
-        mock_validate_review_bug.assert_called_with(
-            bug_id, repo, branch, namespace=namespace, pagure_user="zlopez"
-        )
-
         self.toddler.pagure_io.close_issue.assert_called_with(
             100,
             namespace=scm_request_processor.PROJECT_NAMESPACE,
@@ -923,10 +946,7 @@ class TestCreateNewRepo:
             reason="Invalid",
         )
 
-    @patch(
+    def test_process_new_repo_requester_not_in_dist_git(self):
-        "toddlers.plugins.scm_request_processor.SCMRequestProcessor.validate_review_bug"
-    )
-    def test_create_new_repo_requester_not_in_dist_git(self, mock_validate_review_bug):
         """
         Assert that ticket will be commented on when requester doesn't
         have a valid dist git account.
@@ -955,11 +975,7 @@ class TestCreateNewRepo:
         self.toddler.dist_git.user_exists.return_value = False
         self.toddler.dist_git._pagure_url = "https://src.fedoraproject.org"
 
-        self.toddler.create_new_repo(issue, json)
+        self.toddler.process_new_repo(issue, json)
-
-        mock_validate_review_bug.assert_called_with(
-            bug_id, repo, branch, namespace=namespace, pagure_user="zlopez"
-        )
 
         self.toddler.dist_git.user_exists.assert_called_with("zlopez")
 
@@ -973,10 +989,7 @@ class TestCreateNewRepo:
             comment=message,
         )
 
-    @patch(
+    def test_process_new_repo_project_exists(self):
-        "toddlers.plugins.scm_request_processor.SCMRequestProcessor.validate_review_bug"
-    )
-    def test_create_new_repo_project_exists(self, mock_validate_review_bug):
         """
         Assert that ticket will be closed when repo already exists in dist git.
         """
@@ -1003,11 +1016,7 @@ class TestCreateNewRepo:
 
         self.toddler.dist_git.get_project.return_value = "project"
 
-        self.toddler.create_new_repo(issue, json)
+        self.toddler.process_new_repo(issue, json)
-
-        mock_validate_review_bug.assert_called_with(
-            bug_id, repo, branch, namespace=namespace, pagure_user="zlopez"
-        )
 
         self.toddler.dist_git.get_project.assert_called_with(namespace, repo)
 
@@ -1018,10 +1027,7 @@ class TestCreateNewRepo:
             reason="Invalid",
         )
 
-    @patch(
+    def test_process_new_repo_master_branch(self):
-        "toddlers.plugins.scm_request_processor.SCMRequestProcessor.validate_review_bug"
-    )
-    def test_create_new_repo_master_branch(self, mock_validate_review_bug):
         """
         Assert that ticket will be closed when branch is set to master branch.
         Master branch is no longer allowed.
@@ -1048,11 +1054,7 @@ class TestCreateNewRepo:
         }
         self.toddler.dist_git.get_project.return_value = None
 
-        self.toddler.create_new_repo(issue, json)
+        self.toddler.process_new_repo(issue, json)
-
-        mock_validate_review_bug.assert_called_with(
-            bug_id, repo, branch, namespace=namespace, pagure_user="zlopez"
-        )
 
         self.toddler.dist_git.get_project.assert_called_with(namespace, repo)
 
@@ -1063,7 +1065,7 @@ class TestCreateNewRepo:
             reason="Invalid",
         )
 
-    def test_create_new_repo_unsupported_namespace(self):
+    def test_process_new_repo_unsupported_namespace(self):
         """
         Assert that ticket will be closed when requested namespace is not recognized.
         """
@@ -1087,7 +1089,7 @@ class TestCreateNewRepo:
             "monitor": monitor,
             "exception": exception,
         }
-        self.toddler.create_new_repo(issue, json)
+        self.toddler.process_new_repo(issue, json)
 
         error = (
             "The requested namespace '{0}' is not recognized. "
@@ -1105,14 +1107,15 @@ class TestCreateNewRepo:
     @pytest.mark.parametrize(
         "namespace, branch",
         [
-            ("rpms", "rawhide"),
+            ("tests", "main"),
             ("container", "rawhide"),
             ("flatpaks", "stable"),
         ],
     )
-    def test_create_new_repo_namespaces(self, namespace, branch):
+    def test_process_new_repo_namespaces(self, namespace, branch):
         """
-        Assert that ticket will be processed when everything is in order and namespace is correct.
+        Assert that maintainers will be notified when repo is created in
+        non rpms namespace.
         """
         # Preparation
         user = "zlopez"
@@ -1120,7 +1123,8 @@ class TestCreateNewRepo:
             "id": 100,
             "user": {"name": user},
             "comments": [
-                {"comment": "valid", "user": {"name": user}, "notification": False}
+                {"notification": False, "comment": "First"},
+                {"notification": True},
             ],
         }
 
@@ -1146,57 +1150,43 @@ class TestCreateNewRepo:
         self.toddler.pagure_io.get_project_contributors.return_value = {
             "users": {"admin": [user], "commit": [], "ticket": []}
         }
-        self.toddler.validation_comment = "valid"
 
         # Method to test
-        self.toddler.create_new_repo(issue, json)
+        self.toddler.process_new_repo(issue, json)
 
         # asserts
-        self.toddler.dist_git.get_project.assert_called_with(namespace, repo)
+        self.toddler.pagure_io.add_comment_to_issue.assert_called_with(
-        self.toddler.dist_git.new_project.assert_called_with(
+            issue["id"],
-            namespace, repo, "", "", branch, initial_commit=True, alias=True
-        )
-        self.toddler.dist_git.set_monitoring_status.assert_called_with(
-            namespace, repo, monitor
-        )
-        self.toddler.dist_git.change_project_main_admin.assert_called_with(
-            namespace, repo, "zlopez"
-        )
-
-        message = "The Pagure repository was created at {0}/{1}/{2}".format(
-            dist_git_url, namespace, repo
-        )
-
-        self.toddler.pagure_io.close_issue.assert_called_with(
-            100,
             namespace=scm_request_processor.PROJECT_NAMESPACE,
-            message=message,
+            comment="@" + user,
-            reason="Processed",
         )
 
-    def test_create_new_repo_tests_namespace(self):
+    def test_process_new_repo_ping_comment_exists(self, caplog):
         """
-        Assert that ticket will be processed when everything is in order and namespace
+        Assert that maintainers are not notified multiple times.
-        is set to tests.
         """
         # Preparation
+        caplog.set_level(logging.INFO)
         user = "zlopez"
         issue = {
             "id": 100,
             "user": {"name": user},
             "comments": [
-                {"comment": "valid", "user": {"name": user}, "notification": False}
+                {
+                    "comment": "@" + user,
+                    "notification": False,
+                }
             ],
         }
 
         repo = "repo"
-        branch = "main"
+        namespace = "rpms"
-        namespace = "tests"
+        branch = "rawhide"
         bug_id = ""
         action = "new_repo"
         sls = {branch: "2050-06-01"}
         monitor = "monitor"
-        exception = False
+        exception = True
         json = {
             "repo": repo,
             "branch": branch,
@@ -1213,42 +1203,18 @@ class TestCreateNewRepo:
         self.toddler.pagure_io.get_project_contributors.return_value = {
             "users": {"admin": [user], "commit": [], "ticket": []}
         }
-        self.toddler.validation_comment = "valid"
 
-        self.toddler.create_new_repo(issue, json)
+        # Method to test
+        self.toddler.process_new_repo(issue, json)
 
         # asserts
-        self.toddler.pagure_io.get_project_contributors.assert_called_with(
+        assert caplog.records[-1].message == "- Ping comment found."
-            scm_request_processor.PROJECT_NAMESPACE.split("/")[0],
-            scm_request_processor.PROJECT_NAMESPACE.split("/")[1],
-        )
-        self.toddler.dist_git.get_project.assert_called_with(namespace, repo)
-        self.toddler.dist_git.new_project.assert_called_with(
-            namespace, repo, "", "", branch, initial_commit=True, alias=True
-        )
-        self.toddler.dist_git.set_monitoring_status.assert_called_with(
-            namespace, repo, monitor
-        )
-        self.toddler.dist_git.change_project_main_admin.assert_called_with(
-            namespace, repo, "zlopez"
-        )
-
-        message = "The Pagure repository was created at {0}/{1}/{2}".format(
-            dist_git_url, namespace, repo
-        )
-
-        self.toddler.pagure_io.close_issue.assert_called_with(
-            100,
-            namespace=scm_request_processor.PROJECT_NAMESPACE,
-            message=message,
-            reason="Processed",
-        )
 
     @patch("toddlers.plugins.scm_request_processor.bugzilla_system")
     @patch(
         "toddlers.plugins.scm_request_processor.SCMRequestProcessor.validate_review_bug"
     )
-    def test_create_new_repo_non_default_branch(
+    def test_process_new_repo_non_default_branch(
         self, mock_validate_review_bug, mock_bz
     ):
         """
@@ -1281,7 +1247,7 @@ class TestCreateNewRepo:
         self.toddler.dist_git.get_project.return_value = None
         self.toddler.dist_git._pagure_url = dist_git_url
 
-        self.toddler.create_new_repo(issue, json)
+        self.toddler.process_new_repo(issue, json)
 
         # asserts
         mock_validate_review_bug.assert_called_with(
@@ -1317,7 +1283,7 @@ class TestCreateNewRepo:
     @patch(
         "toddlers.plugins.scm_request_processor.SCMRequestProcessor.validate_review_bug"
     )
-    def test_create_new_repo_default_brach(self, mock_validate_review_bug, mock_bz):
+    def test_process_new_repo_default_brach(self, mock_validate_review_bug, mock_bz):
         """
         Assert that repo will be created with default branch when everything is in order.
         """
@@ -1346,7 +1312,7 @@ class TestCreateNewRepo:
         self.toddler.dist_git.get_project.return_value = None
         self.toddler.dist_git._pagure_url = dist_git_url
 
-        self.toddler.create_new_repo(issue, json)
+        self.toddler.process_new_repo(issue, json)
 
         # asserts
         mock_validate_review_bug.assert_called_with(

toddlers.toml.example

@@ -249,8 +249,6 @@ pagure_url = "https://pagure.io"
 pagure_api_key = "API token for pagure"
 # Monitoring choices for release-monitoring.org
 monitor_choices = ['no-monitoring', 'monitoring', 'monitoring-with-scratch']
-# What we should look for in validation comment
-validation_comment = "valid"
 # Text for the ping if the ticket needs to be manually verified
 ping_comment = "This request wants to skip bugzilla validation! {maintainers} could you check if this is correct? If yes, please respond to this ticket with 'valid' comment"
 

toddlers/plugins/scm_request_processor.py

@@ -186,7 +186,10 @@ class SCMRequestProcessor(ToddlerBase):
         bugzilla_system.set_bz(config)
 
         try:
-            self.process_ticket(issue)
+            if message.topic.endswith("pagure.issue.comment.added"):
+                self.process_comment(issue)
+            else:
+                self.process_ticket(issue)
         except BaseException:
             self.pagure_io.add_comment_to_issue(
                 issue["id"],
@@ -196,6 +199,80 @@ class SCMRequestProcessor(ToddlerBase):
                 ).format(traceback.format_exc()),
             )
 
+    def process_comment(self, issue: dict):
+        """
+        Process comment on the issue.
+
+        Params:
+          issue: A dictionary containing the issue.
+        """
+        _log.info("Processing comment on pagure ticket '{0}'".format(issue["full_url"]))
+
+        last_comment = issue["comments"][-1]
+
+        # Comment is notification
+        if last_comment["notification"]:
+            _log.info("Comment is notification. Ignoring.")
+            return
+        # Check if the comment is addressed to bot user
+        if "@" + self.pagure_user not in last_comment["comment"]:
+            _log.info("Comment is not for '{0}'. Ignoring.".format(self.pagure_user))
+            return
+
+        # Extract command from comment @releng-bot command
+        command = last_comment["comment"].split()[1]
+
+        if command == "help":
+            self.pagure_io.add_comment_to_issue(
+                issue["id"],
+                namespace=PROJECT_NAMESPACE,
+                comment=(
+                    "{0} recognizes following commands on {1}.\n"
+                    "help - prints this help message\n"
+                    "retry - retries the failed request processing\n"
+                    "valid - can be used by authorized user to mark the request as valid\n"
+                ).format(self.pagure_user, PROJECT_NAMESPACE),
+            )
+        elif command == "retry":
+            self.process_ticket(issue)
+        elif command == "valid":
+            # Get full list of contributors
+            contributors = self.pagure_io.get_project_contributors(
+                PROJECT_NAMESPACE.split("/")[0], PROJECT_NAMESPACE.split("/")[1]
+            )
+            # Get the list of maintainers of the scm_requests repository
+            maintainers = (
+                set(contributors["users"]["admin"])
+                | set(contributors["users"]["commit"])
+                | set(contributors["users"]["ticket"])
+            )
+            if last_comment["user"]["name"] in maintainers:
+                _log.info(
+                    "Ticket is validated by {}, continue processing".format(
+                        last_comment["user"]["name"]
+                    )
+                )
+                issue_body_json = json.loads(issue["content"].strip("`").strip("\n"))
+                if issue_body_json.get("action") == "new_repo":
+                    self._create_new_repo(issue, issue_body_json)
+                else:
+                    self.pagure_io.add_comment_to_issue(
+                        issue["id"],
+                        namespace=PROJECT_NAMESPACE,
+                        comment=(
+                            "Validation can be done for 'new_repo' requests only."
+                        ),
+                    )
+            else:
+                self.pagure_io.add_comment_to_issue(
+                    issue["id"],
+                    namespace=PROJECT_NAMESPACE,
+                    comment=(
+                        "User {0} is not a maintainer of {1}."
+                        "Can't validate this request."
+                    ).format(last_comment["user"]["name"], PROJECT_NAMESPACE),
+                )
+
     def process_ticket(self, issue: dict):
         """
         Process a single ticket
@@ -294,7 +371,7 @@ class SCMRequestProcessor(ToddlerBase):
             return
 
         if issue_body.get("action") == "new_repo":
-            self.create_new_repo(
+            self.process_new_repo(
                 issue, issue_body, initial_commit=issue_body.get("initial_commit", True)
             )
         elif issue_body.get("action") == "new_branch":
@@ -350,11 +427,11 @@ class SCMRequestProcessor(ToddlerBase):
                     'The SL "{0}" must expire on June 1st or December 1st'.format(eol)
                 )
 
-    def create_new_repo(
+    def process_new_repo(
         self, issue: dict, issue_body_json: dict, initial_commit: bool = True
     ):
         """
-        A helper function that process request for new repo.
+        Process request for new repo.
 
         Params:
           issue: the dictionary representing the issue
@@ -382,6 +459,23 @@ class SCMRequestProcessor(ToddlerBase):
                 )
                 return
 
+        # Validate the request first
+        if self._validate_new_repo_request(issue, issue_body_json):
+            _log.info("Ticket passed all validations. Creating repository.")
+            self._create_new_repo(issue, issue_body_json, initial_commit)
+
+    def _validate_new_repo_request(self, issue: dict, issue_body_json: dict) -> bool:
+        """
+        Validate request for new repo.
+
+        Params:
+          issue: the dictionary representing the issue
+          issue_body_json: a partially validated dictionary of the JSON in the
+            issue body
+
+        Returns:
+          True if the request is valid, false otherwise.
+        """
         requester = issue["user"]["name"]
         namespace = issue_body_json["namespace"].strip()
         repo = issue_body_json["repo"].strip()
@@ -403,7 +497,7 @@ class SCMRequestProcessor(ToddlerBase):
                 message=message,
                 reason="Invalid",
             )
-            return
+            return False
 
         if not re.match(PROJECT_NAME_REGEX, repo):
             error = (
@@ -418,7 +512,57 @@ class SCMRequestProcessor(ToddlerBase):
                 message=error,
                 reason="Invalid",
             )
-            return
+            return False
+
+        # This should never trigger because if the user requested an EPEL branch
+        # here, it'd have to be accompanied by a Bugzilla bug for the "Fedora EPEL"
+        # product. So this will only trigger if the reviewer made a mistake.
+        if re.match(EPEL_REGEX, branch_name) and not self.valid_epel_package(
+            repo, branch_name
+        ):
+            self.pagure_io.close_issue(
+                issue["id"],
+                namespace=PROJECT_NAMESPACE,
+                message=INVALID_EPEL_ERROR,
+                reason="Invalid",
+            )
+            return False
+
+        _log.info(
+            "- Checking if user {0} has an account in dist-git.".format(requester)
+        )
+        if not self.dist_git.user_exists(requester):
+            sync_comment = (
+                "@{0} needs to login to {1} to sync accounts "
+                "before we can proceed.".format(requester, self.dist_git._pagure_url)
+            )
+            self.pagure_io.add_comment_to_issue(
+                issue["id"], namespace=PROJECT_NAMESPACE, comment=sync_comment
+            )
+            return False
+
+        _log.info(
+            "- Checking if {0}/{1} already exists in dist-git.".format(namespace, repo)
+        )
+        project = self.dist_git.get_project(namespace, repo)
+        if project:
+            self.pagure_io.close_issue(
+                issue["id"],
+                namespace=PROJECT_NAMESPACE,
+                message="The Pagure project already exists",
+                reason="Invalid",
+            )
+            return False
+
+        # Close the ticket if the requested default branch is master
+        if branch_name == "master":
+            self.pagure_io.close_issue(
+                issue["id"],
+                namespace=PROJECT_NAMESPACE,
+                message="Branch `master` cannot be created, please request the right branch.",
+                reason="Invalid",
+            )
+            return False
 
         if exception is True or namespace in (
             "flatpaks",
@@ -436,44 +580,24 @@ class SCMRequestProcessor(ToddlerBase):
                 | set(contributors["users"]["ticket"])
             )
             # Ping the maintainers if toddler didn't comment on the ticket yet
-            ping_comment = True
+            ping_comment = self.ping_comment.format(
-            # This is set to true if the validation comment is set by maintainer
+                maintainers=" ".join(["@" + user for user in sorted(maintainers)])
-            valid = False
+            )
-            # There is already a comment on this ticket, we should check if this is
-            # the validation ticket and if we already responded to the ticket
             if len(issue["comments"]) > 0:
-                _log.info("- Check for validation comment")
+                _log.info("- Check for ping comment")
                 for comment in issue["comments"]:
                     # Skip the notification comments
                     if not comment["notification"]:
-                        if comment["comment"].strip() == self.validation_comment:
+                        # We already pinged the maintainers
-                            if comment["user"]["name"] in maintainers:
+                        if comment["comment"].strip() == ping_comment:
-                                _log.info(
+                            _log.info("- Ping comment found.")
-                                    "- Ticket is validated by {}, continue processing".format(
+                            return False
-                                        comment["user"]["name"]
-                                    )
-                                )
-                                valid = True
-                        # Toddler already commented on the ticket
-                        if comment["user"]["name"] == self.pagure_user:
-                            ping_comment = False
-                if not valid and not ping_comment:
-                    _log.info("- Ticket is not yet validated. Skipping...")
-                    return
-
             # We didn't commented on the ticket yet, ping the maintainers
-            if ping_comment and not valid:
+            _log.info("- Notify the responsible users")
-                _log.info("- Notify the responsible users")
+            self.pagure_io.add_comment_to_issue(
-                self.pagure_io.add_comment_to_issue(
+                issue["id"], namespace=PROJECT_NAMESPACE, comment=ping_comment
-                    issue["id"],
+            )
-                    namespace=PROJECT_NAMESPACE,
+            return False
-                    comment=self.ping_comment.format(
-                        maintainers=" ".join(
-                            ["@" + user for user in sorted(maintainers)]
-                        )
-                    ),
-                )
-                return
         else:
             if not bug_id:
                 self.pagure_io.close_issue(
@@ -482,7 +606,7 @@ class SCMRequestProcessor(ToddlerBase):
                     message="An invalid Bugzilla bug was provided",
                     reason="Invalid",
                 )
-                return
+                return False
             try:
                 _log.info("- Checking that #{0} is a valid RHBZ.".format(bug_id))
                 self.validate_review_bug(
@@ -499,59 +623,30 @@ class SCMRequestProcessor(ToddlerBase):
                     message=str(error),
                     reason="Invalid",
                 )
-                return
+                return False
 
-        # This should never trigger because if the user requested an EPEL branch
+        return True
-        # here, it'd have to be accompanied by a Bugzilla bug for the "Fedora EPEL"
-        # product. So this will only trigger if the reviewer made a mistake.
-        if re.match(EPEL_REGEX, branch_name) and not self.valid_epel_package(
-            repo, branch_name
-        ):
-            self.pagure_io.close_issue(
-                issue["id"],
-                namespace=PROJECT_NAMESPACE,
-                message=INVALID_EPEL_ERROR,
-                reason="Invalid",
-            )
-            return
 
-        _log.info(
+    def _create_new_repo(
-            "- Checking if user {0} has an account in dist-git.".format(requester)
+        self, issue: dict, issue_body_json: dict, initial_commit: bool = True
-        )
+    ):
-        if not self.dist_git.user_exists(requester):
+        """
-            sync_comment = (
+        Create new repository.
-                "@{0} needs to login to {1} to sync accounts "
-                "before we can proceed.".format(requester, self.dist_git._pagure_url)
-            )
-            self.pagure_io.add_comment_to_issue(
-                issue["id"], namespace=PROJECT_NAMESPACE, comment=sync_comment
-            )
-            return
-
-        _log.info(
-            "- Checking if {0}/{1} already exists in dist-git.".format(namespace, repo)
-        )
-        project = self.dist_git.get_project(namespace, repo)
-        if project:
-            self.pagure_io.close_issue(
-                issue["id"],
-                namespace=PROJECT_NAMESPACE,
-                message="The Pagure project already exists",
-                reason="Invalid",
-            )
-            return
 
+        Params:
+          issue: the dictionary representing the issue
+          issue_body_json: a partially validated dictionary of the JSON in the
+            issue body
+          initial_commit: indicate whether to create an initial commit.
+        """
+        requester = issue["user"]["name"]
+        namespace = issue_body_json["namespace"].strip()
+        repo = issue_body_json["repo"].strip()
+        bug_id = str(issue_body_json["bug_id"]).strip()
+        branch_name = issue_body_json["branch"].strip()
         description = issue_body_json.get("description", "").strip()
         upstreamurl = issue_body_json.get("upstreamurl", "").strip()
-        # Close the ticket if the requested default branch is master
+
-        if branch_name == "master":
-            self.pagure_io.close_issue(
-                issue["id"],
-                namespace=PROJECT_NAMESPACE,
-                message="Branch `master` cannot be created, please request the right branch.",
-                reason="Invalid",
-            )
-            return
         if namespace in ["rpms", "container"]:
             default_branch = "rawhide"
         elif namespace in ["flatpaks"]:
@@ -559,8 +654,6 @@ class SCMRequestProcessor(ToddlerBase):
         elif namespace in ["tests"]:
             default_branch = "main"
 
-        _log.info("Ticket passed all validations. Creating repository.")
-
         # Create the Pagure repo
         dist_git_url = "{0}/{1}/{2}".format(
             self.dist_git._pagure_url.rstrip("/"), namespace, repo