Skip to content
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

Fixed dependency tree right click action #483

Merged
merged 6 commits into from
Sep 23, 2024

Conversation

eyalk007
Copy link
Contributor

@eyalk007 eyalk007 commented Sep 17, 2024

  • All tests passed. If this feature is not already covered by the tests, I added new tests.

This PR fixes the right click action in the tree for dependencies with vulnerabilities.

Package json now works with scopes, and Maven and Gradle work now when inheriting version from parent.

The problem for gradle and maven mainly resided in the fact we sent the version(that was not needed) to the comparison with the impact tree.
In package json, the scope from the package name was removed and as a result, caused issues in identifying the package.

(pyton is not implemented)
@eyalk007 eyalk007 changed the title FIxed dependency tree right click action Fixed dependency tree right click action Sep 17, 2024
@eyalk007 eyalk007 self-assigned this Sep 17, 2024
@eyalk007 eyalk007 force-pushed the bug-fix/dependnecy-link-in-tree branch from bacf6d4 to b37a752 Compare September 18, 2024 08:21
@eyalk007 eyalk007 marked this pull request as ready for review September 18, 2024 08:43
@eyalk007 eyalk007 added bug Something isn't working safe to test Approve running integration tests on a pull request labels Sep 18, 2024
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Sep 18, 2024
@eyalk007 eyalk007 added the safe to test Approve running integration tests on a pull request label Sep 19, 2024
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Sep 19, 2024
createNodePopupMenu((DependencyNode) selected);
DescriptorFileTreeNode descriptorFileTreeNode = (DescriptorFileTreeNode) selectedPath.getParentPath().getLastPathComponent();
String descriptorPath = descriptorFileTreeNode.getSubtitle();
this.createNodePopupMenu((DependencyNode) selected, descriptorPath);
} else if (selected instanceof VulnerabilityNode) {
createIgnoreRuleOption((VulnerabilityNode) selected, e);
} else if (selected instanceof ApplicableIssueNode) {
createIgnoreRuleOption(((ApplicableIssueNode) selected).getIssue(), e);
} else {
Copy link
Collaborator

Choose a reason for hiding this comment

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

instead of removing the comment here - I suggest adding comments to all for cases of the if/else

Copy link
Contributor Author

@eyalk007 eyalk007 Sep 22, 2024

Choose a reason for hiding this comment

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

okay tell me your opinion after my changes

Copy link
Collaborator

@hadarshjfrog hadarshjfrog left a comment

Choose a reason for hiding this comment

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

The code changes themselves are ok - but from what I read in the description - we're eliminating all version comparison.
How can that effect complex project with multiple packages from different types. should we identify them as vulnerabilities?

@eyalk007
Copy link
Contributor Author

eyalk007 commented Sep 22, 2024

@hadarshjfrog. what your saying about the versions is a dilema. I can try and keep it in cases it exists(meaning from parent) and not in cases it does not.
but you will always have the possibility of a case where there are two parents in the project. one has version lets say 1.0.0
and the other has version 1.2.0

they both will look the same as they both dont have a version(because they both inherit from their parents)
a possible solution is to use the maven and gradle apis(as they are the only supported multi module projects). but we didn't want to rely on them from the beginning.

Copy link

👍 Frogbot scanned this pull request and did not find any new security issues.


@eyalk007 eyalk007 merged commit 353935a into jfrog:master Sep 23, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants