← 返回首页
fix(epics): use actual group_id for save/delete operations on nested epics by JohnVillalovos · Pull Request #3279 · python-gitlab/python-gitlab · GitHub
Skip to content

Navigation Menu

Toggle navigation
Sign in
Appearance settings
Search or jump to...

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

Provide feedback

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

Include my email address so I can be contacted

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
Resetting focus

fix(epics): use actual group_id for save/delete operations on nested epics#3279

Open
JohnVillalovos wants to merge 1 commit into
mainfrom
jlvillal/3261_epic_group
Open

fix(epics): use actual group_id for save/delete operations on nested epics#3279
JohnVillalovos wants to merge 1 commit into
mainfrom
jlvillal/3261_epic_group

Conversation

Copy link
Copy Markdown
Member

JohnVillalovos commented Oct 18, 2025
edited
Loading

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.

Add an internal custom path hook for object save and delete operations, and let
UpdateMixin and DeleteMixin accept an explicit _custom_path keyword. GroupEpic
uses this hook to build mutation paths from the epic's server-provided group_id,
so save() and delete() target the epic's owning group.

Avoid using parent manager attrs as a fallback for the epic group path. Lazy
epics do not have a server-provided group_id, so raise a clear error instead of
silently using the parent group path.

Add unit coverage for custom update/delete paths, GroupEpic save/delete path
selection, and lazy epic path failures. Add a functional test for saving a
subgroup epic discovered through the parent group listing.

Assisted-by: OpenAI Codex (GPT-5)

Closes: #3261

Copilot AI review requested due to automatic review settings October 18, 2025 17:41
JohnVillalovos marked this pull request as draft October 18, 2025 17:41
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

Pull Request Overview

This PR fixes an issue where save() and delete() operations on epics retrieved through a parent group's epic listing would fail because they used the wrong group path. The fix ensures that operations use the epic's actual group_id attribute to construct the correct API path.

  • Overrides save() and delete() methods in GroupEpic to use the epic's group_id for API path construction
  • Adds helper method _epic_path() to compute the correct API path using the epic's real group
  • Adds comprehensive test coverage for both unit and functional scenarios

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
gitlab/v4/objects/epics.py Implements the fix by overriding save() and delete() methods to use epic's group_id for path construction
tests/unit/objects/test_epics.py Adds unit tests to verify save() and delete() operations use the correct group path
tests/functional/api/test_epics.py Adds functional test demonstrating the fix works for nested epics accessed through parent groups

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread gitlab/v4/objects/epics.py Outdated Show resolved Hide resolved
Comment thread gitlab/v4/objects/epics.py Outdated Show resolved Hide resolved
JohnVillalovos force-pushed the jlvillal/3261_epic_group branch from 0f7065d to 3e75d60 Compare October 21, 2025 22:59
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

Pull Request Overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread gitlab/mixins.py Outdated Show resolved Hide resolved
Comment thread gitlab/v4/objects/epics.py Outdated Show resolved Hide resolved
JohnVillalovos force-pushed the jlvillal/3261_epic_group branch from 3e75d60 to 3993fe7 Compare October 21, 2025 23:05
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

Pull Request Overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread gitlab/v4/objects/epics.py Outdated Show resolved Hide resolved
JohnVillalovos force-pushed the jlvillal/3261_epic_group branch 2 times, most recently from 3e855f3 to 27bd80c Compare October 21, 2025 23:33
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread gitlab/v4/objects/epics.py Show resolved Hide resolved
JohnVillalovos force-pushed the jlvillal/3261_epic_group branch from 27bd80c to dcc9847 Compare October 21, 2025 23:42
Copy link
Copy Markdown

codecov Bot commented Oct 22, 2025
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.77%. Comparing base (659c648) to head (e3d27cc).

Additional details and impacted files
@@ Coverage Diff @@ ## main #3279 +/- ## ========================================== + Coverage 92.16% 95.77% +3.61% ========================================== Files 100 100 Lines 6125 6153 +28 ========================================== + Hits 5645 5893 +248 + Misses 480 260 -220
Flag Coverage Δ
api_func_v4 83.76% <90.90%> (?)
cli_func_v4 78.46% <60.60%> (-0.12%) ⬇️
unit 90.28% <100.00%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
gitlab/mixins.py 91.86% <100.00%> (+5.56%) ⬆️
gitlab/v4/objects/epics.py 91.22% <100.00%> (+13.45%) ⬆️

... and 22 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

JohnVillalovos force-pushed the jlvillal/3261_epic_group branch from dcc9847 to f420862 Compare October 22, 2025 00:25
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Copy Markdown
Member Author

FYI: This was tested and reported as working #3261 (comment)

JohnVillalovos requested a review from nejch December 6, 2025 18:12
JohnVillalovos force-pushed the jlvillal/3261_epic_group branch from 8397bd0 to 45a8599 Compare December 11, 2025 00:56
JohnVillalovos force-pushed the jlvillal/3261_epic_group branch from 45a8599 to 0dd6ab1 Compare December 24, 2025 21:42
Copy link
Copy Markdown
Member

nejch left a comment

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

Thanks @JohnVillalovos, looks good and that's a tricky one. Not sure about lazily fetched epics - just have a few questions there.

Comment thread gitlab/v4/objects/epics.py Outdated Show resolved Hide resolved
Comment thread gitlab/v4/objects/epics.py Outdated Show resolved Hide resolved
Comment thread gitlab/mixins.py Outdated Show resolved Hide resolved
Comment thread tests/functional/api/test_epics.py Show resolved Hide resolved
JohnVillalovos force-pushed the jlvillal/3261_epic_group branch from ce876ef to 0f029e9 Compare January 19, 2026 23:14
JohnVillalovos force-pushed the jlvillal/3261_epic_group branch 4 times, most recently from 83feda8 to cfe29a4 Compare April 18, 2026 18:04
JohnVillalovos marked this pull request as draft April 18, 2026 18:25
JohnVillalovos force-pushed the jlvillal/3261_epic_group branch 2 times, most recently from f187c43 to 4702278 Compare April 18, 2026 19:42
…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. Add an internal custom path hook for object save and delete operations, and let UpdateMixin and DeleteMixin accept an explicit `_custom_path` keyword. GroupEpic uses this hook to build mutation paths from the epic's server-provided group_id, so save() and delete() target the epic's owning group. Avoid using parent manager attrs as a fallback for the epic group path. Lazy epics do not have a server-provided group_id, so raise a clear error instead of silently using the parent group path. Add unit coverage for custom update/delete paths, GroupEpic save/delete path selection, and lazy epic path failures. Add a functional test for saving a subgroup epic discovered through the parent group listing. Assisted-by: OpenAI Codex (GPT-5) Closes: #3261
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread gitlab/v4/objects/epics.py Show resolved Hide resolved
JohnVillalovos marked this pull request as ready for review April 18, 2026 20:27
JohnVillalovos requested a review from nejch April 18, 2026 20:27
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GroupEpic uses "iid" as ID attribute but should use "id"?

3 participants

Footer

© 2026 GitHub, Inc.