Skip to content

Refactor finding detail and fix finding search routes - #689

Merged
nscuro merged 4 commits into
DependencyTrack:masterfrom
mykter:finding-audit-route
Jan 15, 2024
Merged

Refactor finding detail and fix finding search routes#689
nscuro merged 4 commits into
DependencyTrack:masterfrom
mykter:finding-audit-route

Conversation

@mykter

@mykter mykter commented Dec 26, 2023

Copy link
Copy Markdown
Contributor

Description

This PR fixes a pair of bugs and factors out the finding detail view into its own single file component.

It might be easiest to review commit by commit.

Addressed Issue

Fixes #688 in 3895307

Additional Details

I originally planned to make a new view to show a specific finding, and wanted to re-use the detail view from the audit page. This was inlined in a way that was very hard to re-use (or read!). In my journey to try and re-use it I came across Steve's earlier investigations, and was eventually able to pull it out into its own component as it looks like he originally wanted to.

I then discovered that there is already a route that shows this information, it just doesn't work.

I think the refactoring is still worthwhile by itself - maintaining this view in SFC form is easier than in the inline form. The refactoring allowed eslint to find some issues in the inlined code, for example.

Checklist

Comment thread vue.config.js
proxy: { "/api": { target: process.env.VUE_APP_SERVER_URL} }
},
configureWebpack: {
devtool: 'source-map',

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a drive-by improvement from the docs, which I used to make debugging easier.

@mykter
mykter force-pushed the finding-audit-route branch 2 times, most recently from e9d0705 to 0b2e876 Compare December 28, 2023 11:21
@mykter mykter changed the title Refactor finding detail and fix finding search route Refactor finding detail and fix finding search routes Dec 28, 2023
@mykter
mykter marked this pull request as draft December 28, 2023 11:36
@mykter
mykter force-pushed the finding-audit-route branch 2 times, most recently from 2c199ca to d42e6a8 Compare December 28, 2023 19:42
Comment thread src/router/index.js
{
path: 'projects/:uuid/findings/:vulnerability',
name: 'Project Finding Lookup',
name: 'Project Vulnerability Lookup',

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I noticed this existing name clash because vue-router was complaining about it in the console.

@mykter
mykter marked this pull request as ready for review December 28, 2023 20:28
@@ -0,0 +1,247 @@
<template>

@mykter mykter Dec 28, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

74dbc9f was mostly a copy-and-paste from the inlined version, b10be4c fixed some issues in the inlined version that static analysis found once it was factored out.

Signed-off-by: mykter <git@mykter.com>
Signed-off-by: mykter <git@mykter.com>
Signed-off-by: mykter <git@mykter.com>

@nscuro nscuro left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @mykter, I really appreciate the attention to detail in this PR.

From laying out your reasoning in the description, to making sure each change is understandable one-by-one, this was a pleasure to look at.

@nscuro nscuro added the defect Something isn't working label Jan 15, 2024
@nscuro
nscuro merged commit 9d8fe90 into DependencyTrack:master Jan 15, 2024
heyiamwahab236 pushed a commit to tecan/frontend_custom that referenced this pull request Jun 17, 2026
Refactor finding detail and fix finding search routes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

defect Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Finding routes don't work when component or vulnerability specified

2 participants