Adjust summary format

[davelosert]

Hey @febuiles and @courtneycl ,

so here is the PR with the adjusted formatting for the Dependency Review Summary. This PR includes a lot of changes so let me unroll them bit by bit:

Formatting Changes

• If there are findings, the summary on top will include icons depending on the status:
  • ✅ if nothing found for that category
  • ❌ for findings that fail the pr
  • ⚠️ for findings which only yield a Warning (only Unknown Licenses right now)
• If nothing at all was found, the summary will print:
  • "✅ No vulnerabilities or license issues found." on default
  • "✅ No license issues found" if only scanned for licenses (vulnerability_check option set to false)
  • "✅ No vulnerabilities found." if only scanned for vulnerabilities (license_check set to false)
• Made it so that the main sections (Vulnerabilities and License Issues) only appear if there was an actual finding in them and/or they were enabled
• In License Issues, also made it so that only the subsections (Incompatible Licenses, Invalid SPDX License, Unknown License) where findings occured will be included
• Renamed Header Licenses to License Issues
• Reworded the severity statement to Only included vulnerabilities with severity high or higher. and also made it only appear if severity is higher than low (as else anything is included)
• Put the Allowed Licenses and Denied Licenses below of the vulnerability-table (was on top before)
• Adjusted the header sizes and separators for the License Issues Part for better visibility

You can see examples of the new formatting here: davelosert/action-test-repository#2

Technical Changes

• As agreed with @febuiles, I adjusted the tsconfig.json setup to also include tests in the __test__ directory. This also enables eslint for this directory - where I had to do some minor formatting changes to conform with the rules
• Added tests for the summary.ts - there is no full coverage yet, but it's a start
• Added a script to generate the summary-markdown for easier visualization of changes (it will output them to the tmp dir which I also put onto the .gitignore path)

Open Questions

Overall, I'd say I am pretty much done and would love your feedback on the changes.

However, I still have two questions:

• What to do if the PR does not contain any dependency changes? I'd say to not even print anything into the PR - or exactly that message 'Dependency Review did not detect any changes to manifests or dependencies'
• The Sub-Sections for the License Issues are still pretty separated and very large. I was thinking if maybe putting them into a single table and just adding a column for the license issue type (Incompatible, Invalid SPDX, Unknown) so it would look like this:

Package	Version	License	Issue Type
octoinvader 1	4.17.20	Apache 2	Incompatible
octoinvader 2	4.17.20	MIT2	Invalid SPDX
octoinvader 3	4.17.20	null	Unknown License

[febuiles]

@davelosert

What to do if the PR does not contain any dependency changes? I'd say to not even print anything into the PR - or exactly that message 'Dependency Review did not detect any changes to manifests or dependencies'

I agree with you that not adding anything to the PR would be ideal, we don't need to be noisy neighbors.

The Sub-Sections for the License Issues are still pretty separated and very large. I was thinking if maybe putting them into a single table and just adding a column for the license issue type (Incompatible, Invalid SPDX, Unknown) so it would look like this

This looks much better! Do you want to add the changes to this PR, or will that be a follow-up?

[febuiles]

When I run npm run all locally, two new JS files are being generated:

Untracked files:
  (use "git add <file>..." to include in what will be committed)
	__tests__/fixtures/create-test-change.js
	__tests__/fixtures/create-test-vulnerability.js

I would expect the config in tsconfig.build.json would ignore these fixtures, what am I missing?

[davelosert]

This looks much better! Do you want to add the changes to this PR, or will that be a follow-up?

I can still include it in this PR - but I wanted to check first as this is a bit of a larger change. Will work on it, but might take me until the end of the week 🙂

I will also make it that no summary is printed if no changes were detected!

Thank you for the quick feedback 👍🏻

[febuiles]

@davelosert No rush to ship the changes, EOW sounds good! 👍

I just pulled from main, and although the scripts folder looks fixed now, fixtures are still creating JS files:

~/w/dr-action (adjust_summary_format)
$ gl | head -n1
* 5e6910e 2023-03-01 | Built the library in it's current state [David Losert]

~/w/dr-action (adjust_summary_format)
$ tsc -p tsconfig.build.json

~/w/dr-action (adjust_summary_format)
$ gst
On branch adjust_summary_format
Your branch is up to date with 'davelosert/adjust_summary_format'.

Untracked files:
  (use "git add <file>..." to include in what will be committed)
	__tests__/fixtures/create-test-vulnerability.js

nothing added to commit but untracked files present (use "git add" to track)

[davelosert]

@febuiles : That is weird, it doesn't happen for me. Also, the Check dist workflow seems to work and don't run into this problem. Are you sure the file wasn't still there from a previous run before I fixed things?

[febuiles]

@davelosert that's very strange, until your last commit the check-dist thing was failing too so I thought it was not an isolated issue. I probably forgot to remove the file when regenerating the new build artifacts. Thank you 🙇.

[davelosert]

Yeah I was also a bit confused why I had to rebuild the entire thing again after my last commit - but now it works. As I will be making some more changes to adjust the License, let's see if it occurs again. Maybe I also missed something.

[davelosert]

@febuiles : I just went ahead and built the two changes in:

• New License Issues Format - can be viewed in this test comment (last comment) and or this test pr
• Dependency Review is skipped now of no changes found

Happy to get feedback about it 🙂

[febuiles]

@davelosert this looks pretty great, ty!

I was planning to get this merged and released today, but with #422 popping up I'll do a full release once the bug (not related to your changes) is fixed.