Skip to content

Commit 3993fe7

Browse files
JohnVillalovosJohnVillalovos
committed
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. Alao 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. This commit was created with the assistance of AI Closes: #3261
1 parent 0f5655c commit 3993fe7

File tree

7 files changed

+159
-5
lines changed

7 files changed

+159
-5
lines changed

‎gitlab/mixins.py‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,7 @@ def update(
292292
self,
293293
id: str | int | None = None,
294294
new_data: dict[str, Any] | None = None,
295+
_pg_custom_path: str | None = None,
295296
**kwargs: Any,
296297
) -> dict[str, Any]:
297298
"""Update an object on the server.
@@ -314,6 +315,8 @@ def update(
314315
path = self.path
315316
else:
316317
path = f"{self.path}/{utils.EncodedId(id)}"
318+
if _pg_custom_path is not None:
319+
path = _pg_custom_path
317320

318321
excludes = []
319322
if self._obj_cls is not None and self._obj_cls._id_attr is not None:
@@ -413,7 +416,9 @@ def _get_updated_data(self) -> dict[str, Any]:
413416

414417
return updated_data
415418

416-
def save(self, **kwargs: Any) -> dict[str, Any] | None:
419+
def save(
420+
self, _pg_custom_path: str | None = None, **kwargs: Any
421+
) -> dict[str, Any] | None:
417422
"""Save the changes made to the object to the server.
418423
419424
The object is updated to match what the server returns.
@@ -437,7 +442,9 @@ def save(self, **kwargs: Any) -> dict[str, Any] | None:
437442
obj_id = self.encoded_id
438443
if TYPE_CHECKING:
439444
assert isinstance(self.manager, UpdateMixin)
440-
server_data = self.manager.update(obj_id, updated_data, **kwargs)
445+
server_data = self.manager.update(
446+
id=obj_id, new_data=updated_data, _pg_custom_path=_pg_custom_path, **kwargs
447+
)
441448
self._update_attrs(server_data)
442449
return server_data
443450

‎gitlab/v4/objects/appearance.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ def update(
4040
self,
4141
id: str | int | None = None,
4242
new_data: dict[str, Any] | None = None,
43+
_pg_custom_path: str | None = None,
4344
**kwargs: Any,
4445
) -> dict[str, Any]:
4546
"""Update an object on the server.

‎gitlab/v4/objects/epics.py‎

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
from typing import Any, TYPE_CHECKING
44

5+
import gitlab.utils
56
from gitlab import exceptions as exc
67
from gitlab import types
78
from gitlab.base import RESTObject
@@ -24,11 +25,71 @@
2425

2526
class GroupEpic(ObjectDeleteMixin, SaveMixin, RESTObject):
2627
_id_attr = "iid"
28+
manager: GroupEpicManager
2729

2830
issues: GroupEpicIssueManager
2931
resourcelabelevents: GroupEpicResourceLabelEventManager
3032
notes: GroupEpicNoteManager
3133

34+
def _epic_path(self) -> str:
35+
"""Return the API path for this epic using its real group."""
36+
if not hasattr(self, "group_id") or self.group_id is None:
37+
raise AttributeError(
38+
"Cannot compute epic path: attribute 'group_id' is missing."
39+
)
40+
encoded_group_id = gitlab.utils.EncodedId(self.group_id)
41+
return f"/groups/{encoded_group_id}/epics/{self.encoded_id}"
42+
43+
@exc.on_http_error(exc.GitlabUpdateError)
44+
def save(
45+
self, _pg_custom_path: str | None = None, **kwargs: Any
46+
) -> dict[str, Any] | None:
47+
"""Save the changes made to the object to the server.
48+
49+
The object is updated to match what the server returns.
50+
51+
This method uses the epic's group_id attribute to construct the correct
52+
API path. This is important when the epic was retrieved from a parent
53+
group but actually belongs to a sub-group.
54+
55+
Args:
56+
**kwargs: Extra options to send to the server (e.g. sudo)
57+
58+
Returns:
59+
The new object data (*not* a RESTObject)
60+
61+
Raises:
62+
GitlabAuthenticationError: If authentication is not correct
63+
GitlabUpdateError: If the server cannot perform the request
64+
"""
65+
# Use the epic's actual group_id to construct the correct path.
66+
path = self._epic_path() if _pg_custom_path is None else _pg_custom_path
67+
68+
# Call SaveMixin.save() method
69+
return super().save(_pg_custom_path=path, **kwargs)
70+
71+
@exc.on_http_error(exc.GitlabDeleteError)
72+
def delete(self, **kwargs: Any) -> None:
73+
"""Delete the object from the server.
74+
75+
This method uses the epic's group_id attribute to construct the correct
76+
API path. This is important when the epic was retrieved from a parent
77+
group but actually belongs to a sub-group.
78+
79+
Args:
80+
**kwargs: Extra options to send to the server (e.g. sudo)
81+
82+
Raises:
83+
GitlabAuthenticationError: If authentication is not correct
84+
GitlabDeleteError: If the server cannot perform the request
85+
"""
86+
if TYPE_CHECKING:
87+
assert self.encoded_id is not None
88+
89+
# Use the epic's actual group_id to construct the correct path.
90+
path = self._epic_path()
91+
self.manager.gitlab.http_delete(path, **kwargs)
92+
3293

3394
class GroupEpicManager(CRUDMixin[GroupEpic]):
3495
_path = "/groups/{group_id}/epics"
@@ -51,7 +112,7 @@ class GroupEpicIssue(ObjectDeleteMixin, SaveMixin, RESTObject):
51112
# 'self.manager.update()' call in the 'save' method.
52113
manager: GroupEpicIssueManager
53114

54-
def save(self, **kwargs: Any) -> None:
115+
def save(self, _pg_custom_path: str | None = None, **kwargs: Any) -> None:
55116
"""Save the changes made to the object to the server.
56117
57118
The object is updated to match what the server returns.

