webhooks/gitlab: Add an option to exclude MR title from topics.

Since the title of a merge request can often change, it shouldn't be a
part of the topic that we send the message to. Otherwise things would
get messy and confusing.

But at the same time we don't want to make this mandatory. So we add
a new boolean GET parameter that can toggle whether or not the topic
should include the MR title (`use_merge_request_title`).

Fixes #15951.

Signed-off-by: Hemanth V. Alluri <hdrive1999@gmail.com>

zerver/webhooks/gitlab/doc.md

@@ -4,7 +4,11 @@ Receive GitLab notifications in Zulip!
 
 1. {!create-bot-construct-url-indented.md!}
 
-   {!git-webhook-url-with-branches-indented.md!}
+    By default, the Zulip topics for Merge Requests will contain the
+    title of the GitLab Merge Request.  You can change the topic format to
+    just contain the Merge Request ID by adding
+    `&use_merge_request_title=false` at the end of the URL.
+    {!git-webhook-url-with-branches-indented.md!}
 
 1. Go to your repository on GitLab and click **Settings** on the left
    sidebar.  Click on **Integrations**.

zerver/webhooks/gitlab/tests.py

@@ -253,6 +253,15 @@ class GitlabHookTests(WebhookTestCase):
             expected_topic,
             expected_message)
 
+    def test_note_merge_request_event_message_without_merge_request_title(self) -> None:
+        expected_topic = "my-awesome-project / MR #1"
+        expected_message = "Tomasz Kolek [commented](https://gitlab.com/tomaszkolek0/my-awesome-project/merge_requests/1#note_14171860) on [MR #1](https://gitlab.com/tomaszkolek0/my-awesome-project/merge_requests/1):\n\n~~~ quote\nNice merge request!\n~~~"
+        self.url = self.build_webhook_url(use_merge_request_title="false")  # To keep things as valid JSON.
+        self.send_and_test_stream_message(
+            'note_hook__merge_request_note',
+            expected_topic,
+            expected_message)
+
     def test_note_merge_request_with_custom_topic_in_url(self) -> None:
         self.url = self.build_webhook_url(topic='notifications')
         expected_topic = "notifications"
@@ -346,6 +355,15 @@ class GitlabHookTests(WebhookTestCase):
             expected_topic,
             expected_message)
 
+    def test_merge_request_closed_event_message_without_using_title(self) -> None:
+        expected_topic = "my-awesome-project / MR #2"
+        expected_message = "Tomasz Kolek closed [MR #2](https://gitlab.com/tomaszkolek0/my-awesome-project/merge_requests/2)."
+        self.url = self.build_webhook_url(use_merge_request_title="false")
+        self.send_and_test_stream_message(
+            'merge_request_hook__merge_request_closed',
+            expected_topic,
+            expected_message)
+
     def test_merge_request_closed_with_custom_topic_in_url(self) -> None:
         self.url = self.build_webhook_url(topic='notifications')
         expected_topic = "notifications"

zerver/webhooks/gitlab/view.py

@@ -8,6 +8,7 @@ from django.http import HttpRequest, HttpResponse
 from zerver.decorator import api_key_only_webhook_view
 from zerver.lib.request import REQ, has_request_variables
 from zerver.lib.response import json_success
+from zerver.lib.validator import check_bool
 from zerver.lib.webhooks.common import (
     UnexpectedWebhookEventType,
     check_send_webhook_message,
@@ -357,6 +358,7 @@ EVENT_FUNCTION_MAPPER = {
 def api_gitlab_webhook(request: HttpRequest, user_profile: UserProfile,
                        payload: Dict[str, Any]=REQ(argument_type='body'),
                        branches: Optional[str]=REQ(default=None),
+                       use_merge_request_title: bool=REQ(default=True, validator=check_bool),
                        user_specified_topic: Optional[str]=REQ("topic", default=None)) -> HttpResponse:
     event = get_event(request, payload, branches)
     if event is not None:
@@ -374,14 +376,14 @@ def api_gitlab_webhook(request: HttpRequest, user_profile: UserProfile,
             project_url = f"[{get_repo_name(payload)}]({get_project_homepage(payload)})"
             body = f"[{project_url}] {body}"
 
-        topic = get_subject_based_on_event(event, payload)
+        topic = get_subject_based_on_event(event, payload, use_merge_request_title)
         check_send_webhook_message(request, user_profile, topic, body)
     return json_success()
 
 def get_body_based_on_event(event: str) -> Any:
     return EVENT_FUNCTION_MAPPER[event]
 
-def get_subject_based_on_event(event: str, payload: Dict[str, Any]) -> str:
+def get_subject_based_on_event(event: str, payload: Dict[str, Any], use_merge_request_title: bool) -> str:
     if event == 'Push Hook':
         return f"{get_repo_name(payload)} / {get_branch_name(payload)}"
     elif event == 'Job Hook' or event == 'Build Hook':
@@ -395,7 +397,7 @@ def get_subject_based_on_event(event: str, payload: Dict[str, Any]) -> str:
             repo=get_repo_name(payload),
             type='MR',
             id=payload['object_attributes'].get('iid'),
-            title=payload['object_attributes'].get('title'),
+            title=payload['object_attributes'].get('title') if use_merge_request_title else "",
         )
     elif event.startswith('Issue Hook') or event.startswith('Confidential Issue Hook'):
         return TOPIC_WITH_PR_OR_ISSUE_INFO_TEMPLATE.format(
@@ -416,7 +418,7 @@ def get_subject_based_on_event(event: str, payload: Dict[str, Any]) -> str:
             repo=get_repo_name(payload),
             type='MR',
             id=payload['merge_request'].get('iid'),
-            title=payload['merge_request'].get('title'),
+            title=payload['merge_request'].get('title') if use_merge_request_title else "",
         )
 
     elif event == 'Note Hook Snippet':