Skip to content

Conversation

@JohnVillalovos
Copy link
Member

@JohnVillalovos JohnVillalovos commented Oct 18, 2025

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

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

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.

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.

@JohnVillalovos JohnVillalovos force-pushed the jlvillal/3261_epic_group branch from 0f7065d to 3e75d60 Compare October 21, 2025 22:59
Copy link

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.

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.

@JohnVillalovos JohnVillalovos force-pushed the jlvillal/3261_epic_group branch from 3e75d60 to 3993fe7 Compare October 21, 2025 23:05
Copy link

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.

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.

@JohnVillalovos 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

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.

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.

@JohnVillalovos JohnVillalovos force-pushed the jlvillal/3261_epic_group branch from 27bd80c to dcc9847 Compare October 21, 2025 23:42
@codecov
Copy link

codecov bot commented Oct 22, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.73%. Comparing base (4221195) to head (45a8599).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3279      +/-   ##
==========================================
+ Coverage   95.72%   95.73%   +0.01%     
==========================================
  Files          98       98              
  Lines        6052     6068      +16     
==========================================
+ Hits         5793     5809      +16     
  Misses        259      259              
Flag Coverage Δ
api_func_v4 83.66% <93.75%> (+0.02%) ⬆️
cli_func_v4 78.59% <43.75%> (-0.10%) ⬇️
unit 90.21% <100.00%> (+0.02%) ⬆️

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

Files with missing lines Coverage Δ
gitlab/mixins.py 91.55% <100.00%> (+0.04%) ⬆️
gitlab/v4/objects/epics.py 88.13% <100.00%> (+3.69%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

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.

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.

@JohnVillalovos
Copy link
Member Author

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

@JohnVillalovos JohnVillalovos requested a review from nejch December 6, 2025 18:12
…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
@JohnVillalovos JohnVillalovos force-pushed the jlvillal/3261_epic_group branch from 8397bd0 to 45a8599 Compare December 11, 2025 00:56
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"?

2 participants