From 45a8599841756faeb351b7425345df23d06e5f19 Mon Sep 17 00:00:00 2001 From: "John L. Villalovos" Date: Tue, 21 Oct 2025 17:34:53 -0700 Subject: [PATCH] fix(epics): use actual group_id for save/delete operations on nested epics When an epic belonging to a subgroup is retrieved through a parent group's epic listing, save() and delete() operations would fail because they used the parent group's path instead of the epic's actual group_id. This commit overrides save() and delete() methods in GroupEpic to use the epic's group_id attribute to construct the correct API path, ensuring operations work correctly regardless of how the epic was retrieved. Also add the ability to pass a custom path using `_pg_custom_path` to the `UpdateMixin.update()` and `SaveMixin.save()` methods. This allowed the override of the `update()` method to re-use the `SaveMixin.save()` method. Closes: #3261 --- gitlab/mixins.py | 2 ++ gitlab/v4/objects/epics.py | 58 ++++++++++++++++++++++++++++++ tests/functional/api/test_epics.py | 44 +++++++++++++++++++++++ tests/unit/objects/test_epics.py | 52 +++++++++++++++++++++++++++ 4 files changed, 156 insertions(+) create mode 100644 tests/unit/objects/test_epics.py diff --git a/gitlab/mixins.py b/gitlab/mixins.py index 51de97876..37dbaa43c 100644 --- a/gitlab/mixins.py +++ b/gitlab/mixins.py @@ -314,6 +314,8 @@ def update( path = self.path else: path = f"{self.path}/{utils.EncodedId(id)}" + if "_pg_custom_path" in kwargs: + path = kwargs.pop("_pg_custom_path") excludes = [] if self._obj_cls is not None and self._obj_cls._id_attr is not None: diff --git a/gitlab/v4/objects/epics.py b/gitlab/v4/objects/epics.py index 06400528f..14c498ff4 100644 --- a/gitlab/v4/objects/epics.py +++ b/gitlab/v4/objects/epics.py @@ -2,6 +2,7 @@ from typing import Any, TYPE_CHECKING +import gitlab.utils from gitlab import exceptions as exc from gitlab import types from gitlab.base import RESTObject @@ -29,6 +30,63 @@ class GroupEpic(ObjectDeleteMixin, SaveMixin, RESTObject): resourcelabelevents: GroupEpicResourceLabelEventManager notes: GroupEpicNoteManager + def _epic_path(self) -> str: + """Return the API path for this epic using its real group.""" + if not hasattr(self, "group_id") or self.group_id is None: + raise AttributeError( + "Cannot compute epic path: attribute 'group_id' is missing." + ) + encoded_group_id = gitlab.utils.EncodedId(self.group_id) + return f"/groups/{encoded_group_id}/epics/{self.encoded_id}" + + @exc.on_http_error(exc.GitlabUpdateError) + def save(self, **kwargs: Any) -> dict[str, Any] | None: + """Save the changes made to the object to the server. + + The object is updated to match what the server returns. + + This method uses the epic's group_id attribute to construct the correct + API path. This is important when the epic was retrieved from a parent + group but actually belongs to a sub-group. + + Args: + **kwargs: Extra options to send to the server (e.g. sudo) + + Returns: + The new object data (*not* a RESTObject) + + Raises: + GitlabAuthenticationError: If authentication is not correct + GitlabUpdateError: If the server cannot perform the request + """ + # Use the epic's actual group_id to construct the correct path. + path = self._epic_path() + + # Call SaveMixin.save() method + return super().save(_pg_custom_path=path, **kwargs) + + @exc.on_http_error(exc.GitlabDeleteError) + def delete(self, **kwargs: Any) -> None: + """Delete the object from the server. + + This method uses the epic's group_id attribute to construct the correct + API path. This is important when the epic was retrieved from a parent + group but actually belongs to a sub-group. + + Args: + **kwargs: Extra options to send to the server (e.g. sudo) + + Raises: + GitlabAuthenticationError: If authentication is not correct + GitlabDeleteError: If the server cannot perform the request + """ + if TYPE_CHECKING: + assert self.encoded_id is not None + + # Use the epic's actual group_id to construct the correct path. + path = self._epic_path() + self.manager.gitlab.http_delete(path, **kwargs) + class GroupEpicManager(CRUDMixin[GroupEpic]): _path = "/groups/{group_id}/epics" diff --git a/tests/functional/api/test_epics.py b/tests/functional/api/test_epics.py index a4f6765da..5647044f7 100644 --- a/tests/functional/api/test_epics.py +++ b/tests/functional/api/test_epics.py @@ -1,5 +1,10 @@ +import uuid + import pytest +import gitlab.exceptions +from tests.functional import helpers + pytestmark = pytest.mark.gitlab_premium @@ -30,3 +35,42 @@ def test_epic_notes(epic): epic.notes.create({"body": "Test note"}) assert epic.notes.list() + + +def test_epic_save_from_parent_group_updates_subgroup_epic(gl, group): + subgroup_id = uuid.uuid4().hex + subgroup = gl.groups.create( + { + "name": f"subgroup-{subgroup_id}", + "path": f"sg-{subgroup_id}", + "parent_id": group.id, + } + ) + + nested_epic = subgroup.epics.create( + {"title": f"Nested epic {subgroup_id}", "description": "Nested epic"} + ) + + try: + fetched_epics = group.epics.list(search=nested_epic.title) + assert fetched_epics, "Expected to discover nested epic via parent group list" + subgroup.epics.get(nested_epic.iid) + + fetched_epic = next( + (epic for epic in fetched_epics if epic.id == nested_epic.id), None + ) + assert ( + fetched_epic is not None + ), "Parent group listing did not include nested epic" + + new_label = f"nested-{subgroup_id}" + fetched_epic.labels = [new_label] + fetched_epic.save() + + refreshed_epic = subgroup.epics.get(nested_epic.iid) + assert new_label in refreshed_epic.labels + finally: + helpers.safe_delete(nested_epic) + with pytest.raises(gitlab.exceptions.GitlabGetError): + subgroup.epics.get(nested_epic.iid) + helpers.safe_delete(subgroup) diff --git a/tests/unit/objects/test_epics.py b/tests/unit/objects/test_epics.py new file mode 100644 index 000000000..6d1aea26f --- /dev/null +++ b/tests/unit/objects/test_epics.py @@ -0,0 +1,52 @@ +import pytest +import responses + +from gitlab.v4.objects.epics import GroupEpic + + +def _build_epic(manager, iid=3, group_id=2, title="Epic"): + data = {"iid": iid, "group_id": group_id, "title": title} + return GroupEpic(manager, data) + + +def test_group_epic_save_uses_actual_group_path(group): + epic_manager = group.epics + epic = _build_epic(epic_manager, title="Original") + epic.title = "Updated" + + with responses.RequestsMock() as rsps: + rsps.add( + method=responses.PUT, + url="http://localhost/api/v4/groups/2/epics/3", + json={"iid": 3, "group_id": 2, "title": "Updated"}, + content_type="application/json", + status=200, + match=[responses.matchers.json_params_matcher({"title": "Updated"})], + ) + + epic.save() + + assert epic.title == "Updated" + + +def test_group_epic_delete_uses_actual_group_path(group): + epic_manager = group.epics + epic = _build_epic(epic_manager) + + with responses.RequestsMock() as rsps: + rsps.add( + method=responses.DELETE, + url="http://localhost/api/v4/groups/2/epics/3", + status=204, + ) + + epic.delete() + + assert len(epic._updated_attrs) == 0 + + +def test_group_epic_path_requires_group_id(fake_manager): + epic = GroupEpic(manager=fake_manager, attrs={"iid": 5}) + + with pytest.raises(AttributeError): + epic._epic_path()