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

Build filetree with missing directories to support --only-findings scans #624

Merged
merged 3 commits into from
Jan 22, 2024

Conversation

OmkarPh
Copy link
Collaborator

@OmkarPh OmkarPh commented Jan 1, 2024

Fixes #404 #393

  • Handles scans with --only-findings option by imputing required intermediate directories
  • Fixed root path resolution for cases like empty scans.

Reference scans -
allsamples-onlyfindings.json.txt
allsamples-onlyfindings-noresources.json.txt

Signed-off-by: Omkar Phansopkar <omkarphansopkar@gmail.com>
@OmkarPh OmkarPh linked an issue Jan 10, 2024 that may be closed by this pull request
Copy link
Contributor

@AyanSinhaMahapatra AyanSinhaMahapatra left a comment

Choose a reason for hiding this comment

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

@OmkarPh thanks++
Looks good and needs a couple more edge-cases handling, based on problems detected in larger scans.

Consider the following scans:

postgresml-2.8.1.json
postgresml-2.8.1-only-findings.json

See comments below:

  1. In the only-findings scan, the file-tree is not behaving correctly for nested directories (as the corresponding parent directories are missing?) should we populate missing parent directories ourself while importing the scan? Example resource: "postgresml-2.8.1/pgml-cms/docs/resources/benchmarks/mindsdb-vs-postgresml.md" this is not present in the file tree on the left. And the expand directory icon also hangs for some reason?
  2. In your allsamples-onlyfindings-noresources.json.txt example, when we import this, nothing happens are there are no resources. I would like to see something to indicate that there was no resources in the scan and so we can't show anything.

Signed-off-by: Omkar Phansopkar <omkarphansopkar@gmail.com>
@OmkarPh
Copy link
Collaborator Author

OmkarPh commented Jan 15, 2024

  1. In the only-findings scan, the file-tree is not behaving correctly for nested directories (as the corresponding parent directories are missing?) should we populate missing parent directories ourself while importing the scan? Example resource: "postgresml-2.8.1/pgml-cms/docs/resources/benchmarks/mindsdb-vs-postgresml.md" this is not present in the file tree on the left. And the expand directory icon also hangs for some reason?

Thank you for pointing this out, there was an issue with handling files in batches (for large scans)
I've fixed it. Should work fine now, see
image

@OmkarPh
Copy link
Collaborator Author

OmkarPh commented Jan 15, 2024

  1. In your allsamples-onlyfindings-noresources.json.txt example, when we import this, nothing happens are there are no resources. I would like to see something to indicate that there was no resources in the scan and so we can't show anything.

Sure, I'll add a warning/error snackbar with this message
But, should we abort importing such scans or load all views with empty data?

@AyanSinhaMahapatra
Copy link
Contributor

But, should we abort importing such scans or load all views with empty data?

If there's no data at all (here I mainly mean no file data, as other codebase level attributes are atleast option-dependent) there's no use showing empty views, so it's fine if we abort importing and show the message there.

Signed-off-by: Omkar Phansopkar <omkarphansopkar@gmail.com>
@OmkarPh
Copy link
Collaborator Author

OmkarPh commented Jan 22, 2024

Thanks for the feedback @AyanSinhaMahapatra
I've updated the PR

Copy link
Contributor

@AyanSinhaMahapatra AyanSinhaMahapatra left a comment

Choose a reason for hiding this comment

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

@OmkarPh LGTM! Thanks++
The issues mentioned above are fixed now and this is ready to merge.
Please release too 🚀

@OmkarPh OmkarPh merged commit 2914011 into develop Jan 22, 2024
10 checks passed
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.

Handle scancode output files that include --only-findings view "Directory Tree" does not work
2 participants