Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions 2 gitlab/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,8 @@ def update(
path = self.path
else:
path = f"{self.path}/{utils.EncodedId(id)}"
if "_pg_custom_path" in kwargs:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: pg_ always makes me think of postgres here, but maybe I've spent too much time in gitlab.rb. Also depending on the answer in the other questions, would it make sense to have this as a keyword argument instead of popping it here?

Suggested change
if "_pg_custom_path" in kwargs:
if "_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:
Expand Down
58 changes: 58 additions & 0 deletions 58 gitlab/v4/objects/epics.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can a group_id actually be none for an epic, since it's always a group resource? Or can we go with a simple try/except here?

One other thought - will this work if we get an epic with lazy=True from an arbitrary group?

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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if we should have some more generic _get_custom_path or _resolve_path helpers that would automatically get picked up by the mixins, as we already do this twice for delete/update. That way we don't need an override here (or maybe also in some other places), but I'm not sure if feasible or worth it.


@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"
Expand Down
44 changes: 44 additions & 0 deletions 44 tests/functional/api/test_epics.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import uuid

import pytest

import gitlab.exceptions
from tests.functional import helpers

pytestmark = pytest.mark.gitlab_premium


Expand Down Expand Up @@ -32,3 +37,42 @@ def test_epic_notes(epic):
epic.notes.create({"body": "Test note"})
new_notes = epic.notes.list(get_all=True)
assert len(new_notes) == (len(notes) + 1), f"{new_notes} {notes}"


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(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we have this one as a fixture that yields the epic, to avoid the manual cleanup below?

Or I'm just confused by the try/finally block, maybe it was meant to test deletion even if previous things failed.

{"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)
52 changes: 52 additions & 0 deletions 52 tests/unit/objects/test_epics.py
Original file line number Diff line number Diff line change
@@ -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()
Morty Proxy This is a proxified and sanitized view of the page, visit original site.