diff --git a/api/integrations/gitlab/client/__init__.py b/api/integrations/gitlab/client/__init__.py index d08d2fb464b8..d60b0d0fdd43 100644 --- a/api/integrations/gitlab/client/__init__.py +++ b/api/integrations/gitlab/client/__init__.py @@ -1,9 +1,13 @@ from integrations.gitlab.client.api import ( + GitLabResourceKind, + add_flagsmith_label_to_gitlab_resource, + create_flagsmith_label, create_issue_note, create_merge_request_note, create_project_hook, delete_project_hook, fetch_gitlab_projects, + remove_flagsmith_label_from_gitlab_resource, search_gitlab_issues, search_gitlab_merge_requests, ) @@ -21,11 +25,15 @@ "GitLabPage", "GitLabProject", "GitLabProjectHook", + "GitLabResourceKind", + "add_flagsmith_label_to_gitlab_resource", + "create_flagsmith_label", "create_issue_note", "create_merge_request_note", "create_project_hook", "delete_project_hook", "fetch_gitlab_projects", + "remove_flagsmith_label_from_gitlab_resource", "search_gitlab_issues", "search_gitlab_merge_requests", ] diff --git a/api/integrations/gitlab/client/api.py b/api/integrations/gitlab/client/api.py index 9f5db9a9b982..da6e0fdef892 100644 --- a/api/integrations/gitlab/client/api.py +++ b/api/integrations/gitlab/client/api.py @@ -1,5 +1,5 @@ from collections.abc import Mapping -from typing import Any +from typing import Any, Literal from urllib.parse import quote import requests @@ -12,7 +12,14 @@ GitLabProjectHook, T, ) -from integrations.gitlab.constants import GITLAB_CLIENT_TIMEOUT_SECONDS +from integrations.gitlab.constants import ( + GITLAB_CLIENT_TIMEOUT_SECONDS, + GITLAB_FLAGSMITH_LABEL, + GITLAB_FLAGSMITH_LABEL_COLOUR, + GITLAB_FLAGSMITH_LABEL_DESCRIPTION, +) + +GitLabResourceKind = Literal["issues", "merge_requests"] def _get_from_gitlab_api( @@ -26,6 +33,7 @@ def _get_from_gitlab_api( f"{instance_url}/api/v4/{path}", headers={"PRIVATE-TOKEN": access_token}, params=params, + timeout=GITLAB_CLIENT_TIMEOUT_SECONDS, ) response.raise_for_status() return response @@ -171,6 +179,7 @@ def create_project_hook( "merge_requests_events": True, "enable_ssl_verification": True, }, + timeout=GITLAB_CLIENT_TIMEOUT_SECONDS, ) response.raise_for_status() payload = response.json() @@ -187,6 +196,7 @@ def delete_project_hook( response = requests.delete( f"{instance_url}/api/v4/projects/{project_id}/hooks/{hook_id}", headers={"PRIVATE-TOKEN": access_token}, + timeout=GITLAB_CLIENT_TIMEOUT_SECONDS, ) if response.status_code == 404: return @@ -227,3 +237,71 @@ def create_merge_request_note( timeout=GITLAB_CLIENT_TIMEOUT_SECONDS, ) response.raise_for_status() + + +def create_flagsmith_label( + instance_url: str, + access_token: str, + *, + project_path: str, +) -> bool: + """Create the "Flagsmith Feature" label on a GitLab project. + + Returns True if the label was created, False if it already existed. + """ + encoded_path = quote(project_path, safe="") + try: + response = requests.post( + f"{instance_url}/api/v4/projects/{encoded_path}/labels", + headers={"PRIVATE-TOKEN": access_token}, + json={ + "name": GITLAB_FLAGSMITH_LABEL, + "color": GITLAB_FLAGSMITH_LABEL_COLOUR, + "description": GITLAB_FLAGSMITH_LABEL_DESCRIPTION, + }, + timeout=GITLAB_CLIENT_TIMEOUT_SECONDS, + ) + response.raise_for_status() + except requests.HTTPError as exc: + if exc.response is not None and exc.response.status_code == 409: + return False + raise + return True + + +def add_flagsmith_label_to_gitlab_resource( + instance_url: str, + access_token: str, + *, + project_path: str, + resource_kind: GitLabResourceKind, + resource_iid: int, +) -> None: + """Apply the "Flagsmith Feature" label to a GitLab issue or MR, additively.""" + encoded_path = quote(project_path, safe="") + response = requests.put( + f"{instance_url}/api/v4/projects/{encoded_path}/{resource_kind}/{resource_iid}", + headers={"PRIVATE-TOKEN": access_token}, + json={"add_labels": GITLAB_FLAGSMITH_LABEL}, + timeout=GITLAB_CLIENT_TIMEOUT_SECONDS, + ) + response.raise_for_status() + + +def remove_flagsmith_label_from_gitlab_resource( + instance_url: str, + access_token: str, + *, + project_path: str, + resource_kind: GitLabResourceKind, + resource_iid: int, +) -> None: + """Remove the "Flagsmith Feature" label from a GitLab issue or MR.""" + encoded_path = quote(project_path, safe="") + response = requests.put( + f"{instance_url}/api/v4/projects/{encoded_path}/{resource_kind}/{resource_iid}", + headers={"PRIVATE-TOKEN": access_token}, + json={"remove_labels": GITLAB_FLAGSMITH_LABEL}, + timeout=GITLAB_CLIENT_TIMEOUT_SECONDS, + ) + response.raise_for_status() diff --git a/api/integrations/gitlab/constants.py b/api/integrations/gitlab/constants.py index 8dd1f33a92fa..812417005b62 100644 --- a/api/integrations/gitlab/constants.py +++ b/api/integrations/gitlab/constants.py @@ -5,6 +5,12 @@ GITLAB_TAG_COLOR = "#FC6D26" GITLAB_CLIENT_TIMEOUT_SECONDS = 10 +GITLAB_FLAGSMITH_LABEL = "Flagsmith Feature" +GITLAB_FLAGSMITH_LABEL_COLOUR = "#6633FF" +GITLAB_FLAGSMITH_LABEL_DESCRIPTION = ( + "This GitLab Issue/MR is linked to a Flagsmith feature" +) + class GitLabTagLabel(Enum): ISSUE_OPEN = "Issue Open" diff --git a/api/integrations/gitlab/migrations/0003_gitlabconfiguration_labeling_enabled.py b/api/integrations/gitlab/migrations/0003_gitlabconfiguration_labeling_enabled.py new file mode 100644 index 000000000000..98cbf4ad2507 --- /dev/null +++ b/api/integrations/gitlab/migrations/0003_gitlabconfiguration_labeling_enabled.py @@ -0,0 +1,16 @@ +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("gitlab", "0002_add_gitlab_webhook_model"), + ] + + operations = [ + migrations.AddField( + model_name="gitlabconfiguration", + name="labeling_enabled", + field=models.BooleanField(default=False), + ), + ] diff --git a/api/integrations/gitlab/models.py b/api/integrations/gitlab/models.py index c60367dc37c3..1d4708f7f209 100644 --- a/api/integrations/gitlab/models.py +++ b/api/integrations/gitlab/models.py @@ -11,6 +11,7 @@ class GitLabConfiguration(SoftDeleteExportableModel): ) gitlab_instance_url = models.URLField(max_length=200) access_token = models.CharField(max_length=300) + labeling_enabled = models.BooleanField(default=False) class GitLabWebhook(SoftDeleteExportableModel): diff --git a/api/integrations/gitlab/serializers.py b/api/integrations/gitlab/serializers.py index f6e3ba0f7f64..0c0297d29fb0 100644 --- a/api/integrations/gitlab/serializers.py +++ b/api/integrations/gitlab/serializers.py @@ -11,7 +11,7 @@ class GitLabConfigurationSerializer(BaseProjectIntegrationModelSerializer): class Meta: model = GitLabConfiguration - fields = ("id", "gitlab_instance_url", "access_token") + fields = ("id", "gitlab_instance_url", "access_token", "labeling_enabled") def to_representation(self, instance: GitLabConfiguration) -> dict[str, Any]: data = super().to_representation(instance) diff --git a/api/integrations/gitlab/services/__init__.py b/api/integrations/gitlab/services/__init__.py index 95cb178131c3..4e0e4247634b 100644 --- a/api/integrations/gitlab/services/__init__.py +++ b/api/integrations/gitlab/services/__init__.py @@ -5,6 +5,10 @@ post_state_change_comment, post_unlinked_comment, ) +from integrations.gitlab.services.labels import ( + GITLAB_RESOURCE_KIND_BY_TYPE, + apply_flagsmith_label_to_resource, +) from integrations.gitlab.services.tagging import ( apply_initial_tag, apply_tag_for_event, @@ -24,6 +28,8 @@ ) __all__ = [ + "GITLAB_RESOURCE_KIND_BY_TYPE", + "apply_flagsmith_label_to_resource", "apply_initial_tag", "apply_tag_for_event", "clear_tag_for_resource", diff --git a/api/integrations/gitlab/services/labels.py b/api/integrations/gitlab/services/labels.py new file mode 100644 index 000000000000..642e40944e3d --- /dev/null +++ b/api/integrations/gitlab/services/labels.py @@ -0,0 +1,76 @@ +from typing import Literal + +import requests +import structlog + +from features.feature_external_resources.models import ( + FeatureExternalResource, + ResourceType, +) +from integrations.gitlab.client import ( + add_flagsmith_label_to_gitlab_resource, + create_flagsmith_label, +) +from integrations.gitlab.models import GitLabConfiguration +from integrations.gitlab.services.url_parsing import ( + parse_project_path, + parse_resource_iid, +) + +logger = structlog.get_logger("gitlab") + +GitLabResourceKind = Literal["issues", "merge_requests"] + +GITLAB_RESOURCE_KIND_BY_TYPE: dict[str, GitLabResourceKind] = { + ResourceType.GITLAB_ISSUE.value: "issues", + ResourceType.GITLAB_MR.value: "merge_requests", +} + + +def apply_flagsmith_label_to_resource( + resource: FeatureExternalResource, +) -> None: + """Ensure the "Flagsmith Feature" label exists on the resource's GitLab + project and apply it to the resource. No-op if labelling is disabled, + unconfigured, or the URL is unparseable. Never raises — failures are + logged via ``label.failed``. + """ + project = resource.feature.project + config: GitLabConfiguration | None = GitLabConfiguration.objects.filter( + project=project + ).first() + if not config or not config.labeling_enabled: + return + + path_with_namespace = parse_project_path(resource.url) + resource_iid = parse_resource_iid(resource.url) + if path_with_namespace is None or resource_iid is None: + return + + log = logger.bind( + organisation__id=project.organisation_id, + project__id=project.id, + feature__id=resource.feature_id, + gitlab_project__path=path_with_namespace, + resource__type=resource.type, + resource__iid=resource_iid, + ) + + try: + created = create_flagsmith_label( + config.gitlab_instance_url, + config.access_token, + project_path=path_with_namespace, + ) + if created: + log.info("label.created") + + add_flagsmith_label_to_gitlab_resource( + config.gitlab_instance_url, + config.access_token, + project_path=path_with_namespace, + resource_kind=GITLAB_RESOURCE_KIND_BY_TYPE[resource.type], + resource_iid=resource_iid, + ) + except requests.RequestException: + log.exception("label.failed") diff --git a/api/integrations/gitlab/tasks.py b/api/integrations/gitlab/tasks.py index c41d1f2975ad..352675c21ebc 100644 --- a/api/integrations/gitlab/tasks.py +++ b/api/integrations/gitlab/tasks.py @@ -1,7 +1,10 @@ +import requests +import structlog from task_processor.decorators import register_task_handler from features.feature_external_resources.models import FeatureExternalResource from features.models import FeatureState +from integrations.gitlab.client import remove_flagsmith_label_from_gitlab_resource from integrations.gitlab.models import GitLabConfiguration from integrations.gitlab.services import ( deregister_webhook_for_path, @@ -12,6 +15,16 @@ post_state_change_comment, post_unlinked_comment, ) +from integrations.gitlab.services.labels import ( + GITLAB_RESOURCE_KIND_BY_TYPE, + apply_flagsmith_label_to_resource, +) +from integrations.gitlab.services.url_parsing import ( + parse_project_path, + parse_resource_iid, +) + +logger = structlog.get_logger("gitlab") @register_task_handler() @@ -108,3 +121,65 @@ def post_gitlab_feature_deleted_comment( feature_id=feature_id, project_id=project_id, ) + + +@register_task_handler() +def apply_gitlab_label(resource_id: int) -> None: + """Apply the "Flagsmith Feature" label to the linked GitLab resource. + Dispatched at link time. No-op if labelling is disabled or unconfigured. + """ + try: + resource = FeatureExternalResource.objects.get(id=resource_id) + except FeatureExternalResource.DoesNotExist: + return + apply_flagsmith_label_to_resource(resource) + + +@register_task_handler() +def remove_gitlab_label( + *, + project_id: int, + feature_id: int, + resource_pk: int, + resource_url: str, + resource_type: str, +) -> None: + """Remove the "Flagsmith Feature" label from a GitLab issue/MR. + No-op if another FeatureExternalResource still references the same URL. + """ + config: GitLabConfiguration | None = GitLabConfiguration.objects.filter( + project_id=project_id + ).first() + if not config or not config.labeling_enabled: + return + if ( + FeatureExternalResource.objects.filter(url=resource_url) + .exclude(pk=resource_pk) + .exists() + ): + return + + path_with_namespace = parse_project_path(resource_url) + resource_iid = parse_resource_iid(resource_url) + if path_with_namespace is None or resource_iid is None: + return + + log = logger.bind( + project__id=project_id, + feature__id=feature_id, + gitlab_project__path=path_with_namespace, + resource__type=resource_type, + resource__iid=resource_iid, + ) + + try: + remove_flagsmith_label_from_gitlab_resource( + config.gitlab_instance_url, + config.access_token, + project_path=path_with_namespace, + resource_kind=GITLAB_RESOURCE_KIND_BY_TYPE[resource_type], + resource_iid=resource_iid, + ) + log.info("label.removed") + except requests.RequestException: + log.exception("label.removal_failed") diff --git a/api/integrations/vcs/services.py b/api/integrations/vcs/services.py index 995635e8b7ef..f40638b96a3c 100644 --- a/api/integrations/vcs/services.py +++ b/api/integrations/vcs/services.py @@ -23,6 +23,7 @@ def dispatch_vcs_on_resource_create(resource: FeatureExternalResource) -> None: if resource.type in GITLAB_RESOURCE_TYPES: gitlab.register_gitlab_webhook_for_resource(resource) gitlab.apply_initial_tag(resource) + gitlab_tasks.apply_gitlab_label.delay(args=(resource.id,)) gitlab_tasks.post_gitlab_linked_comment.delay(args=(resource.id,)) gitlab_logger.info( "resource.linked", @@ -38,6 +39,15 @@ def dispatch_vcs_on_resource_destroy(resource: FeatureExternalResource) -> None: has been destroyed. `resource` is a memory-only object at this point. """ if resource.type in GITLAB_RESOURCE_TYPES: + gitlab_tasks.remove_gitlab_label.delay( + kwargs={ + "project_id": resource.feature.project_id, + "feature_id": resource.feature_id, + "resource_pk": resource.pk, + "resource_url": resource.url, + "resource_type": resource.type, + }, + ) gitlab_tasks.post_gitlab_unlinked_comment.delay( args=( resource.feature.name, diff --git a/api/tests/integration/features/test_gitlab_external_resources.py b/api/tests/integration/features/test_gitlab_external_resources.py index 816daa80a2f0..c1ed8a9e2a73 100644 --- a/api/tests/integration/features/test_gitlab_external_resources.py +++ b/api/tests/integration/features/test_gitlab_external_resources.py @@ -580,6 +580,190 @@ def test_list_external_resources__gitlab_issue__returns_200( assert results[0]["metadata"] == {"title": "Fix login bug", "state": "opened"} +@responses.activate +def test_create_external_resource__gitlab_issue_with_labeling__applies_label( + admin_client: APIClient, + organisation: int, + project: int, + feature: int, + log: StructuredLogCapture, +) -> None: + # Given + project_instance = Project.objects.get(id=project) + GitLabConfiguration.objects.create( + project=project_instance, + gitlab_instance_url="https://gitlab.example.com", + access_token="glpat-test-token", + labeling_enabled=True, + ) + responses.post( + "https://gitlab.example.com/api/v4/projects/testorg%2Ftestrepo/labels", + json={"name": "Flagsmith Feature"}, + status=201, + ) + responses.put( + "https://gitlab.example.com/api/v4/projects/testorg%2Ftestrepo/issues/42", + json={"iid": 42}, + status=200, + match=[ + responses.matchers.json_params_matcher( + {"add_labels": "Flagsmith Feature"}, + ), + ], + ) + responses.post( + "https://gitlab.example.com/api/v4/projects/testorg%2Ftestrepo/hooks", + json={"id": 1, "project_id": 1}, + status=201, + ) + responses.post( + "https://gitlab.example.com/api/v4/projects/testorg%2Ftestrepo/issues/42/notes", + json={"id": 1}, + status=201, + ) + + # When + response = admin_client.post( + f"/api/v1/projects/{project}/features/{feature}/feature-external-resources/", + data={ + "type": "GITLAB_ISSUE", + "url": "https://gitlab.example.com/testorg/testrepo/-/issues/42", + "feature": feature, + "metadata": {"title": "Bug fix", "state": "opened"}, + }, + format="json", + ) + + # Then + assert response.status_code == status.HTTP_201_CREATED + assert any(e["event"] == "label.created" for e in log.events) + assert any(e["event"] == "resource.linked" for e in log.events) + + +@responses.activate +def test_create_external_resource__gitlab_issue_with_labeling_disabled__skips_label_api( + admin_client: APIClient, + project: int, + feature: int, + log: StructuredLogCapture, +) -> None: + # Given + project_instance = Project.objects.get(id=project) + GitLabConfiguration.objects.create( + project=project_instance, + gitlab_instance_url="https://gitlab.example.com", + access_token="glpat-test-token", + labeling_enabled=False, + ) + responses.post( + "https://gitlab.example.com/api/v4/projects/testorg%2Ftestrepo/hooks", + json={"id": 1, "project_id": 1}, + status=201, + ) + responses.post( + "https://gitlab.example.com/api/v4/projects/testorg%2Ftestrepo/issues/42/notes", + json={"id": 1}, + status=201, + ) + + # When + response = admin_client.post( + f"/api/v1/projects/{project}/features/{feature}/feature-external-resources/", + data={ + "type": "GITLAB_ISSUE", + "url": "https://gitlab.example.com/testorg/testrepo/-/issues/42", + "feature": feature, + "metadata": {"title": "Bug fix", "state": "opened"}, + }, + format="json", + ) + + # Then — no label API called, resource still created. + assert response.status_code == status.HTTP_201_CREATED + assert not any(e["event"] == "label.created" for e in log.events) + + +@responses.activate +def test_create_external_resource__gitlab_issue_label_api_failure__still_creates_resource( + admin_client: APIClient, + project: int, + feature: int, + log: StructuredLogCapture, +) -> None: + # Given + project_instance = Project.objects.get(id=project) + GitLabConfiguration.objects.create( + project=project_instance, + gitlab_instance_url="https://gitlab.example.com", + access_token="glpat-test-token", + labeling_enabled=True, + ) + responses.post( + "https://gitlab.example.com/api/v4/projects/testorg%2Ftestrepo/labels", + status=403, + ) + responses.post( + "https://gitlab.example.com/api/v4/projects/testorg%2Ftestrepo/hooks", + json={"id": 1, "project_id": 1}, + status=201, + ) + responses.post( + "https://gitlab.example.com/api/v4/projects/testorg%2Ftestrepo/issues/42/notes", + json={"id": 1}, + status=201, + ) + + # When + response = admin_client.post( + f"/api/v1/projects/{project}/features/{feature}/feature-external-resources/", + data={ + "type": "GITLAB_ISSUE", + "url": "https://gitlab.example.com/testorg/testrepo/-/issues/42", + "feature": feature, + "metadata": {"title": "Bug fix", "state": "opened"}, + }, + format="json", + ) + + # Then — resource created, webhook registered, comment posted despite label failure. + assert response.status_code == status.HTTP_201_CREATED + assert FeatureExternalResource.objects.exists() + assert any(e["event"] == "label.failed" for e in log.events) + assert any(e["event"] == "webhook.registered" for e in log.events) + assert any(e["event"] == "comment.posted" for e in log.events) + assert any(e["event"] == "resource.linked" for e in log.events) + + +def test_create_external_resource__gitlab_issue_with_labeling_unparseable_url__still_creates_resource( + admin_client: APIClient, + project: int, + feature: int, +) -> None: + # Given + project_instance = Project.objects.get(id=project) + GitLabConfiguration.objects.create( + project=project_instance, + gitlab_instance_url="https://gitlab.example.com", + access_token="glpat-test-token", + labeling_enabled=True, + ) + + # When + response = admin_client.post( + f"/api/v1/projects/{project}/features/{feature}/feature-external-resources/", + data={ + "type": "GITLAB_ISSUE", + "url": "https://gitlab.example.com/not-a-resource", + "feature": feature, + }, + format="json", + ) + + # Then + assert response.status_code == status.HTTP_201_CREATED + assert FeatureExternalResource.objects.exists() + + def test_list_external_resources__gitlab_merge_request__returns_200( admin_client: APIClient, project: int, diff --git a/api/tests/unit/integrations/gitlab/test_client.py b/api/tests/unit/integrations/gitlab/test_client.py index 0c8e9df6fdda..c75828eeadf0 100644 --- a/api/tests/unit/integrations/gitlab/test_client.py +++ b/api/tests/unit/integrations/gitlab/test_client.py @@ -3,9 +3,11 @@ import responses from integrations.gitlab.client import ( + create_flagsmith_label, create_issue_note, create_merge_request_note, fetch_gitlab_projects, + remove_flagsmith_label_from_gitlab_resource, search_gitlab_issues, search_gitlab_merge_requests, ) @@ -316,3 +318,82 @@ def test_create_merge_request_note__server_error__raises() -> None: merge_request_iid=7, body="MR comment", ) + + +@responses.activate +def test_remove_flagsmith_label_from_gitlab_resource__gitlab_issue__puts_remove_labels() -> ( + None +): + # Given + responses.put( + f"{INSTANCE_URL}/api/v4/projects/g%2Fp/issues/42", + json={"iid": 42, "labels": []}, + status=200, + match=[ + responses.matchers.header_matcher({"PRIVATE-TOKEN": ACCESS_TOKEN}), + responses.matchers.json_params_matcher( + {"remove_labels": "Flagsmith Feature"}, + ), + ], + ) + + # When + remove_flagsmith_label_from_gitlab_resource( + INSTANCE_URL, + ACCESS_TOKEN, + project_path="g/p", + resource_kind="issues", + resource_iid=42, + ) + + # Then + assert len(responses.calls) == 1 + + +@responses.activate +def test_remove_flagsmith_label_from_gitlab_resource__gitlab_mr__puts_remove_labels() -> ( + None +): + # Given + responses.put( + f"{INSTANCE_URL}/api/v4/projects/g%2Fp/merge_requests/7", + json={"iid": 7, "labels": []}, + status=200, + match=[ + responses.matchers.header_matcher({"PRIVATE-TOKEN": ACCESS_TOKEN}), + responses.matchers.json_params_matcher( + {"remove_labels": "Flagsmith Feature"}, + ), + ], + ) + + # When + remove_flagsmith_label_from_gitlab_resource( + INSTANCE_URL, + ACCESS_TOKEN, + project_path="g/p", + resource_kind="merge_requests", + resource_iid=7, + ) + + # Then + assert len(responses.calls) == 1 + + +@responses.activate +def test_create_flagsmith_label__label_already_exists__returns_false() -> None: + # Given + responses.post( + f"{INSTANCE_URL}/api/v4/projects/g%2Fp/labels", + status=409, + ) + + # When + result = create_flagsmith_label( + INSTANCE_URL, + ACCESS_TOKEN, + project_path="g/p", + ) + + # Then + assert result is False diff --git a/api/tests/unit/integrations/gitlab/test_configuration.py b/api/tests/unit/integrations/gitlab/test_configuration.py index eb430983c89d..c66b77794eca 100644 --- a/api/tests/unit/integrations/gitlab/test_configuration.py +++ b/api/tests/unit/integrations/gitlab/test_configuration.py @@ -182,7 +182,6 @@ def test_delete_configuration__with_registered_webhooks__deregisters_each_and_cl assert len(deregister_events) == 2 assert {e["gitlab__hook__id"] for e in deregister_events} == {11, 22} - # Rows are cleared so a new config can register the same project afresh. assert not GitLabWebhook.objects.exists() @@ -225,7 +224,6 @@ def test_delete_configuration__gitlab_delete_fails_for_one__still_removes_config # Then assert response.status_code == status.HTTP_204_NO_CONTENT assert not GitLabConfiguration.objects.filter(project=project).exists() - # Second hook still deregistered; failure for the first was logged. assert any( e["event"] == "webhook.deregistered" and e["gitlab__hook__id"] == 22 for e in log.events diff --git a/api/tests/unit/integrations/gitlab/test_tasks.py b/api/tests/unit/integrations/gitlab/test_tasks.py index 5527c922ac27..8f5f8713595b 100644 --- a/api/tests/unit/integrations/gitlab/test_tasks.py +++ b/api/tests/unit/integrations/gitlab/test_tasks.py @@ -1,14 +1,27 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING + import pytest import pytest_mock +import responses +from pytest_structlog import StructuredLogCapture -from features.models import FeatureState +from features.feature_external_resources.models import FeatureExternalResource +from features.models import Feature, FeatureState +from integrations.gitlab.models import GitLabConfiguration from integrations.gitlab.tasks import ( + apply_gitlab_label, post_gitlab_feature_deleted_comment, post_gitlab_linked_comment, post_gitlab_state_change_comment, post_gitlab_unlinked_comment, + remove_gitlab_label, ) +if TYPE_CHECKING: + from projects.models import Project + @pytest.mark.django_db def test_post_gitlab_linked_comment_task__resource_missing__noop( @@ -109,3 +122,153 @@ def test_post_gitlab_feature_deleted_comment_task__called__delegates_to_service( feature_id=99, project_id=42, ) + + +@pytest.mark.django_db +def test_apply_gitlab_label__resource_missing__noop( + mocker: pytest_mock.MockerFixture, +) -> None: + # Given + mock_apply = mocker.patch( + "integrations.gitlab.tasks.apply_flagsmith_label_to_resource", + ) + + # When + apply_gitlab_label(resource_id=999_999) + + # Then + mock_apply.assert_not_called() + + +@pytest.mark.django_db +def test_apply_gitlab_label__resource_exists__calls_apply_flagsmith_label( + mocker: pytest_mock.MockerFixture, + feature: Feature, +) -> None: + # Given + resource = FeatureExternalResource.objects.create( + url="https://gitlab.example.com/testorg/testrepo/-/issues/1", + type="GITLAB_ISSUE", + feature=feature, + ) + mock_apply = mocker.patch( + "integrations.gitlab.tasks.apply_flagsmith_label_to_resource", + ) + + # When + apply_gitlab_label(resource_id=resource.id) + + # Then + mock_apply.assert_called_once() + [call_args] = mock_apply.call_args_list + assert call_args.args[0].id == resource.id + + +@pytest.mark.django_db +def test_remove_gitlab_label__unparseable_url__noop( + project: Project, +) -> None: + # Given + config = GitLabConfiguration.objects.create( + project=project, + gitlab_instance_url="https://gitlab.example.com", + access_token="glpat-test-token", + labeling_enabled=True, + ) + + # When + remove_gitlab_label( + project_id=config.project_id, + feature_id=0, + resource_pk=0, + resource_url="https://gitlab.example.com/not-a-resource", + resource_type="GITLAB_ISSUE", + ) + + # Then + assert GitLabConfiguration.objects.filter(project=project).exists() + + +@pytest.mark.django_db +def test_remove_gitlab_label__another_resource_exists__noop( + project: Project, + feature: Feature, +) -> None: + # Given + GitLabConfiguration.objects.create( + project=project, + gitlab_instance_url="https://gitlab.example.com", + access_token="glpat-test-token", + labeling_enabled=True, + ) + url = "https://gitlab.example.com/testorg/testrepo/-/issues/1" + resource = FeatureExternalResource.objects.create( + url=url, + type="GITLAB_ISSUE", + feature=feature, + ) + other = FeatureExternalResource.objects.create( + url=url, + type="GITLAB_ISSUE", + feature=Feature.objects.create( + name="other_flag", + project=project, + initial_value="", + ), + ) + + # When + remove_gitlab_label( + project_id=project.id, + feature_id=feature.id, + resource_pk=resource.pk, + resource_url=url, + resource_type="GITLAB_ISSUE", + ) + + # Then + assert FeatureExternalResource.objects.filter(pk=other.pk).exists() + + +@pytest.mark.django_db +@responses.activate +@pytest.mark.parametrize( + "api_status, expected_event", + [ + (200, "label.removed"), + (500, "label.removal_failed"), + ], + ids=["success", "api_failure"], +) +def test_remove_gitlab_label__last_resource__calls_api_and_logs( + project: Project, + feature: Feature, + log: StructuredLogCapture, + api_status: int, + expected_event: str, +) -> None: + # Given + GitLabConfiguration.objects.create( + project=project, + gitlab_instance_url="https://gitlab.example.com", + access_token="glpat-test-token", + labeling_enabled=True, + ) + responses.put( + "https://gitlab.example.com/api/v4/projects/testorg%2Ftestrepo/issues/1", + json={"iid": 1}, + status=api_status, + ) + + # When + remove_gitlab_label( + project_id=project.id, + feature_id=feature.id, + resource_pk=0, + resource_url="https://gitlab.example.com/testorg/testrepo/-/issues/1", + resource_type="GITLAB_ISSUE", + ) + + # Then + assert len(responses.calls) == 1 + assert any(e["event"] == expected_event for e in log.events) diff --git a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md index a2d3ba898f7b..6b8a9e53c8a8 100644 --- a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md +++ b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md @@ -133,10 +133,60 @@ Attributes: - `project.id` - `tag.label` +### `gitlab.label.created` + +Logged at `info` from: + - `api/integrations/gitlab/services/labels.py:66` + +Attributes: + - `feature.id` + - `gitlab_project.path` + - `organisation.id` + - `project.id` + - `resource.iid` + - `resource.type` + +### `gitlab.label.failed` + +Logged at `exception` from: + - `api/integrations/gitlab/services/labels.py:76` + +Attributes: + - `feature.id` + - `gitlab_project.path` + - `organisation.id` + - `project.id` + - `resource.iid` + - `resource.type` + +### `gitlab.label.removal_failed` + +Logged at `exception` from: + - `api/integrations/gitlab/tasks.py:185` + +Attributes: + - `feature.id` + - `gitlab_project.path` + - `project.id` + - `resource.iid` + - `resource.type` + +### `gitlab.label.removed` + +Logged at `info` from: + - `api/integrations/gitlab/tasks.py:183` + +Attributes: + - `feature.id` + - `gitlab_project.path` + - `project.id` + - `resource.iid` + - `resource.type` + ### `gitlab.resource.linked` Logged at `info` from: - - `api/integrations/vcs/services.py:27` + - `api/integrations/vcs/services.py:28` Attributes: - `feature.id` @@ -147,7 +197,7 @@ Attributes: ### `gitlab.resource.unlinked` Logged at `info` from: - - `api/integrations/vcs/services.py:53` + - `api/integrations/vcs/services.py:63` Attributes: - `feature.id` diff --git a/docs/docs/third-party-integrations/project-management/gitlab.md b/docs/docs/third-party-integrations/project-management/gitlab.md index 5b23fe3ff0e0..5a157db076e7 100644 --- a/docs/docs/third-party-integrations/project-management/gitlab.md +++ b/docs/docs/third-party-integrations/project-management/gitlab.md @@ -53,7 +53,7 @@ Flagsmith will post a comment to the linked issue or MR with the flag's current state across all environments. When the flag state changes, a new comment is posted automatically. -A **Flagsmith Flag** label is added to linked issues and merge requests so your +A **Flagsmith Feature** label is added to linked issues and merge requests so your team can filter for them in GitLab. ## Automatic state sync diff --git a/frontend/common/stores/default-flags.ts b/frontend/common/stores/default-flags.ts index cb8cb6f03fe0..1af253c52265 100644 --- a/frontend/common/stores/default-flags.ts +++ b/frontend/common/stores/default-flags.ts @@ -102,6 +102,12 @@ const defaultFlags = { 'key': 'access_token', 'label': 'Access Token', }, + { + 'default': false, + 'inputType': 'checkbox', + 'key': 'labeling_enabled', + 'label': 'Add "Flagsmith Feature" label to linked issues and MRs', + }, ], 'image': '/static/images/integrations/gitlab.svg', 'perEnvironment': false, diff --git a/frontend/web/components/ExternalResourcesTable.tsx b/frontend/web/components/ExternalResourcesTable.tsx index 3d3b16ecea68..f67027612c46 100644 --- a/frontend/web/components/ExternalResourcesTable.tsx +++ b/frontend/web/components/ExternalResourcesTable.tsx @@ -61,10 +61,11 @@ const ExternalResourceRow: FC = ({ - {`${ - externalResource?.metadata?.title - } (#${externalResource?.url.replace(/\D/g, '')})`}{' '} -
+ {externalResource?.metadata?.title} + + {`(#${externalResource?.url.replace(/\D/g, '')})`} + +
@@ -77,7 +78,7 @@ const ExternalResourceRow: FC = ({
-
+
{externalResource?.metadata?.state}
@@ -115,7 +116,7 @@ const ExternalResourceRow: FC = ({ }) }} > - Delete + Unlink
, diff --git a/frontend/web/components/modals/CreateEditIntegrationModal.tsx b/frontend/web/components/modals/CreateEditIntegrationModal.tsx index 5fa4ac7e0f75..d539e1eb0f12 100644 --- a/frontend/web/components/modals/CreateEditIntegrationModal.tsx +++ b/frontend/web/components/modals/CreateEditIntegrationModal.tsx @@ -105,6 +105,8 @@ const CreateEditIntegration: FC = (props) => { }, ) } + // Intentionally runs only on mount — Slack channel setup is a one-time fetch. + // eslint-disable-next-line react-hooks/exhaustive-deps }, []) const update = (key: string, e: any) => { @@ -261,24 +263,52 @@ const CreateEditIntegration: FC = (props) => { /> )} - {fields.map((field) => ( -
-
- -
- - {readOnly ? ( + {fields.map((field) => { + if (field.inputType === 'checkbox') { + if (readOnly) { + return ( +
+ {}} + disabled + type='checkbox' + /> +
+ ) + } + return ( +
+ update(field.key, e)} + type='checkbox' + /> +
+ ) + } + let fieldControl: React.ReactNode + if (readOnly) { + fieldControl = (
{field.hidden ? formData[field.key].replace(/./g, '*') : formData[field.key]}
- ) : field.options ? ( + ) + } else if (field.options) { + const selectedOption = field.options.find( + (v: IntegrationFieldOption) => v.value === formData[field.key], + ) + fieldControl = (
= (props) => { type={field.hidden ? 'password' : field.inputType || 'text'} className='full-width mb-2' /> - )} -
- ))} + ) + } + return ( +
+ + {fieldControl} +
+ ) + })} {authorised && id === 'slack' && (
Can't see your channel? Enter your channel ID here (C0xxxxxx)