-
Notifications
You must be signed in to change notification settings - Fork 13
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
Add HTML exporting feature for reports #183
base: master
Are you sure you want to change the base?
Conversation
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.
There are two significant things to refactor:
-
The logic generating the HTML report is a bit hard to follow, I think an approach of having a single function putting the required variables inside a unique Html template would be better, as the SVG template in the console here: https://github.com/nodesource/nsolid-console-ng/blob/master/lib/svg-template.js. This would be easier to maintain; lets split the template and the logic to create it in different files to create a separation of concerns.
-
Remove any third-party dependencies for the CSS and JS embedded in the template, we want a report that could be used offline, also a whole framework is kind of an overkill for what we need here.
lib/report/html.js
Outdated
module.exports = htmlReport | ||
|
||
async function htmlReport (report, whitelist, dir, output) { | ||
if (output === true) output = path.join(process.cwd(), `ncm-report-${Date.now()}.html`) |
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.
Would be nice to include the name of the app in the name
Changes completed as per your review @edsadr |
284f98f
to
560cc0e
Compare
560cc0e
to
32660ff
Compare
} | ||
|
||
return vulns | ||
} |
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.
Migration of formatting logic for reusability in the HTML reporting feature.
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.
The report is working, great work on this @mster, just some items to consider:
-
Currently, the report contains different information than the one being generated by the CLI, this could confuse the customer. let's use the same information and also support all filters like
--filter=compliance
-
I know I suggested not to include external assets, but let's add the styles to use our fonts, if the customer has it it will render properly, if not fall back to another font, ask @pdevay about it.
…lement filtering into html report.
…ange font weight, use nbsp in risk badges
See: https://github.com/nodesource/process/issues/36
Description:
Allows for users to generate and export a NCM project report in HTML (as per customer request).
--long
report.--html path/to/report.html
(default exports to cwd with a timestamped report name).TODO:
Screenshot(s): a very ugly duckling