‎gitlab/v4/objects/labels.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ class GroupLabel(SubscribableMixin, SaveMixin, ObjectDeleteMixin, RESTObject):
2525

2626
# Update without ID, but we need an ID to get from list.
2727
@exc.on_http_error(exc.GitlabUpdateError)
28-
def save(self, **kwargs: Any) -> None:
28+
def save(self, _pg_custom_path: str | None = None, **kwargs: Any) -> None:
2929
"""Saves the changes made to the object to the server.
3030
3131
The object is updated to match what the server returns.
@@ -86,7 +86,7 @@ class ProjectLabel(
8686

8787
# Update without ID, but we need an ID to get from list.
8888
@exc.on_http_error(exc.GitlabUpdateError)
89-
def save(self, **kwargs: Any) -> None:
89+
def save(self, _pg_custom_path: str | None = None, **kwargs: Any) -> None:
9090
"""Saves the changes made to the object to the server.
9191
9292
The object is updated to match what the server returns.

‎gitlab/v4/objects/settings.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ def update(
9696
self,
9797
id: str | int | None = None,
9898
new_data: dict[str, Any] | None = None,
99+
_pg_custom_path: str | None = None,
99100
**kwargs: Any,
100101
) -> dict[str, Any]:
101102
"""Update an object on the server.

‎tests/functional/api/test_epics.py‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
1+
import uuid
2+
13
import pytest
24

5+
from tests.functional import helpers
6+
37
pytestmark = pytest.mark.gitlab_premium
48

59

@@ -30,3 +34,39 @@ def test_epic_notes(epic):
3034

3135
epic.notes.create({"body": "Test note"})
3236
assert epic.notes.list()
37+
38+
39+
def test_epic_save_from_parent_group_updates_subgroup_epic(gl, group):
40+
subgroup_id = uuid.uuid4().hex
41+
subgroup = gl.groups.create(
42+
{
43+
"name": f"subgroup-{subgroup_id}",
44+
"path": f"sg-{subgroup_id}",
45+
"parent_id": group.id,
46+
}
47+
)
48+
49+
nested_epic = subgroup.epics.create(
50+
{"title": f"Nested epic {subgroup_id}", "description": "Nested epic"}
51+
)
52+
53+
try:
54+
fetched_epics = group.epics.list(search=nested_epic.title)
55+
assert fetched_epics, "Expected to discover nested epic via parent group list"
56+
57+
fetched_epic = next(
58+
(epic for epic in fetched_epics if epic.id == nested_epic.id), None
59+
)
60+
assert (
61+
fetched_epic is not None
62+
), "Parent group listing did not include nested epic"
63+
64+
new_label = f"nested-{subgroup_id}"
65+
fetched_epic.labels = [new_label]
66+
fetched_epic.save()
67+
68+
refreshed_epic = subgroup.epics.get(nested_epic.iid)
69+
assert new_label in refreshed_epic.labels
70+
finally:
71+
helpers.safe_delete(nested_epic)
72+
helpers.safe_delete(subgroup)

‎tests/unit/objects/test_epics.py‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import responses
2+
3+
from gitlab.v4.objects.epics import GroupEpic
4+
5+
6+
def _build_epic(manager, iid=3, group_id=2, title="Epic"):
7+
data = {"iid": iid, "group_id": group_id, "title": title}
8+
return GroupEpic(manager, data)
9+
10+
11+
def test_group_epic_save_uses_actual_group_path(group):
12+
epic_manager = group.epics
13+
epic = _build_epic(epic_manager, title="Original")
14+
epic.title = "Updated"
15+
16+
with responses.RequestsMock() as rsps:
17+
rsps.add(
18+
method=responses.PUT,
19+
url="http://localhost/api/v4/groups/2/epics/3",
20+
json={"iid": 3, "group_id": 2, "title": "Updated"},
21+
content_type="application/json",
22+
status=200,
23+
match=[responses.matchers.json_params_matcher({"title": "Updated"})],
24+
)
25+
26+
epic.save()
27+
28+
assert epic.title == "Updated"
29+
30+
31+
def test_group_epic_delete_uses_actual_group_path(group):
32+
epic_manager = group.epics
33+
epic = _build_epic(epic_manager)
34+
35+
with responses.RequestsMock() as rsps:
36+
rsps.add(
37+
method=responses.DELETE,
38+
url="http://localhost/api/v4/groups/2/epics/3",
39+
status=204,
40+
)
41+
42+
epic.delete()
43+
44+
assert len(epic._updated_attrs) == 0

0 commit comments

Comments
 (0)