Add Better Error Display #5298
Add Better Error Display #5298
Conversation
ChrisHinchey
commented
Nov 29, 2023
•
ChrisHinchey
commented
- Fixes The error and backtrace should be in the test results and control details #5226
Signed-off-by: Christopher Andrew Hinchey <[email protected]>
Signed-off-by: Christopher Andrew Hinchey <[email protected]>
Signed-off-by: Christopher Andrew Hinchey <[email protected]>
Signed-off-by: Christopher Andrew Hinchey <[email protected]>
apps/frontend/src/components/cards/controltable/ControlRowCol.vue
Outdated
Show resolved
Hide resolved
| justification || | ||
| rationale || | ||
| comments || | ||
| errorMessage !== '' |
There was a problem hiding this comment.
similar to the other ones blocks, unless we're explicitly only checking for empty string, we should probably do the 'string as boolean' thing so that it handles null/undefined as well
There was a problem hiding this comment.
Since we aren't checking for a value from the control.hdf object and rather a custom string based on a condition, wouldn't we want to still check for an empty string unless we make the fail condition a null/undefined
There was a problem hiding this comment.
Re-reading the code, I see what you mean w/r to that function basically returning an enum of [warning | empty-string]. However, in that case, it makes even more sense to just do v-if="stuff || comments || errorMessage" cause "errorMessage !== ''" is semantically equivalent to "errorMessage (as a boolean expression)" due to the empty string being falsy. Consequently, this is mostly just a stylistic choice here for the coding in order to simplify it.
apps/frontend/src/components/cards/controltable/ControlRowDetails.vue
Outdated
Show resolved
Hide resolved
Signed-off-by: Christopher Andrew Hinchey <[email protected]>
…ils.vue Co-authored-by: Amndeep Singh Mann <[email protected]>
|
This has been very useful over the last few days. Hopefully we can get it merged during this release. |
|
Related mitre/saf#2074 |
|
Related inspec/inspec#6900 |
| justification || | ||
| rationale || | ||
| comments || | ||
| errorMessage !== '' |
There was a problem hiding this comment.
Re-reading the code, I see what you mean w/r to that function basically returning an enum of [warning | empty-string]. However, in that case, it makes even more sense to just do v-if="stuff || comments || errorMessage" cause "errorMessage !== ''" is semantically equivalent to "errorMessage (as a boolean expression)" due to the empty string being falsy. Consequently, this is mostly just a stylistic choice here for the coding in order to simplify it.
There was a problem hiding this comment.
I'd appreciate it if you made that stylistic change but it's not the most important thing so I'mma approve this pr. Merge it in after you make your choice. If you choose not to make the change, you're gonna need to re-run the e2e test until it decides to pass before applying the ready-to-merge label.
Signed-off-by: Christopher Andrew Hinchey <[email protected]>
|
I don't mind making the stylistic change, I just wasn't sure if we wanted to make it explicit since it is handled differently from the rest. Just pushed the change |
ChrisHinchey
added
the
ready-to-merge
|
Kudos, SonarCloud Quality Gate passed!
|
* Added backtrace to control row col Signed-off-by: Christopher Andrew Hinchey <[email protected]> * Add additional error display Signed-off-by: Christopher Andrew Hinchey <[email protected]> * Removed icon Signed-off-by: Christopher Andrew Hinchey <[email protected]> * Removed console log Signed-off-by: Christopher Andrew Hinchey <[email protected]> * Fixed backtrace always being present Signed-off-by: Christopher Andrew Hinchey <[email protected]> * Update apps/frontend/src/components/cards/controltable/ControlRowDetails.vue Co-authored-by: Amndeep Singh Mann <[email protected]> * Stylistic change Signed-off-by: Christopher Andrew Hinchey <[email protected]> --------- Signed-off-by: Christopher Andrew Hinchey <[email protected]> Co-authored-by: Amndeep Singh Mann <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
