-
Notifications
You must be signed in to change notification settings - Fork 3
DNM: Placeholder for tracking issue: Inspecjs store #43
base: master
Are you sure you want to change the base?
Conversation
|
Jacob when you're ready for this to get merged into master click ready for review button on github PR. We set this PR up is a demo |
aaronlippold
left a comment
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.
I put this in just because it connects with some of the research I was doing this weekend. I think it is a good learning example. :) However, this PR will not be used given the migration over to the Vuesax template correct? However, some of the things I found could be useful there and I point them out in connection to this code as we are using this as the model to help us develop the components and patterns for the refactored app.
| getControls: function () { | ||
| if (isMounted == true) { | ||
| var val = store.getControls(); | ||
| var controls = store.getFilteredNistControls(); |
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.
My brain says a function signature like getControlsBy('nist') would be the more general type of interface.
| methods: { | ||
| // Called once, to create the hierarchical data representation | ||
| initialize () { | ||
| let that = this |
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.
I think this is cute - aka -this and that but is this good practice?
| store.setSelectedControl(parseInt(event.target.getAttribute("unique_id"))); | ||
| } | ||
| }, | ||
| clear: function (event) { |
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.
Can this be make into a more general loop or forEach type of thing, so that when we add new things this code wouldn't have to be updated.
| }, | ||
| activeImpact: function () { | ||
| return store.getImpact(); | ||
| return store.getSeverityCount(); |
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.
Once we are able to load multiple 'evaluations' aka profiles - will we need to add both singular and plural interfaces at some point?
| rowHtml(item) { | ||
| var table_rows = '<tr><td class="details-control"><button class="' + helpers.status_btn(item.status) + '" style="width:120px">' + item.status + '</td>' | ||
| + '<td>' + item.gid + '</td><td>' + item.rule_title + '</td><td>' + item.severity + '</td><td>' + item.nist +'</td></tr>'; | ||
| rowHtml(control) { |
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.
This seems like it should be a set of render functions or even Functional Components
https://www.youtube.com/watch?v=KS4eizPXRCQ
and the section in the VueMastery Advanced Components course
| <AboutContent v-if="shouldShowAbout" /> | ||
| <SspContent v-else-if="shouldShowSSP" /> | ||
| <b-container v-else="shouldShowResults"> | ||
| <b-container v-else-if="shouldShowResults"> |
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.
Please review this video as to how this could be a bit cleaner overall: https://www.youtube.com/watch?v=KS4eizPXRCQ
Swap in inspecjs and add hooks to store.js
Reference #23