-
-
Notifications
You must be signed in to change notification settings - Fork 702
Fix tag deletion failing when tag is used by project collection logic #4858
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix tag deletion failing when tag is used by project collection logic #4858
Conversation
1fcfa5c to
13c923b
Compare
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
There was a problem hiding this 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 tag deletion fails when a tag is used in project collection logic. It adds new tests to verify deletion behavior with collection projects and updates the query logic in the TagQueryManager to account for collection project relationships.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/test/java/org/dependencytrack/resources/v1/TagResourceTest.java | Adds tests for deleting tags when used by collection projects with different permissions |
| src/main/java/org/dependencytrack/persistence/TagQueryManager.java | Updates SQL/JDOQL queries and deletion logic to properly handle tags associated with collection projects |
Comments suppressed due to low confidence (2)
src/test/java/org/dependencytrack/resources/v1/TagResourceTest.java:439
- [nitpick] Consider adding an assertion to verify the state of tag 'bar' in deleteTagsWhenAssignedToCollectionProjectTest to clearly confirm if it is intended to be deleted (or retained) when used by a collection project.
assertThat(qm.getTagByName("foo")).isNull();
src/main/java/org/dependencytrack/persistence/TagQueryManager.java:414
- [nitpick] The variable name candidateIdsWithCollectionProjects is a bit ambiguous. Consider renaming it to something like accessibleCollectionTagIds to clearly indicate that it holds the IDs of tags associated with accessible collection projects.
final List<Long> candidateIdsWithCollectionProjects = candidateRows.stream()
13c923b to
4aa868e
Compare
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) |
4aa868e to
c1b678b
Compare
There was a problem hiding this 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 the issue where tag deletion was failing when a tag was used by project collection logic. Key changes include updating the API response and tests to include a new "collectionProjectCount" field, adjusting SQL queries in the persistence layer to account for collection projects, and modifying deletion logic to properly handle tags assigned to collection projects.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/test/java/org/dependencytrack/resources/v1/TagResourceTest.java | Updated tests to assert collectionProjectCount values and add new scenarios for collection project–related deletion behavior. |
| src/main/java/org/dependencytrack/resources/v1/vo/TagListResponseItem.java | Added the collectionProjectCount field to the API response record. |
| src/main/java/org/dependencytrack/resources/v1/TagResource.java | Mapped the new collectionProjectCount field from the query into the response object. |
| src/main/java/org/dependencytrack/persistence/TagQueryManager.java | Revised SQL queries and deletion logic to properly compute and handle collection project counts, including accessible counts and error messages. |
Comments suppressed due to low confidence (1)
src/main/java/org/dependencytrack/persistence/TagQueryManager.java:250
- [nitpick] Consider renaming the 'id' field in TagDeletionCandidateRow to 'tagId' to improve clarity about what the identifier represents.
long id,
There was a problem hiding this 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 tag deletion was failing if a tag was in use by collection projects. Key changes include:
- Adding a new count for collection projects ("collectionProjectCount") to tag response items and SQL queries.
- Enhancing deletion logic in TagQueryManager to account for tags used by collection projects.
- Updating tests to verify both successful and failed deletion scenarios when collection projects are involved.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/test/java/org/dependencytrack/resources/v1/TagResourceTest.java | Added tests checking JSON responses for new collection project counts and deletion behavior. |
| src/main/java/org/dependencytrack/resources/v1/vo/TagListResponseItem.java | Updated API documentation and record parameters for including collection project count. |
| src/main/java/org/dependencytrack/resources/v1/TagResource.java | Modified tag mapping logic to include the new collectionProjectCount field. |
| src/main/java/org/dependencytrack/persistence/TagQueryManager.java | Revised SQL queries and deletion logic to incorporate collection project counts and permissions. |
Comments suppressed due to low confidence (1)
src/main/java/org/dependencytrack/persistence/TagQueryManager.java:445
- Ensure that switching the deletion query filter from tag names to tag ids is fully compatible with all remaining logic and that candidate IDs are correctly populated. A brief inline note about this change could aid future maintainers.
deletionQuery.setFilter(":ids.contains(id)");
c1b678b to
978fc4f
Compare
There was a problem hiding this 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 tag deletion was failing when the tag was being used by project collection logic. Key changes include updating test assertions to verify collection project counts, adding new API endpoints and corresponding value objects for tagged collection projects, and modifying persistence queries in TagQueryManager and QueryManager to handle collection project counts correctly.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/org/dependencytrack/resources/v1/TagResourceTest.java | Updated tests to verify new collection project count behavior and deletion rules |
| src/main/java/org/dependencytrack/resources/v1/vo/* | Replaced @parameter annotations with @Schema annotations and added a new VO for collection projects |
| src/main/java/org/dependencytrack/resources/v1/TagResource.java | Introduced a new endpoint to retrieve tagged collection projects |
| src/main/java/org/dependencytrack/persistence/TagQueryManager.java | Updated queries and deletion logic to account for collection projects |
| src/main/java/org/dependencytrack/persistence/QueryManager.java | Exposed method for retrieving tagged collection projects through TagQueryManager |
Comments suppressed due to low confidence (3)
src/main/java/org/dependencytrack/v1/TagResource.java:290
- Consider adding integration tests for the new 'getTaggedCollectionProjects' endpoint to verify its behavior and response format.
@GET
@Path("/{name}/collectionProject")
src/main/java/org/dependencytrack/persistence/TagQueryManager.java:393
- Verify that using the ':ids.contains(id)' filter in the deletion query is supported by JDOQL; consider using an 'IN' clause for clarity and compatibility.
if (row.collectionProjectCount() > 0 && !hasPortfolioManagementPermission) {
src/main/java/org/dependencytrack/persistence/TagQueryManager.java:604
- Ensure that the 'filter', 'orderBy', and 'orderDirection' variables are properly defined and initialized within the scope, as they are used to construct the SQL query.
if (filter != null) {
Signed-off-by: nscuro <nscuro@protonmail.com>
978fc4f to
1e111d3
Compare
There was a problem hiding this 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 the issue where tag deletion fails when a tag is in use by project collection logic. It adds tests and query enhancements to correctly handle collection projects in tag deletion and listing operations.
- Updated tests (TagResourceTest.java) to cover collection project scenarios during tag deletion.
- Extended response objects and query manager logic to include new collectionProjectCount fields.
- Added a new endpoint in TagResource.java to list collection projects associated with a tag.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| TagResourceTest.java | New and updated tests validating correct behavior for collection projects in tag deletion scenarios. |
| TaggedProjectListResponseItem.java, TaggedPolicyListResponseItem.java, TaggedNotificationRuleListResponseItem.java, TagListResponseItem.java, TaggedCollectionProjectListResponseItem.java | Updated/added VO classes to include collection project information. |
| TagResource.java | Introduced a new endpoint (/{name}/collectionProject) to retrieve collection projects linked to a tag. |
| TagQueryManager.java | Modified queries to calculate collectionProjectCount and integrated deletion logic for tags used by collection projects. |
| QueryManager.java | Exposed the getTaggedCollectionProjects method to support the new endpoint. |
Comments suppressed due to low confidence (1)
src/main/java/org/dependencytrack/persistence/TagQueryManager.java:604
- The variables 'filter', 'orderBy', and 'orderDirection' are referenced in the SQL query construction without local declaration; ensure these variables are properly defined in the context or passed as method parameters to avoid compile time issues.
if (filter != null) {
Description
Fixes tag deletion failing when tag is used by project collection logic.
Addressed Issue
N/A
Additional Details
N/A
Checklist
This PR implements an enhancement, and I have provided tests to verify that it works as intendedThis PR introduces changes to the database model, and I have added corresponding update logicThis PR introduces new or alters existing behavior, and I have updated the documentation accordingly