From 59d5d29ed857dabe470c0c40eaf782846ec6571a Mon Sep 17 00:00:00 2001 From: Varun-Kolanu Date: Mon, 31 Mar 2025 19:00:36 +0530 Subject: [PATCH] integrations: Add support for GitLab design comments. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #26199. Co-authored-by: Barış Co-authored-by: Satyam Bansal --- .../fixtures/note_hook__design_note.json | 94 +++++++++++++++++++ zerver/webhooks/gitlab/tests.py | 13 +++ zerver/webhooks/gitlab/view.py | 68 +++++++++++++- 3 files changed, 174 insertions(+), 1 deletion(-) create mode 100644 zerver/webhooks/gitlab/fixtures/note_hook__design_note.json diff --git a/zerver/webhooks/gitlab/fixtures/note_hook__design_note.json b/zerver/webhooks/gitlab/fixtures/note_hook__design_note.json new file mode 100644 index 0000000000..747eb15efe --- /dev/null +++ b/zerver/webhooks/gitlab/fixtures/note_hook__design_note.json @@ -0,0 +1,94 @@ +{ + "object_kind": "note", + "event_type": "note", + "user": { + "id": 14613894, + "name": "Satyam Bansal", + "username": "sbansal1999", + "avatar_url": "https://gitlab.com/uploads/-/system/user/avatar/14613894/avatar.png", + "email": "[REDACTED]" + }, + "project_id": 46186758, + "project": { + "id": 46186758, + "name": "testing-zulip-gitlab-integration", + "description": null, + "web_url": "https://gitlab.com/sbansal1999/testing-zulip-gitlab-integration", + "avatar_url": null, + "git_ssh_url": "git@gitlab.com:sbansal1999/testing-zulip-gitlab-integration.git", + "git_http_url": "https://gitlab.com/sbansal1999/testing-zulip-gitlab-integration.git", + "namespace": "Satyam Bansal", + "visibility_level": 0, + "path_with_namespace": "sbansal1999/testing-zulip-gitlab-integration", + "default_branch": "main", + "ci_config_path": "", + "homepage": "https://gitlab.com/sbansal1999/testing-zulip-gitlab-integration", + "url": "git@gitlab.com:sbansal1999/testing-zulip-gitlab-integration.git", + "ssh_url": "git@gitlab.com:sbansal1999/testing-zulip-gitlab-integration.git", + "http_url": "https://gitlab.com/sbansal1999/testing-zulip-gitlab-integration.git" + }, + "object_attributes": { + "attachment": null, + "author_id": 14613894, + "change_position": { + "base_sha": null, + "start_sha": null, + "head_sha": null, + "old_path": null, + "new_path": null, + "position_type": "text", + "old_line": null, + "new_line": null, + "line_range": null + }, + "commit_id": null, + "created_at": "2023-07-05 16:38:53 UTC", + "discussion_id": "90c3bd62a51e132229eb2433a42af0707c1b7091", + "id": 1458583152, + "line_code": null, + "note": "hello", + "noteable_id": 601586, + "noteable_type": "DesignManagement::Design", + "original_position": { + "base_sha": "0000000000000000000000000000000000000000", + "start_sha": "0000000000000000000000000000000000000000", + "head_sha": "3e8137cae02e78675cd220ae1dd2ced26feba62f", + "old_path": null, + "new_path": "designs/issue-1/Screenshot.png", + "position_type": "image", + "width": 740, + "height": 499, + "x": 413, + "y": 315 + }, + "position": { + "base_sha": "0000000000000000000000000000000000000000", + "start_sha": "0000000000000000000000000000000000000000", + "head_sha": "3e8137cae02e78675cd220ae1dd2ced26feba62f", + "old_path": null, + "new_path": "designs/issue-1/Screenshot.png", + "position_type": "image", + "width": 740, + "height": 499, + "x": 413, + "y": 315 + }, + "project_id": 46186758, + "resolved_at": null, + "resolved_by_id": null, + "resolved_by_push": null, + "st_diff": null, + "system": false, + "type": "DiffNote", + "updated_at": "2023-07-05 16:38:53 UTC", + "updated_by_id": null, + "description": "hello", + "url": null + }, + "repository": { + "name": "testing-zulip-gitlab-integration", + "url": "git@gitlab.com:sbansal1999/testing-zulip-gitlab-integration.git", + "description": null, + "homepage": "https://gitlab.com/sbansal1999/testing-zulip-gitlab-integration" + } +} diff --git a/zerver/webhooks/gitlab/tests.py b/zerver/webhooks/gitlab/tests.py index 0703541d41..004bb9008c 100644 --- a/zerver/webhooks/gitlab/tests.py +++ b/zerver/webhooks/gitlab/tests.py @@ -257,6 +257,12 @@ class GitlabHookTests(WebhookTestCase): self.check_webhook("note_hook__issue_note", expected_topic_name, expected_message) + def test_note_design_event_message(self) -> None: + expected_topic_name = "testing-zulip-gitlab-integration / design Screenshot.png" + expected_message = "Satyam Bansal [commented](https://gitlab.com/sbansal1999/testing-zulip-gitlab-integration/-/issues/1/designs/Screenshot.png#note_1458583152) on design [Screenshot.png](https://gitlab.com/sbansal1999/testing-zulip-gitlab-integration/-/issues/1/designs/Screenshot.png):\n\n~~~ quote\nhello\n~~~" + + self.check_webhook("note_hook__design_note", expected_topic_name, expected_message) + def test_note_confidential_issue_event_message(self) -> None: expected_subject = "testing-zulip-gitlab-integration / issue #1 Add more lines" expected_message = "Satyam Bansal [commented](https://gitlab.com/sbansal1999/testing-zulip-gitlab-integration/-/issues/1#note_1406130881) on [issue #1](https://gitlab.com/sbansal1999/testing-zulip-gitlab-integration/-/issues/1):\n\n~~~ quote\nSome more comments\n~~~" @@ -270,6 +276,13 @@ class GitlabHookTests(WebhookTestCase): self.check_webhook("note_hook__issue_note", expected_topic_name, expected_message) + def test_note_design_with_custom_topic_in_url(self) -> None: + self.url = self.build_webhook_url(topic="notifications") + expected_topic_name = "notifications" + expected_message = "[[testing-zulip-gitlab-integration](https://gitlab.com/sbansal1999/testing-zulip-gitlab-integration)] Satyam Bansal [commented](https://gitlab.com/sbansal1999/testing-zulip-gitlab-integration/-/issues/1/designs/Screenshot.png#note_1458583152) on design [Screenshot.png](https://gitlab.com/sbansal1999/testing-zulip-gitlab-integration/-/issues/1/designs/Screenshot.png):\n\n~~~ quote\nhello\n~~~" + + self.check_webhook("note_hook__design_note", expected_topic_name, expected_message) + def test_note_snippet_old_event_message(self) -> None: expected_topic_name = "my-awesome-project / snippet #2 test" expected_message = "Tomasz Kolek [commented](https://gitlab.com/tomaszkolek0/my-awesome-project/snippets/2#note_14172058) on [snippet #2](https://gitlab.com/tomaszkolek0/my-awesome-project/-/snippets/2):\n\n~~~ quote\nNice snippet\n~~~" diff --git a/zerver/webhooks/gitlab/view.py b/zerver/webhooks/gitlab/view.py index 6ec02bd3ff..2cf18470d7 100644 --- a/zerver/webhooks/gitlab/view.py +++ b/zerver/webhooks/gitlab/view.py @@ -31,6 +31,11 @@ from zerver.lib.webhooks.git import ( ) from zerver.models import UserProfile +TOPIC_WITH_DESIGN_INFO_TEMPLATE = "{repo} / {type} {design_name}" +DESIGN_COMMENT_MESSAGE_TEMPLATE = ( + "{user_name} {action} on design [{design_name}]({design_url}):\n{content_message}" +) + def fixture_to_headers(fixture_name: str) -> dict[str, str]: if fixture_name.startswith("build"): @@ -209,6 +214,23 @@ def replace_assignees_username_with_name( return formatted_assignees +def parse_design_comment(comment: WildValue, repository_url: str) -> tuple[str, str, str]: + note_id = comment["id"].tame(check_int) + + # As there is no issue field in the payloads related to designs, + # we need to parse the issue number from the new_path field. + design_path = comment["position"]["new_path"].tame(check_string) + + # Sample design_path: "designs/issue-1/Screenshot_20250302_230445.png" + _, issue_subpath, design_name = design_path.split("/") + + issue_number = issue_subpath.split("-")[-1] + design_url = f"{repository_url}/-/issues/{issue_number}/designs/{design_name}" + comment_url = f"{design_url}#note_{note_id}" + + return comment_url, design_url, design_name + + def get_commented_commit_event_body(payload: WildValue, include_title: bool) -> str: comment = payload["object_attributes"] action = "[commented]({})".format(comment["url"].tame(check_string)) @@ -253,6 +275,23 @@ def get_commented_issue_event_body(payload: WildValue, include_title: bool) -> s ) +def get_commented_design_event_body(payload: WildValue, include_title: bool) -> str: + comment = payload["object_attributes"] + repository_url = payload["repository"]["homepage"].tame(check_string) + + comment_url, design_url, design_name = parse_design_comment(comment, repository_url) + action = f"[commented]({comment_url})" + content_message = CONTENT_MESSAGE_TEMPLATE.format(message=comment["note"].tame(check_string)) + + return DESIGN_COMMENT_MESSAGE_TEMPLATE.format( + user_name=get_issue_user_name(payload), + action=action, + design_name=design_name, + design_url=design_url, + content_message=content_message, + ) + + def get_commented_snippet_event_body(payload: WildValue, include_title: bool) -> str: comment = payload["object_attributes"] action = "[commented]({}) on".format(comment["url"].tame(check_string)) @@ -405,6 +444,17 @@ def get_object_url(payload: WildValue) -> str: return payload["object_attributes"]["url"].tame(check_string) +def skip_previews(event: str) -> bool: + # Add event names to this array for which previews should be skipped. + return event in [ + # Design events link to images, but the images cannot be + # accessed without authentication, so trying to preview them + # doesn't work. + "Note Hook DesignManagement::Design", + "Confidential Note Hook DesignManagement::Design", + ] + + class EventFunction(Protocol): def __call__(self, payload: WildValue, include_title: bool) -> str: ... @@ -425,6 +475,8 @@ EVENT_FUNCTION_MAPPER: dict[str, EventFunction] = { "Note Hook MergeRequest": get_commented_merge_request_event_body, "Note Hook Issue": get_commented_issue_event_body, "Confidential Note Hook Issue": get_commented_issue_event_body, + "Note Hook DesignManagement::Design": get_commented_design_event_body, + "Confidential Note Hook DesignManagement::Design": get_commented_design_event_body, "Note Hook Snippet": get_commented_snippet_event_body, "Merge Request Hook approved": partial(get_merge_request_event_body, "approved"), "Merge Request Hook unapproved": partial(get_merge_request_event_body, "unapproved"), @@ -469,7 +521,9 @@ def api_gitlab_webhook( body = f"[{project_url}] {body}" topic_name = get_topic_based_on_event(event, payload, use_merge_request_title) - check_send_webhook_message(request, user_profile, topic_name, body, event) + check_send_webhook_message( + request, user_profile, topic_name, body, event, no_previews=skip_previews(event) + ) return json_success(request) @@ -514,6 +568,18 @@ def get_topic_based_on_event(event: str, payload: WildValue, use_merge_request_t id=payload["issue"]["iid"].tame(check_int), title=payload["issue"]["title"].tame(check_string), ) + elif event in ( + "Note Hook DesignManagement::Design", + "Confidential Note Hook DesignManagement::Design", + ): + design_path = payload["object_attributes"]["position"]["new_path"].tame(check_string) + design_name = design_path.split("/")[-1] + + return TOPIC_WITH_DESIGN_INFO_TEMPLATE.format( + repo=get_repo_name(payload), + type="design", + design_name=design_name, + ) elif event == "Note Hook MergeRequest": return TOPIC_WITH_PR_OR_ISSUE_INFO_TEMPLATE.format( repo=get_repo_name(payload),