Export dashboard as CSV#159
Conversation
jllanosg
requested review from
Icksir,
caverav,
iTzGooDLife,
massi-ponce and
sealra
📝 Walkthrough📝 WalkthroughWalkthroughEste pull request introduce cambios significativos en varios componentes del frontend. Se eliminan implementaciones locales de funciones relacionadas con el CVSS y se reemplazan por importaciones desde un módulo de utilidades. Además, se mejora la funcionalidad de exportación de datos al CSV, permitiendo a los usuarios seleccionar entre diferentes formatos de exportación. Se añaden nuevos mensajes de localización para mejorar la retroalimentación del usuario, y se realizan ajustes en el estado y la lógica de los componentes para manejar estos cambios. Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Outside diff range and nitpick comments (14)
frontend/src/components/dashboard/ExportModal.tsx (1)
Line range hint 41-83: ¡El modal no cumple con los estándares de accesibilidad!
El componente carece de atributos ARIA esenciales y manejo de navegación por teclado.
Implementa las siguientes mejoras:
- <div className="fixed inset-0 bg-black bg-opacity-50 flex items-center justify-center">
+ <div
+ className="fixed inset-0 bg-black bg-opacity-50 flex items-center justify-center"
+ role="dialog"
+ aria-modal="true"
+ aria-labelledby="modal-title"
+ onKeyDown={(e) => e.key === 'Escape' && onClose()}
+ >
<div className="bg-gray-800 p-6 rounded-lg">
- <h2 className="text-2xl font-bold mb-4">{`${t('exportDashboard')} (${type})`}</h2>
+ <h2 id="modal-title" className="text-2xl font-bold mb-4">{`${t('exportDashboard')} (${type})`}</h2>frontend/src/components/dashboard/CVSSScore.tsx (4)
Line range hint 22-38: El estado inicial del gráfico debe ser consistente
El array de datos inicial no coincide con la cantidad de etiquetas. Tienes 5 etiquetas pero solo 4 valores en el array de datos. Además, los arrays de colores tampoco coinciden.
datasets: [
{
- data: [0, 0, 0, 0],
- backgroundColor: ['#FF4136', '#FF851B', '#FFDC00', '#2ECC40'],
- borderColor: ['#FF4136', '#FF851B', '#FFDC00', '#2ECC40'],
+ data: [0, 0, 0, 0, 0],
+ backgroundColor: ['#FF4136', '#FF851B', '#FFDC00', '#2ECC40', '#53AA33'],
+ borderColor: ['#FF4136', '#FF851B', '#FFDC00', '#2ECC40', '#53AA33'],
borderWidth: 1,
},
],Line range hint 39-85: Optimizar el procesamiento de datos en useEffect
La lógica actual realiza múltiples iteraciones sobre el mismo array de findings. Esto puede optimizarse utilizando una única iteración.
useEffect(() => {
getAuditById(auditId)
.then(audit => {
+ const severityCounts = {
+ C: 0, H: 0, M: 0, L: 0, I: 0
+ };
+ audit.datas.findings.forEach(finding => {
+ const severity = cvssStringToSeverity(finding.cvssv3);
+ if (severity in severityCounts) {
+ severityCounts[severity]++;
+ }
+ });
setPieChartData({
labels: [
t('critical'),
t('cvss.high'),
t('cvss.medium'),
t('cvss.low'),
'Informative',
],
datasets: [
{
data: [
- audit.datas.findings.filter(
- finding => cvssStringToSeverity(finding.cvssv3) === 'C',
- ).length,
- audit.datas.findings.filter(
- finding => cvssStringToSeverity(finding.cvssv3) === 'H',
- ).length,
- audit.datas.findings.filter(
- finding => cvssStringToSeverity(finding.cvssv3) === 'M',
- ).length,
- audit.datas.findings.filter(
- finding => cvssStringToSeverity(finding.cvssv3) === 'L',
- ).length,
- audit.datas.findings.filter(
- finding => cvssStringToSeverity(finding.cvssv3) === 'I',
- ).length,
+ severityCounts.C,
+ severityCounts.H,
+ severityCounts.M,
+ severityCounts.L,
+ severityCounts.I,
],Line range hint 39-85: El manejo de errores es insuficiente
No es aceptable simplemente loggear el error a la consola. Los usuarios deben ser notificados cuando algo sale mal.
})
- .catch(console.error);
+ .catch(error => {
+ console.error('Error al cargar datos del audit:', error);
+ // TODO: Implementar un toast o notificación para el usuario
+ // toast.error(t('errors.audit_load_failed'));
+ });Line range hint 87-111: Considerar la accesibilidad del gráfico
El gráfico carece de elementos de accesibilidad. Deberías añadir un título descriptivo y textos alternativos para usuarios que utilizan lectores de pantalla.
const pieChartOptions = {
responsive: true,
maintainAspectRatio: false,
plugins: {
+ title: {
+ display: true,
+ text: t('dashboard.cvss_distribution'),
+ color: 'white'
+ },
legend: {
position: 'right' as const,
labels: {
color: 'white',
},
},
},
};
return (
<div className="bg-gray-800 rounded-lg p-6">
+ <div role="heading" aria-level={2} className="sr-only">
+ {t('dashboard.cvss_distribution')}
+ </div>
<div className="chart-container" style={{ height: '400px' }}>
<Pie data={pieChartData} options={pieChartOptions} />
</div>
</div>
);frontend/src/components/dashboard/CIATriad.tsx (1)
Line range hint 1-134: Problemas arquitectónicos en el componente
El componente tiene varios problemas que necesitan atención inmediata:
- La generación de colores aleatorios en tiempo de ejecución causará inconsistencias visuales en cada renderizado.
- La lógica de obtención de datos está acoplada directamente al componente.
- No hay manejo de estados de carga o error.
Recomiendo las siguientes mejoras:
- Extraer la generación de colores a una función determinista:
const generateColorForIndex = (index: number) => {
const colors = [
{ bg: 'rgba(255, 99, 132, 0.2)', border: 'rgba(255, 99, 132, 1)' },
{ bg: 'rgba(54, 162, 235, 0.2)', border: 'rgba(54, 162, 235, 1)' },
// Añadir más colores predefinidos...
];
return colors[index % colors.length];
};- Separar la lógica de datos usando un hook personalizado:
const useAuditData = (auditId: string) => {
const [data, setData] = useState<DataType | null>(null);
const [loading, setLoading] = useState(true);
const [error, setError] = useState<Error | null>(null);
useEffect(() => {
// Lógica de obtención de datos aquí
}, [auditId]);
return { data, loading, error };
};- Implementar estados de carga y error en el componente:
if (loading) return <LoadingSpinner />;
if (error) return <ErrorMessage message={error.message} />;frontend/src/components/dashboard/AverageCVSS.tsx (4)
Line range hint 1-17: ¡Las importaciones necesitan una mejor organización!
Las importaciones están desordenadas. Deberías agruparlas de la siguiente manera:
- Dependencias externas (React, chart.js)
- Componentes y utilidades internas
+// External dependencies
import {
BarElement,
CategoryScale,
Chart as ChartJS,
ChartOptions,
Legend,
LinearScale,
Title,
Tooltip,
} from 'chart.js';
import annotationPlugin from 'chartjs-plugin-annotation';
import React, { useEffect, useState } from 'react';
import { Bar } from 'react-chartjs-2';
import { useParams } from 'react-router-dom';
+// Internal imports
import { getAuditById } from '@/services/audits';
import { cvssStringToScore } from '@/lib/utils';Line range hint 28-35: ¡La lógica de manejo del auditId es redundante!
El código actual es innecesariamente complejo. La verificación del auditId se repite en el componente y en el useEffect.
Simplifica el código así:
-const paramId = useParams().auditId;
-if (!auditId) {
- auditId = paramId;
-}
+const { auditId: paramId } = useParams();
+const finalAuditId = auditId || paramId;Line range hint 47-77: ¡El manejo de errores es insuficiente y el código es demasiado denso!
La lógica de procesamiento de datos tiene varios problemas graves:
- El console.error no es suficiente para manejar errores
- Las operaciones anidadas hacen el código difícil de mantener
- No hay validación de datos antes de los cálculos
Deberías:
- Implementar un mejor manejo de errores
- Extraer la lógica de cálculo a funciones separadas
- Validar los datos antes de procesarlos
// Funciones auxiliares sugeridas:
const calculateAverageCVSS = (findings: Finding[]): number => {
if (!findings.length) return 0;
const sum = findings.reduce((acc, finding) => acc + cvssStringToScore(finding.cvssv3), 0);
return Math.round((sum / findings.length) * 10) / 10;
};
const getScoreColor = (score: number): string => {
if (score >= 9) return '#FF4136';
if (score >= 7) return '#FF851B';
if (score >= 4) return '#FFDC00';
return '#2ECC40';
};Line range hint 79-142: ¡La configuración del gráfico necesita mejoras importantes!
Hay varios problemas que necesitan atención:
- Las opciones del gráfico deberían estar fuera del componente
- No hay consideraciones de accesibilidad
- El contenedor tiene una altura fija que podría causar problemas en diferentes dispositivos
Recomendaciones:
- Mueve las opciones del gráfico a un archivo de configuración separado
- Agrega atributos aria para accesibilidad
- Considera usar unidades relativas para el tamaño del contenedor
// Ejemplo de configuración separada:
export const chartOptions: ChartOptions<'bar'> = {
// ... configuración actual ...
plugins: {
// ... plugins actuales ...
tooltip: {
callbacks: {
label: (context) => `CVSS Score: ${context.raw}`,
},
},
},
};frontend/src/services/exportToCSV.ts (4)
112-114: Usar forEach en lugar de map para realizar acciones
Se está utilizando map para iterar sobre findingScores y ejecutar una acción, pero map está diseñado para transformar arrays y devolver uno nuevo. En este caso, es más apropiado utilizar forEach para evitar confusiones.
Aplicar este cambio para reemplazar map por forEach:
-findingScores.map(fs =>
+findingScores.forEach(fs =>
csvRows.push(`average-cvss,${fs.label},${fs.value}`),
);148-152: Usar forEach en lugar de map para realizar acciones
Al igual que en el caso anterior, es preferible utilizar forEach en lugar de map cuando no se necesita crear un nuevo array.
Aplicar este cambio para reemplazar map por forEach:
-data.map(cia =>
+data.forEach(cia =>
csvRows.push(
`cia-triad,${cia.label},C${cia.C ?? 0} I${cia.I ?? 0} A${cia.A ?? 0}`,
),
);41-42: Mejorar la claridad de los nombres de variables
Las variables RC y RP pueden resultar poco descriptivas. Es recomendable utilizar nombres más significativos para mejorar la legibilidad del código.
Aplicar este cambio para renombrar las variables de manera más descriptiva:
-let RC = false;
-let RP = false;
+let hasRemediationComplexity = false;
+let hasRemediationPriority = false;Y actualizar las referencias a estas variables en el código correspondiente.
16-17: Considerar inicializar los contadores a cero de forma más eficiente
Los objetos complexityData y priorityData podrían inicializarse utilizando un método más dinámico si en el futuro se agregan más categorías.
Ejemplo de inicialización utilizando reduce:
const complexityCategories = ['easy', 'medium', 'complex'];
const priorityCategories = ['low', 'medium', 'high', 'urgent'];
-const complexityData = { easy: 0, medium: 0, complex: 0 };
-const priorityData = { low: 0, medium: 0, high: 0, urgent: 0 };
+const complexityData = complexityCategories.reduce((acc, category) => ({ ...acc, [category]: 0 }), {});
+const priorityData = priorityCategories.reduce((acc, category) => ({ ...acc, [category]: 0 }), {});📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
- frontend/src/components/dashboard/AverageCVSS.tsx (1 hunks)
- frontend/src/components/dashboard/CIATriad.tsx (2 hunks)
- frontend/src/components/dashboard/CVSSScore.tsx (1 hunks)
- frontend/src/components/dashboard/ExportModal.tsx (4 hunks)
- frontend/src/i18n/en-US/index.ts (3 hunks)
- frontend/src/lib/utils.ts (1 hunks)
- frontend/src/routes/audits/edit/dashboard/dashboard.tsx (4 hunks)
- frontend/src/services/exportToCSV.ts (1 hunks)
🔇 Additional comments (11)
frontend/src/components/dashboard/ExportModal.tsx (3)
Line range hint 1-13: ¡Los cambios en las importaciones y tipos son correctos!
La adición del hook useTranslation y la nueva propiedad type en la interfaz ExportModalProps están bien implementadas.
Line range hint 15-26: ¡La desestructuración de props es correcta!
La adición del prop type mantiene el orden y la estructura existente de las props.
27-28:
¡Necesitas manejar traducciones faltantes!
No hay manejo de errores para casos donde la traducción no exista. Deberías proporcionar un valor predeterminado.
Implementa el manejo de traducciones faltantes así:
- const { t } = useTranslation();
+ const { t } = useTranslation();
+ const fallbackTitle = 'Export Dashboard';Y en el título:
- <h2 className="text-2xl font-bold mb-4">{`${t('exportDashboard')} (${type})`}</h2>
+ <h2 className="text-2xl font-bold mb-4">{`${t('exportDashboard', fallbackTitle)} (${type})`}</h2>Likely invalid or redundant comment.
frontend/src/components/dashboard/CVSSScore.tsx (1)
9-9: ¡Excelente refactorización al utilizar la función de utilidad!
La extracción de la lógica CVSS a una función de utilidad mejora la mantenibilidad y reusabilidad del código.
frontend/src/components/dashboard/CIATriad.tsx (2)
14-14: ¡Excelente movimiento al centralizar la lógica CVSS!
La extracción de la función de procesamiento CVSS a un módulo de utilidades mejora significativamente la mantenibilidad del código. Esta refactorización permite la reutilización y reduce la duplicación de código.
69-71:
Verificar la compatibilidad con vectores CVSS malformados
La implementación actual podría ser vulnerable a vectores CVSS malformados. Es crucial manejar estos casos para evitar errores en tiempo de ejecución.
Necesitas agregar manejo de errores. Sugiero este cambio:
data: [
- cvssStringToCIA('integrity', finding.cvssv3),
- cvssStringToCIA('availability', finding.cvssv3),
- cvssStringToCIA('confidentiality', finding.cvssv3),
+ cvssStringToCIA('integrity', finding.cvssv3) ?? 0,
+ cvssStringToCIA('availability', finding.cvssv3) ?? 0,
+ cvssStringToCIA('confidentiality', finding.cvssv3) ?? 0,
],Ejecutemos una verificación para encontrar vectores CVSS potencialmente malformados:
frontend/src/routes/audits/edit/dashboard/dashboard.tsx (1)
134-134:
¡Validación de auditId ausente!
Estás usando el operador de coalescencia nula (??) con auditId, pero no hay validación real antes de las operaciones de exportación. Esto podría causar problemas si el ID no es válido.
Agrega una validación apropiada:
const handleExportConfirm = async () => {
+ if (!auditId) {
+ toast.error(t('err.invalidAuditId'));
+ return;
+ }
if (exportType === 'pdf') {
setIsExportModalOpen(false);
- await exportToPDF(auditName, selectedDisplays, auditId ?? '');
+ await exportToPDF(auditName, selectedDisplays, auditId);Likely invalid or redundant comment.
frontend/src/i18n/en-US/index.ts (2)
165-165: ¡Los mensajes de exportación están bien implementados!
Los mensajes de éxito y error para la exportación CSV son claros y consistentes con el objetivo del PR.
Also applies to: 206-206
738-738: ¡La etiqueta de exportación es correcta!
La etiqueta genérica "Export Dashboard" es apropiada y reutilizable para diferentes formatos de exportación (CSV, PDF).
frontend/src/lib/utils.ts (1)
5-5: El cambio en la declaración de la función cn es correcto.
Este cambio es consistente con las prácticas modernas de definición de funciones.
frontend/src/services/exportToCSV.ts (1)
118-133: Verificar la lógica de clasificación de severidad CVSS
Las abreviaturas utilizadas para las severidades ('C', 'H', 'M', 'L', 'I') pueden no corresponder con las convenciones estándar ('Critical', 'High', 'Medium', 'Low', 'None'). Asegúrese de que la función cvssStringToSeverity retorna valores que coincidan con estas abreviaturas y que la clasificación sea correcta.
Ejecute el siguiente script para verificar los valores devueltos por cvssStringToSeverity:
Si los valores no coinciden con las abreviaturas utilizadas, ajuste la lógica en las líneas 118-133 para reflejar las severidades correctas.
✅ Verification successful
La implementación de severidades CVSS es correcta y consistente
La función cvssStringToSeverity está correctamente implementada y retorna exactamente las abreviaturas ('C', 'H', 'M', 'L', 'I') que se utilizan en el código de exportación CSV. Las abreviaturas representan:
- 'C' (Critical): CVSS >= 9.0
- 'H' (High): CVSS >= 7.0
- 'M' (Medium): CVSS >= 4.0
- 'L' (Low): CVSS >= 0.1
- 'I' (Informational): Para vectores CVSS inválidos o puntuaciones de 0
El código es consistente en toda la base de código y sigue las convenciones estándar de CVSS v3.1.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Descripción: Verificar las severidades CVSS utilizadas en el código.
# Test: Buscar todas las llamadas a cvssStringToSeverity y listar los valores posibles.
# Esperado: Los valores deben ser 'Critical', 'High', 'Medium', 'Low', 'None'.
rg 'cvssStringToSeverity\(.+\)' -t js -A 0 -B 0 --no-heading | gawk -F '==' '{print $2}' | sort | uniq
Length of output: 102
Script:
#!/bin/bash
# Buscar la definición de la función cvssStringToSeverity
ast-grep --pattern 'function cvssStringToSeverity($_) {
$$$
}'
# Buscar también cualquier uso de las constantes de severidad
rg "['\"](C|H|M|L|I|Critical|High|Medium|Low|None)['\"]" -t ts -A 2 -B 2
# Buscar el archivo específico para ver el contexto completo
cat frontend/src/services/exportToCSV.ts
Length of output: 21636
Script:
#!/bin/bash
# Buscar la implementación de cvssStringToSeverity en lib/utils.ts
cat frontend/src/lib/utils.ts
Length of output: 1346
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (1)
frontend/src/services/exportToCSV.ts (1)
1-2: Las directivas ESLint deben estar justificadas
Las directivas para deshabilitar reglas de ESLint deberían incluir comentarios explicando por qué son necesarias. Si estas reglas no son realmente necesarias, deberían eliminarse.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- frontend/src/services/exportToCSV.ts (1 hunks)
🔇 Additional comments (1)
frontend/src/services/exportToCSV.ts (1)
168-180: ¡Bien implementada la lógica de descarga!
La implementación de la descarga del archivo sigue las mejores prácticas:
- Usa Blob con el tipo MIME correcto
- Limpia los elementos DOM temporales
- Revoca la URL del objeto después de su uso
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- frontend/src/routes/audits/edit/AuditRoot.tsx (1 hunks)
🔇 Additional comments (1)
frontend/src/routes/audits/edit/AuditRoot.tsx (1)
Line range hint 1-143: ¡Verificación necesaria de la integración con la exportación CSV!
El código actual no muestra ninguna implementación relacionada con la exportación CSV mencionada en los objetivos del PR. Necesitamos verificar cómo se integra esta funcionalidad.
caverav
dismissed stale reviews from coderabbitai[bot] and Icksir
via
6e9147a
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
frontend/src/lib/utils.ts(1 hunks)frontend/src/services/exportToCSV.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/services/exportToCSV.ts
🔇 Additional comments (3)
frontend/src/lib/utils.ts (3)
1-5: ¡Las importaciones y la función cn están correctamente implementadas!
La refactorización de la función cn a una función flecha mantiene la funcionalidad mientras mejora la consistencia del código.
29-37:
¡Persiste el problema con el manejo de errores!
Este problema ya fue señalado en revisiones anteriores. Retornar 0 silenciosamente cuando hay un error es una práctica peligrosa que puede causar cálculos incorrectos.
La solución ya fue propuesta anteriormente. Implementa este cambio de inmediato:
export const cvssStringToScore = (cvssScore: string) => {
try {
const cvssVector = new Cvss3P1(cvssScore);
return cvssVector.calculateExactOverallScore();
} catch (error) {
- console.error('Invalid CVSS vector:', error);
- }
- return 0;
+ throw new Error(`Vector CVSS inválido: ${error.message}`);
+ }
};7-27:
¡El manejo de errores es inadecuado!
El código actual oculta silenciosamente los errores y retorna 'I', lo que puede llevar a problemas difíciles de detectar en producción.
Implementa este cambio inmediatamente:
export const cvssStringToSeverity = (cvssScore: string) => {
try {
const cvssVector = new Cvss3P1(cvssScore);
const score = cvssVector.calculateExactOverallScore();
if (score >= 9.0) {
return 'C';
}
if (score >= 7.0) {
return 'H';
}
if (score >= 4.0) {
return 'M';
}
if (score >= 0.1) {
return 'L';
}
+ return 'I';
} catch (error) {
- console.error('Invalid CVSS vector:', error);
- }
- return 'I';
+ throw new Error(`Vector CVSS inválido: ${error.message}`);
+ }
};Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
frontend/src/routes/audits/edit/dashboard/dashboard.tsx(4 hunks)
🔇 Additional comments (1)
frontend/src/routes/audits/edit/dashboard/dashboard.tsx (1)
70-71:
¡El tipado actual es peligroso!
El uso de string vacío como tipo para exportType es propenso a errores. Necesitas implementar un tipo de unión estricto para garantizar que solo se puedan usar valores válidos.
Implementa este cambio inmediatamente:
- const [exportType, setExportType] = useState('');
+ type ExportType = 'pdf' | 'csv' | '';
+ const [exportType, setExportType] = useState<ExportType>('');Likely invalid or redundant comment.
Icksir
left a comment
Icksir
left a comment
There was a problem hiding this comment.
Me sigue funcionando bien
jllanosg
dismissed stale reviews from massi-ponce, Icksir, sealra, and coderabbitai[bot]
via
e2ac179
massi-ponce
left a comment
massi-ponce
left a comment
There was a problem hiding this comment.
LGTM (volví a buildear y a probar todo de nuevo por si acaso xd)
5 participants
Descripción
Permite exportar los datos del dashboard en formato CSV
Motivación y Contexto
HU07
¿Cómo ha sido probado?
Capturas de pantalla (si es apropiado):
N/A
Tipos de cambios
Lista de verificación:
Summary by CodeRabbit
Nuevas Funciones
Mejoras en la Localización
Corrección de Errores
Mejoras en la Interfaz de Usuario