diff --git a/api/integrations/gitlab/mappers.py b/api/integrations/gitlab/mappers.py index 1567e0f3ce08..8bd4467096a2 100644 --- a/api/integrations/gitlab/mappers.py +++ b/api/integrations/gitlab/mappers.py @@ -88,3 +88,29 @@ def map_gitlab_webhook_payload_to_tag_label( is_draft=bool(attrs.get("draft") or attrs.get("work_in_progress")), ) return None + + +def map_gitlab_webhook_payload_to_resource_metadata( + payload: GitLabWebhookPayload, +) -> GitLabResourceMetadata: + attrs = payload.get("object_attributes") or {} + object_kind = payload.get("object_kind") + state = attrs.get("state") + title = attrs.get("title") + metadata: GitLabResourceMetadata = {} + if object_kind == "issue" and state: + metadata["state"] = state + elif object_kind == "merge_request": + if attrs.get("action") == "merge": + metadata["state"] = "merged" + elif state: + metadata["state"] = state + if "draft" in attrs or "work_in_progress" in attrs: + metadata["draft"] = bool( + attrs.get("draft") or attrs.get("work_in_progress") + ) + else: + return metadata + if title: + metadata["title"] = title + return metadata diff --git a/api/integrations/gitlab/services/__init__.py b/api/integrations/gitlab/services/__init__.py index 4e0e4247634b..d7a7f899a034 100644 --- a/api/integrations/gitlab/services/__init__.py +++ b/api/integrations/gitlab/services/__init__.py @@ -9,6 +9,7 @@ GITLAB_RESOURCE_KIND_BY_TYPE, apply_flagsmith_label_to_resource, ) +from integrations.gitlab.services.metadata import update_resource_metadata from integrations.gitlab.services.tagging import ( apply_initial_tag, apply_tag_for_event, @@ -46,4 +47,5 @@ "post_unlinked_comment", "register_gitlab_webhook_for_resource", "set_gitlab_tag", + "update_resource_metadata", ] diff --git a/api/integrations/gitlab/services/metadata.py b/api/integrations/gitlab/services/metadata.py new file mode 100644 index 000000000000..f3c4a5f45159 --- /dev/null +++ b/api/integrations/gitlab/services/metadata.py @@ -0,0 +1,45 @@ +import json + +from features.feature_external_resources.models import ( + FeatureExternalResource, + ResourceType, +) +from integrations.gitlab.mappers import ( + map_gitlab_webhook_payload_to_resource_metadata, + map_resource_url_to_filter_value, +) +from integrations.gitlab.models import GitLabWebhook +from integrations.gitlab.types import GitLabResourceMetadata, GitLabWebhookPayload + +_RESOURCE_TYPE_BY_OBJECT_KIND: dict[str, str] = { + "issue": ResourceType.GITLAB_ISSUE.value, + "merge_request": ResourceType.GITLAB_MR.value, +} + + +def update_resource_metadata( + webhook: GitLabWebhook, + payload: GitLabWebhookPayload, +) -> None: + new_fields = map_gitlab_webhook_payload_to_resource_metadata(payload) + resource_type = _RESOURCE_TYPE_BY_OBJECT_KIND.get(payload.get("object_kind") or "") + resource_url = (payload.get("object_attributes") or {}).get("url") + if not (new_fields and resource_type and resource_url): + return + + resources = FeatureExternalResource.objects.filter( + feature__project=webhook.gitlab_configuration.project, + type=resource_type, + url__in=map_resource_url_to_filter_value(resource_url), + ) + + for resource in resources: + current = json.loads(resource.metadata) if resource.metadata else {} + merged: GitLabResourceMetadata = {**current, **new_fields} + changed = sorted( + name for name, value in new_fields.items() if current.get(name) != value + ) + if not changed: + continue + resource.metadata = json.dumps(merged) + resource.save(update_fields=["metadata"]) diff --git a/api/integrations/gitlab/services/tagging.py b/api/integrations/gitlab/services/tagging.py index f228072906be..c485302d131f 100644 --- a/api/integrations/gitlab/services/tagging.py +++ b/api/integrations/gitlab/services/tagging.py @@ -1,5 +1,3 @@ -import structlog - from features.feature_external_resources.models import ( GITLAB_RESOURCE_TYPES, FeatureExternalResource, @@ -21,8 +19,6 @@ from integrations.gitlab.types import GitLabWebhookPayload from projects.tags.models import Tag, TagType -logger = structlog.get_logger("gitlab") - def set_gitlab_tag(feature: Feature, new_label: GitLabTagLabel) -> None: """Apply a GitLab system tag to ``feature``, replacing any existing GitLab @@ -67,9 +63,7 @@ def apply_tag_for_event( if not (label := map_gitlab_webhook_payload_to_tag_label(payload)): return - if not ( - resource_url := (attrs := payload.get("object_attributes") or {}).get("url") - ): + if not (resource_url := (payload.get("object_attributes") or {}).get("url")): return if not ( @@ -82,17 +76,6 @@ def apply_tag_for_event( return set_gitlab_tag(feature, label) - log = logger.bind( - organisation__id=webhook.gitlab_configuration.project.organisation_id, - project__id=webhook.gitlab_configuration.project_id, - ) - log.info( - "feature.tagged", - feature__id=feature.id, - tag__label=label.value, - object_kind=payload.get("object_kind"), - action=attrs.get("action"), - ) def clear_tag_for_resource(resource: FeatureExternalResource) -> None: diff --git a/api/integrations/gitlab/types.py b/api/integrations/gitlab/types.py index 34fa443cd812..a84a0c92bb18 100644 --- a/api/integrations/gitlab/types.py +++ b/api/integrations/gitlab/types.py @@ -18,6 +18,7 @@ class GitLabWebhookObjectAttributes(TypedDict, total=False): action: str draft: bool work_in_progress: bool + title: str class GitLabWebhookPayload(TypedDict, total=False): diff --git a/api/integrations/gitlab/views/webhook.py b/api/integrations/gitlab/views/webhook.py index 3386175be1ed..0909c1824085 100644 --- a/api/integrations/gitlab/views/webhook.py +++ b/api/integrations/gitlab/views/webhook.py @@ -9,7 +9,7 @@ from rest_framework.response import Response from integrations.gitlab.models import GitLabWebhook -from integrations.gitlab.services import apply_tag_for_event +from integrations.gitlab.services import apply_tag_for_event, update_resource_metadata from integrations.gitlab.types import GitLabWebhookPayload logger = structlog.get_logger("gitlab") @@ -29,8 +29,14 @@ def gitlab_webhook(request: Request, webhook_uuid: str) -> Response: if not hmac.compare_digest(token, webhook.secret): return Response(status=status.HTTP_401_UNAUTHORIZED) - apply_tag_for_event( - webhook=webhook, - payload=cast(GitLabWebhookPayload, request.data), + payload = cast(GitLabWebhookPayload, request.data) + apply_tag_for_event(webhook=webhook, payload=payload) + update_resource_metadata(webhook=webhook, payload=payload) + logger.info( + "webhook.processed", + organisation__id=webhook.gitlab_configuration.project.organisation_id, + project__id=webhook.gitlab_configuration.project_id, + object_kind=payload.get("object_kind"), + action=(payload.get("object_attributes") or {}).get("action"), ) return Response(status=status.HTTP_200_OK) diff --git a/api/tests/unit/integrations/gitlab/conftest.py b/api/tests/unit/integrations/gitlab/conftest.py index ddab787b9c93..278d15b7f8bd 100644 --- a/api/tests/unit/integrations/gitlab/conftest.py +++ b/api/tests/unit/integrations/gitlab/conftest.py @@ -1,10 +1,11 @@ from __future__ import annotations +import uuid from typing import TYPE_CHECKING import pytest -from integrations.gitlab.models import GitLabConfiguration +from integrations.gitlab.models import GitLabConfiguration, GitLabWebhook if TYPE_CHECKING: from projects.models import Project @@ -17,3 +18,15 @@ def gitlab_config(project: Project) -> GitLabConfiguration: gitlab_instance_url="https://gitlab.example.com", access_token="glpat-test-token", ) + + +@pytest.fixture() +def gitlab_webhook(gitlab_config: GitLabConfiguration) -> GitLabWebhook: + return GitLabWebhook.objects.create( # type: ignore[no-any-return] + uuid=uuid.uuid4(), + gitlab_configuration=gitlab_config, + gitlab_project_id=42, + gitlab_path_with_namespace="testorg/testrepo", + gitlab_hook_id=1, + secret="topsecret", + ) diff --git a/api/tests/unit/integrations/gitlab/test_metadata.py b/api/tests/unit/integrations/gitlab/test_metadata.py new file mode 100644 index 000000000000..fb43c56fead9 --- /dev/null +++ b/api/tests/unit/integrations/gitlab/test_metadata.py @@ -0,0 +1,290 @@ +import json + +import pytest + +from features.feature_external_resources.models import ( + FeatureExternalResource, + ResourceType, +) +from features.models import Feature +from integrations.gitlab.models import GitLabWebhook +from integrations.gitlab.services.metadata import update_resource_metadata + + +@pytest.mark.django_db +def test_update_resource_metadata__issue_state_changed__updates_metadata( + feature: Feature, + gitlab_webhook: GitLabWebhook, +) -> None: + # Given + resource = FeatureExternalResource.objects.create( + feature=feature, + url="https://gitlab.example.com/testorg/testrepo/-/issues/1", + type=ResourceType.GITLAB_ISSUE.value, + metadata='{"state": "opened", "title": "Bug"}', + ) + + # When + update_resource_metadata( + webhook=gitlab_webhook, + payload={ + "object_kind": "issue", + "object_attributes": { + "url": "https://gitlab.example.com/testorg/testrepo/-/issues/1", + "state": "closed", + "action": "close", + "title": "Bug", + }, + }, + ) + + # Then + resource.refresh_from_db() + assert resource.metadata is not None + assert json.loads(resource.metadata) == {"state": "closed", "title": "Bug"} + + +@pytest.mark.django_db +def test_update_resource_metadata__issue_title_changed__updates_metadata( + feature: Feature, + gitlab_webhook: GitLabWebhook, +) -> None: + # Given + resource = FeatureExternalResource.objects.create( + feature=feature, + url="https://gitlab.example.com/testorg/testrepo/-/issues/1", + type=ResourceType.GITLAB_ISSUE.value, + metadata='{"state": "opened", "title": "Old name"}', + ) + + # When + update_resource_metadata( + webhook=gitlab_webhook, + payload={ + "object_kind": "issue", + "object_attributes": { + "url": "https://gitlab.example.com/testorg/testrepo/-/issues/1", + "state": "opened", + "title": "New name", + }, + }, + ) + + # Then + resource.refresh_from_db() + assert resource.metadata is not None + assert json.loads(resource.metadata) == {"state": "opened", "title": "New name"} + + +@pytest.mark.django_db +def test_update_resource_metadata__work_item_url_matches_legacy_issue__updates( + feature: Feature, + gitlab_webhook: GitLabWebhook, +) -> None: + # Given + resource = FeatureExternalResource.objects.create( + feature=feature, + url="https://gitlab.example.com/testorg/testrepo/-/issues/1", + type=ResourceType.GITLAB_ISSUE.value, + metadata='{"state": "opened"}', + ) + + # When + update_resource_metadata( + webhook=gitlab_webhook, + payload={ + "object_kind": "issue", + "object_attributes": { + "url": "https://gitlab.example.com/testorg/testrepo/-/work_items/1", + "state": "closed", + }, + }, + ) + + # Then + resource.refresh_from_db() + assert resource.metadata is not None + assert json.loads(resource.metadata) == {"state": "closed"} + + +@pytest.mark.django_db +def test_update_resource_metadata__merge_request_action_merge__sets_merged_state( + feature: Feature, + gitlab_webhook: GitLabWebhook, +) -> None: + # Given + resource = FeatureExternalResource.objects.create( + feature=feature, + url="https://gitlab.example.com/testorg/testrepo/-/merge_requests/7", + type=ResourceType.GITLAB_MR.value, + metadata='{"state": "opened", "title": "MR", "draft": false}', + ) + + # When + update_resource_metadata( + webhook=gitlab_webhook, + payload={ + "object_kind": "merge_request", + "object_attributes": { + "url": "https://gitlab.example.com/testorg/testrepo/-/merge_requests/7", + "state": "opened", + "action": "merge", + "draft": False, + }, + }, + ) + + # Then + resource.refresh_from_db() + assert resource.metadata is not None + assert json.loads(resource.metadata) == { + "state": "merged", + "title": "MR", + "draft": False, + } + + +@pytest.mark.django_db +def test_update_resource_metadata__merge_request_draft_toggled__updates_draft( + feature: Feature, + gitlab_webhook: GitLabWebhook, +) -> None: + # Given + resource = FeatureExternalResource.objects.create( + feature=feature, + url="https://gitlab.example.com/testorg/testrepo/-/merge_requests/7", + type=ResourceType.GITLAB_MR.value, + metadata='{"state": "opened", "draft": true}', + ) + + # When + update_resource_metadata( + webhook=gitlab_webhook, + payload={ + "object_kind": "merge_request", + "object_attributes": { + "url": "https://gitlab.example.com/testorg/testrepo/-/merge_requests/7", + "state": "opened", + "draft": False, + }, + }, + ) + + # Then + resource.refresh_from_db() + assert resource.metadata is not None + assert json.loads(resource.metadata) == {"state": "opened", "draft": False} + + +@pytest.mark.django_db +def test_update_resource_metadata__merge_request_work_in_progress_only__updates_draft( + feature: Feature, + gitlab_webhook: GitLabWebhook, +) -> None: + # Given + resource = FeatureExternalResource.objects.create( + feature=feature, + url="https://gitlab.example.com/testorg/testrepo/-/merge_requests/7", + type=ResourceType.GITLAB_MR.value, + metadata='{"state": "opened", "draft": false}', + ) + + # When + update_resource_metadata( + webhook=gitlab_webhook, + payload={ + "object_kind": "merge_request", + "object_attributes": { + "url": "https://gitlab.example.com/testorg/testrepo/-/merge_requests/7", + "state": "opened", + "work_in_progress": True, + }, + }, + ) + + # Then + resource.refresh_from_db() + assert resource.metadata is not None + assert json.loads(resource.metadata) == {"state": "opened", "draft": True} + + +@pytest.mark.django_db +def test_update_resource_metadata__nothing_changed__skips_write( + feature: Feature, + gitlab_webhook: GitLabWebhook, +) -> None: + # Given + resource = FeatureExternalResource.objects.create( + feature=feature, + url="https://gitlab.example.com/testorg/testrepo/-/issues/1", + type=ResourceType.GITLAB_ISSUE.value, + metadata='{"state": "opened"}', + ) + + # When + update_resource_metadata( + webhook=gitlab_webhook, + payload={ + "object_kind": "issue", + "object_attributes": { + "url": "https://gitlab.example.com/testorg/testrepo/-/issues/1", + "state": "opened", + }, + }, + ) + + # Then + resource.refresh_from_db() + assert resource.metadata is not None + assert json.loads(resource.metadata) == {"state": "opened"} + + +@pytest.mark.django_db +def test_update_resource_metadata__no_linked_resource__noop( + gitlab_webhook: GitLabWebhook, +) -> None: + # Given / When + update_resource_metadata( + webhook=gitlab_webhook, + payload={ + "object_kind": "issue", + "object_attributes": { + "url": "https://gitlab.example.com/testorg/testrepo/-/issues/999", + "state": "closed", + }, + }, + ) + + # Then — no exception raised + assert not FeatureExternalResource.objects.exists() + + +@pytest.mark.django_db +def test_update_resource_metadata__unsupported_object_kind__noop( + feature: Feature, + gitlab_webhook: GitLabWebhook, +) -> None: + # Given + resource = FeatureExternalResource.objects.create( + feature=feature, + url="https://gitlab.example.com/testorg/testrepo/-/issues/1", + type=ResourceType.GITLAB_ISSUE.value, + metadata='{"state": "opened"}', + ) + + # When + update_resource_metadata( + webhook=gitlab_webhook, + payload={ + "object_kind": "push", + "object_attributes": { + "url": "https://gitlab.example.com/testorg/testrepo/-/issues/1", + "state": "closed", + }, + }, + ) + + # Then + resource.refresh_from_db() + assert resource.metadata is not None + assert json.loads(resource.metadata) == {"state": "opened"} diff --git a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md index 9b00b62aca76..dee644229814 100644 --- a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md +++ b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md @@ -163,19 +163,6 @@ Attributes: - `organisation.id` - `project.id` -### `gitlab.feature.tagged` - -Logged at `info` from: - - `api/integrations/gitlab/services/tagging.py:89` - -Attributes: - - `action` - - `feature.id` - - `object_kind` - - `organisation.id` - - `project.id` - - `tag.label` - ### `gitlab.label.created` Logged at `info` from: @@ -271,6 +258,17 @@ Attributes: - `organisation.id` - `project.id` +### `gitlab.webhook.processed` + +Logged at `info` from: + - `api/integrations/gitlab/views/webhook.py:35` + +Attributes: + - `action` + - `object_kind` + - `organisation.id` + - `project.id` + ### `gitlab.webhook.registered` Logged at `info` from: