-
Notifications
You must be signed in to change notification settings - Fork 6
✨ Provide KPI metrics view #700
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
base: main
Are you sure you want to change the base?
Conversation
e631cba to
7bff0c8
Compare
5ba78b7 to
eb635f3
Compare
2ae407c to
66fe1bb
Compare
Fix type error ✅ Fix tests Update CHANGELOG.md ✅ Fix tests 🔧 Try different syntactic expression to avoid CI error Bump vitest version 🔧 Try yet another syntactic expression to avoid CI error 🔧 Try yet another syntactic expression to avoid CI error 🔧 Try node upgrade in test CI ✨ Preserve date range selection 🔧 Refactor test to not use hoisted Fix formatting 🔥 Remove test CI debugging script
Add ean values ✨ Provide filtering by build ✅ Fix test
66fe1bb to
832d2b5
Compare
C-Valen
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.
Generally looking good, just some small considerations/questions.
Also as discussed internally in meeting would be convenient to show the total KPI across all features.
| { | ||
| path: '/kpi', | ||
| name: 'kpi-metrics', | ||
| component: KpiMetricsView, | ||
| meta: { | ||
| title: 'KPI Metrics', | ||
| }, | ||
| }, |
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.
Will we make the data publicly available?
| }); | ||
| }); | ||
|
|
||
| describe('getKpiMetrics', () => { |
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.
Even being the function part of the service, I think it could be better organized in a individual test suite, wdyt?
| 'suggest-cwe': 'Suggest CWE', | ||
| 'suggest-description': 'Suggest Description', | ||
| 'suggest-impact': 'Suggest Impact', | ||
| 'suggest-statement': 'Suggest Statement Mitigation', |
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.
Are title and CVSS included?
| type FeatureLabelsWithAllType = keyof typeof FeatureLabelsWithAll; | ||
| const kpiMetrics = ref<AegisKpiMetrics | null>(null); | ||
| // const chosenFeature = ref<keyof typeof FeatureLabels>('all'); |
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.
Is this still needed for some indication?
| </button> | ||
| </section> | ||
| <h3 v-if="!isFilteredDateRange">Average (All Time)</h3> | ||
| <h3 v-if="isFilteredDateRange">Average (AcrossDate Range)</h3> |
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.
| <h3 v-if="isFilteredDateRange">Average (AcrossDate Range)</h3> | |
| <h3 v-if="isFilteredDateRange">Average (Across Date Range)</h3> |
| ### Added | ||
| * Add KPI metrics data visualization (`AEGIS-196`) |
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.
We can use the existing Added block at the top
| }); | ||
| } catch (error) { | ||
| console.error('AegisAIService::sendFeedback() Error:', error); | ||
| throw error; |
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.
Why removing this throw error?
| const chosenFeature = ref<FeatureLabelsWithAllType>('all'); | ||
| async function fetchKpiMetrics(feature: FeatureLabelsWithAllType = 'all') { | ||
| const featureKey = feature === 'all' ? 'all' : (feature as any); |
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.
Cant we avoid any typing
const featureKey: 'all' | FeatureLabel = feature;
AEGIS-196 KPI Metrics View
Checklist:
Summary:
Changes:
Adds KPI metrics view
Adds UI library for data visualization
Considerations: