Skip to content
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

Add New Chart Components and Client Dashboard #160

Merged
merged 101 commits into from
Nov 5, 2024

Conversation

caverav
Copy link
Owner

@caverav caverav commented Nov 2, 2024

Descripción

Este pull request introduce varios nuevos componentes de gráficos y un dashboard de clientes a la aplicación frontend. Los cambios incluyen:

  1. Nuevas Dependencias:

    • Añadido @visx/text y @visx/wordcloud a package.json.
  2. Nuevos Componentes de Gráficos:

    • CIATriadChart: Un gráfico de radar para mostrar datos del triángulo CIA.
    • CVSSChart: Un gráfico de barras para mostrar puntuaciones CVSS.
    • CWECloud: Una nube de palabras para mostrar datos CWE.
    • RemediationPriorityChart: Un gráfico de barras para mostrar prioridades de remediación.
    • SeverityPieChart: Un gráfico de dona para mostrar vulnerabilidades por severidad.
    • TimePerAuditChart: Un gráfico de barras para mostrar el tiempo por auditoría.
  3. Integración del Dashboard:

    • Creado un nuevo componente ClientDashboard que integra todos los nuevos componentes de gráficos.
    • Añadida una nueva ruta /dashboard para acceder al dashboard de clientes.
    • Actualizado el Navbar para incluir un enlace al dashboard.
  4. Actualizaciones de Servicios:

    • Añadida la función getAuditsByClientName en clients.ts para obtener auditorías por nombre de cliente.
    • Actualizado audits.ts para hacer opcionales los campos remediationComplexity y priority.
  5. Actualizaciones de Componentes:

    • Modificado el componente AverageCVSS para soportar la obtención de datos por nombre de cliente.

Motivación y Contexto

HU16

¿Cómo ha sido probado?

  • Asegurarse de que todos los nuevos componentes de gráficos se rendericen correctamente con datos simulados.
  • Verificar que el dashboard de clientes muestre datos precisos para los clientes seleccionados.
  • Probar la nueva ruta y el enlace de navegación al dashboard.

Tipos de cambios

  • Bugfix (cambio que no interrumpe el funcionamiento y que soluciona un problema)
  • New feature (cambio que no interrumpe el funcionamiento y que añade funcionalidad)
  • Breaking change (corrección o funcionalidad que podría causar que la funcionalidad existente cambie)

Lista de verificación:

  • Mi código sigue el estilo de código de este proyecto.
  • Mi cambio requiere una modificación en la documentación.
  • He actualizado la documentación en consecuencia.
  • Requiere nuevos tests.

Summary by CodeRabbit

  • Nuevas Funciones

    • Se han añadido varias visualizaciones de gráficos, incluyendo gráficos de radar, de barras, de nubes de palabras y gráficos de pastel, para mostrar métricas relacionadas con auditorías y clientes.
    • Se ha implementado una nueva opción de navegación para acceder al panel de control del cliente.
    • Se ha añadido la capacidad de calcular el puntaje promedio de CVSS basado en el nombre del cliente o el ID de la auditoría.
    • Se ha introducido un nuevo componente de panel de control que muestra métricas de auditoría y datos relacionados con los clientes.
    • Se ha añadido un nuevo componente de Tooltip para mejorar la interacción del usuario.
  • Mejoras

    • Se ha mejorado la lógica de procesamiento de prioridades en el componente de RemediationPriority para evitar errores al comparar valores indefinidos.
    • Se ha mejorado la robustez del manejo de datos en los componentes RemediationComplexity y AverageCVSS al permitir propiedades opcionales.
    • Se han añadido nuevas cadenas de texto para mejorar la localización y la claridad de la interfaz de usuario.
  • Correcciones de Errores

    • Se han modificado propiedades en el tipo Finding para permitir campos opcionales, mejorando la flexibilidad del manejo de datos.

caverav and others added 30 commits October 26, 2024 20:12
- Updated the `AverageCVSS` component to accept and handle `clientName`.
- Modified `ClientDashboard` to pass `clientName` to `AverageCVSS` and set it correctly.
- Created a new service `getAuditsByClientName` to fetch audits based on the client's name.
… component

- Added "recharts" library with version "^2.13.3" to the frontend/package.json file.
- Created a new React functional component CIATriadChart in frontend/src/components/charts/CIATriadChart.tsx that displays a radar chart representing the average CIA triad data.
- The component takes an array of CIAData objects containing subject, current, and target properties as props and renders a RadarChart using the recharts library.
- The RadarChart includes PolarGrid, PolarAngleAxis, and two Radar components for "Current" and "Target" data with corresponding colors and styles.
This commit introduces two new components:
1. **CVSSChart**: A React functional component for rendering a bar chart displaying CVSS data, including average CVSS score.
2. **CWECloud**: Another React functional component for displaying Common Weakness Enumeration (CWE) items in a cloud format, highlighting the most common CWE found.
This commit refactors the CIATriadChart component to include additional configurations for the 'r' scale such as angle lines, suggested min/max values, ticks, point labels, and grid styling. It also enhances the plugin settings for legend and tooltip in the chart.

In the ClientDashboard component, the commit introduces changes to calculate the CIA data dynamically based on the findings retrieved from audits. This includes initializing temporary CIA data objects, updating their current values, and computing averages. Finally, the updated CIA data is set in the component state for further use.
@massi-ponce
Copy link
Collaborator

Encontré algunas cosas en cuanto a los CIA triad (tanto el "Average CIA triad" presente en los dashboard por cliente, como el "CIA triad" de audits).

El CIA triad que están en los dashboard de una auditoría tiene 4 niveles, siendo estos: 0 (None), 1 (Low), 2 (NO EXISTE) y 3 (High).
imagen

Mientras que el CIA triad que está en los dashboard por cliente tiene 3 niveles, siendo estos: 0 (None), 1 (Low) y 2 (High).
imagen

Además los nombres están mal puestos, ya que no representan lo que deberían.

Ejemplo para evidenciarlo

Primero creo una nueva auditoría y le agrego un finding el cuál tiene:
imagen

Si vemos el dashboard "CIA triad" de este audit recién creado, notamos que C debería ser High, I debería ser Low y A debería ser None. Sin embargo, aparece que C es None, I es High y A es Low.
imagen

Esto también pasa en "Average CIA triad" de dashboard por cliente, ya que también aparece que C es None, I es High y A es Low, solo que los labels están en diferente orden.
imagen

Esto quiere decir que:

  • Confidentiality la está graficando en Integrity.
  • Integrity la está graficando en Availability.
  • Availability la está graficando en Confidentiality.

Ahora, en cuanto al cálculo del promedio de cada atributo CIA.

Comprobé si el hecho de que en el CIA triad de audit tenga 4 niveles y el Average CIA triad" tenga 3 generaba problemas en el cálculo del promedio de cada atributo CIA, y al parecer no, eso funciona bien. Asi que el problema es simplemente esa inconsistencia en los nombres y lo que se está graficando en cada uno.

….tsx to match confidentiality, integrity, availability order.
@caverav
Copy link
Owner Author

caverav commented Nov 4, 2024

Encontré algunas cosas en cuanto a los CIA triad (tanto el "Average CIA triad" presente en los dashboard por cliente, como el "CIA triad" de audits).

El CIA triad que están en los dashboard de una auditoría tiene 4 niveles, siendo estos: 0 (None), 1 (Low), 2 (NO EXISTE) y 3 (High). imagen

Mientras que el CIA triad que está en los dashboard por cliente tiene 3 niveles, siendo estos: 0 (None), 1 (Low) y 2 (High). imagen

Además los nombres están mal puestos, ya que no representan lo que deberían.

Ejemplo para evidenciarlo

Primero creo una nueva auditoría y le agrego un finding el cuál tiene: imagen

Si vemos el dashboard "CIA triad" de este audit recién creado, notamos que C debería ser High, I debería ser Low y A debería ser None. Sin embargo, aparece que C es None, I es High y A es Low. imagen

Esto también pasa en "Average CIA triad" de dashboard por cliente, ya que también aparece que C es None, I es High y A es Low, solo que los labels están en diferente orden. imagen

Esto quiere decir que:

* Confidentiality la está graficando en Integrity.

* Integrity la está graficando en Availability.

* Availability la está graficando en Confidentiality.

Ahora, en cuanto al cálculo del promedio de cada atributo CIA.

Comprobé si el hecho de que en el CIA triad de audit tenga 4 niveles y el Average CIA triad" tenga 3 generaba problemas en el cálculo del promedio de cada atributo CIA, y al parecer no, eso funciona bien. Asi que el problema es simplemente esa inconsistencia en los nombres y lo que se está graficando en cada uno.

Arreglado el orden

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 7

🧹 Outside diff range and nitpick comments (4)
frontend/src/components/charts/CWECloud.tsx (1)

26-41: ¡La implementación del grid necesita ser más robusta!

  1. Los breakpoints están hardcodeados en las clases de Tailwind
  2. La key del map solo usa item.id, lo cual podría causar problemas si hay IDs duplicados
  3. No hay límite en el número de items mostrados, lo que podría afectar el rendimiento

Considera estas mejoras:

-      <div className="grid grid-cols-1 sm:grid-cols-2 md:grid-cols-3 gap-4">
+      <div className={clsx(
+        'grid gap-4',
+        'grid-cols-1',
+        'sm:grid-cols-2',
+        'md:grid-cols-3',
+        items.length > 9 && 'overflow-y-auto max-h-[500px]'
+      )}>
         {items.map(item => (
           <div
             className="bg-gray-800 p-4 rounded-lg shadow-md flex items-center"
-            key={item.id}
+            key={`${item.id}-${item.size}`}
frontend/src/components/dashboard/CIATriad.tsx (3)

Line range hint 19-31: Mejora necesaria en el manejo de valores CVSS inválidos

La función cvssStringTo no valida si el valor extraído del vector CVSS es válido antes de acceder al objeto values. Esto podría causar errores silenciosos si el vector CVSS está malformado.

Aplica este diff para agregar validación:

const cvssStringTo = (
  field: 'integrity' | 'availability' | 'confidentiality',
  cvssVector: string,
) => {
  const values: Record<string, number> = {
    H: 3,
    M: 2,
    L: 1,
  } as const;
  const substrings = {
    confidentiality: 35,
    integrity: 39,
    availability: 43,
  } as const;
+ const value = cvssVector.substring(substrings[field], substrings[field] + 1);
+ if (!Object.keys(values).includes(value)) {
+   console.error(`Valor CVSS inválido: ${value} para el campo ${field}`);
+   return 0;
+ }
- return values[cvssVector.substring(substrings[field], substrings[field] + 1)];
+ return values[value];
};

Line range hint 66-92: Optimización necesaria en el manejo de datos

El código actual regenera colores aleatorios cada vez que se actualiza el estado, lo que puede causar parpadeo visual. Además, la lógica de mapeo se puede simplificar.

Considera estas mejoras:

+ const generateColor = (index: number) => {
+   // Usar una semilla basada en el índice para generar colores consistentes
+   const r = ((index * 123) % 155) + 100;
+   const g = ((index * 456) % 155) + 100;
+   const b = ((index * 789) % 155) + 100;
+   return `rgba(${r}, ${g}, ${b}, 0.2)`;
+ };

  useEffect(() => {
    getAuditById(auditId)
      .then(audit => {
        setData({
          labels: ['Integrity', 'Availability', 'Confidentiality'],
          datasets: audit.datas.findings.map((finding, index) => ({
            label: finding.title,
            data: finding.cvssv3
              ? [
                  cvssStringTo('integrity', finding.cvssv3),
                  cvssStringTo('availability', finding.cvssv3),
                  cvssStringTo('confidentiality', finding.cvssv3),
                ]
              : [0, 0, 0],
-           backgroundColor: `rgba(${Math.floor(Math.random() * 155 + 100)}, ...`,
+           backgroundColor: generateColor(index),
            borderColor: 'rgba(255, 255, 255, 0.2)',
            borderWidth: 2,
          })),
        });
      })
      .catch(error => {
+       console.error('Error al cargar datos del audit:', error);
+       setData({
+         labels: ['Integrity', 'Availability', 'Confidentiality'],
+         datasets: [],
+       });
      });
  }, [auditId]);

Line range hint 47-48: Validación de ID faltante

No se está validando que el auditId sea una cadena no vacía antes de hacer la llamada a la API.

Agrega validación:

  const paramId = useParams().auditId;
  if (!auditId) {
    auditId = paramId;
  }
+ if (!auditId?.trim()) {
+   console.error('ID de auditoría no válido');
+   return <div className="text-red-500">ID de auditoría no válido</div>;
+ }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 455ddd4 and 34223b9.

📒 Files selected for processing (5)
  • frontend/package.json (2 hunks)
  • frontend/src/components/charts/CWECloud.tsx (1 hunks)
  • frontend/src/components/dashboard/CIATriad.tsx (1 hunks)
  • frontend/src/i18n/en-US/index.ts (1 hunks)
  • frontend/src/routes/dashboard/dashboard.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • frontend/package.json
  • frontend/src/i18n/en-US/index.ts
🔇 Additional comments (2)
frontend/src/routes/dashboard/dashboard.tsx (2)

36-44: ⚠️ Potential issue

¡El manejo de errores en cvssStringToScore es inaceptablemente débil!

La función silenciosamente retorna 0 cuando hay un error, lo que puede llevar a cálculos incorrectos y ocultar problemas graves.

Implementa un manejo de errores apropiado:

 const cvssStringToScore = (cvssScore: string) => {
+  if (!cvssScore?.trim()) {
+    throw new Error('El vector CVSS no puede estar vacío');
+  }
   try {
     const cvssVector = new Cvss3P1(cvssScore);
     return cvssVector.calculateExactOverallScore();
   } catch (error) {
-    console.error('Invalid CVSS vector:', error);
+    throw new Error(`Vector CVSS inválido: ${error.message}`);
   }
-  return 0;
 };

Likely invalid or redundant comment.


46-58: 🛠️ Refactor suggestion

¡La función severityByScore necesita una refactorización urgente!

Los problemas incluyen:

  1. Uso de números mágicos para los umbrales
  2. Retorno de índices numéricos crípticos
  3. Falta de tipado seguro para los valores de retorno

Implementa esta versión mejorada:

+enum Severity {
+  Critical = 0,
+  High = 1,
+  Medium = 2,
+  Low = 3,
+  Informative = 4,
+}
+
+const SEVERITY_THRESHOLDS = {
+  CRITICAL: 9.0,
+  HIGH: 7.0,
+  MEDIUM: 4.0,
+  LOW: 0.1,
+} as const;
+
-const severityByScore = (score: number) => {
+const severityByScore = (score: number): Severity => {
   if (score >= 9.0) {
-    return 0;
+    return Severity.Critical;
   } else if (score >= 7.0) {
-    return 1;
+    return Severity.High;
   } else if (score >= 4.0) {
-    return 2;
+    return Severity.Medium;
   } else if (score >= 0.1) {
-    return 3;
+    return Severity.Low;
   } else {
-    return 4;
+    return Severity.Informative;
   }
 };

Likely invalid or redundant comment.

frontend/src/components/charts/CWECloud.tsx Outdated Show resolved Hide resolved
frontend/src/components/charts/CWECloud.tsx Outdated Show resolved Hide resolved
frontend/src/routes/dashboard/dashboard.tsx Outdated Show resolved Hide resolved
frontend/src/routes/dashboard/dashboard.tsx Show resolved Hide resolved
coderabbitai[bot]
coderabbitai bot previously approved these changes Nov 4, 2024
@massi-ponce
Copy link
Collaborator

Creo que, la utilidad del gráfico "Average CVSS", es mostrar el promedio de los CVSS que tiene el cliente entre todas sus auditorias
imagen

Tal como lo es el de los dashboards por auditoria:
imagen

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Outside diff range and nitpick comments (7)
frontend/src/components/dashboard/CVSSScore.tsx (2)

Line range hint 12-29: ¡La función necesita mejor manejo de errores y tipos!

La implementación actual de cvssStringToSeverity podría ser más robusta. Es necesario:

  1. Agregar tipos TypeScript específicos para el parámetro y el valor de retorno
  2. Mejorar el manejo de errores para casos específicos de CVSS inválidos
  3. Documentar los niveles de severidad y sus rangos

Te sugiero aplicar estos cambios:

-const cvssStringToSeverity = (cvssScore: string) => {
+type SeverityLevel = 'C' | 'H' | 'M' | 'L' | 'I';
+
+/**
+ * Convierte un vector CVSS a un nivel de severidad
+ * @param cvssScore - Vector CVSS v3.1
+ * @returns Nivel de severidad:
+ *  - C: Crítico (9.0-10.0)
+ *  - H: Alto (7.0-8.9)
+ *  - M: Medio (4.0-6.9)
+ *  - L: Bajo (0.1-3.9)
+ *  - I: Informativo (0.0 o inválido)
+ */
+const cvssStringToSeverity = (cvssScore: string): SeverityLevel => {
   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';
     }
   } catch (error) {
-    console.error('Invalid CVSS vector:', error);
+    console.error(`Vector CVSS inválido "${cvssScore}":`, error);
   }
   return 'I';
 };

75-87: ¡Buen manejo de valores nulos, pero podemos mejorarlo!

La implementación del operador de coalescencia nula (??) es correcta y consistente. Sin embargo, podríamos mejorar la seguridad de tipos.

Sugiero agregar una interfaz para el objeto finding:

interface Finding {
  cvssv3: string | undefined;
  // ... otros campos
}

Esto haría el código más mantenible y type-safe.

frontend/src/routes/audits/edit/AuditRoot.tsx (3)

Line range hint 41-63: La función cvssStringToSeverity necesita una mejor gestión de errores

La función actual tiene varias deficiencias:

  1. Solo registra el error en la consola y continúa
  2. Retorna 'I' silenciosamente en caso de error
  3. No valida la entrada antes de crear el objeto Cvss3P1

Te sugiero implementar una gestión de errores más robusta:

 const cvssStringToSeverity = (cvssScore: string) => {
+  if (!cvssScore.trim()) {
+    console.warn('Vector CVSS vacío o inválido');
+    return 'I';
+  }
   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';
     }
   } catch (error) {
-    console.error('Invalid CVSS vector:', error);
+    console.error(`Error al procesar vector CVSS "${cvssScore}":`, error);
+    throw new Error(`Vector CVSS inválido: ${cvssScore}`);
   }
-  return 'I';
 };

Line range hint 106-115: La transformación de findings podría ser más robusta

El mapeo actual de findings no maneja adecuadamente casos extremos y podría beneficiarse de valores por defecto más seguros.

Considera esta implementación más defensiva:

 setFindings(
   audit.datas.findings.map((finding: Finding) => {
     return {
-      id: finding.identifier,
-      name: finding.title,
-      category: 'No Category',
-      severity: cvssStringToSeverity(finding.cvssv3 ?? ''),
-      identifier: finding._id,
+      id: finding.identifier ?? 'sin-id',
+      name: finding.title?.trim() || 'Sin título',
+      category: finding.category?.trim() || 'Sin Categoría',
+      severity: finding.cvssv3 ? cvssStringToSeverity(finding.cvssv3) : 'I',
+      identifier: finding._id ?? `temp-${Date.now()}`,
     };
   }),
 );

Line range hint 65-89: Considera manejar errores de red de forma más elegante

El uso de catch(console.error) es demasiado simplista para una aplicación en producción.

Implementa un mejor manejo de errores:

 useEffect(() => {
   getCustomSections()
     .then(section => {
       setSections(
         section.datas.map((item: Section) => ({
           field: item.field,
           name: item.name,
           icon: item.icon,
         })),
       );
     })
-    .catch(console.error);
+    .catch(error => {
+      console.error('Error al obtener secciones:', error);
+      // TODO: Mostrar un mensaje de error al usuario
+      setSections([]);
+    });
 }, []);
frontend/src/components/charts/CVSSChart.tsx (2)

100-100: Internacionaliza el texto "Average CVSS"

El texto "Average CVSS" está hardcoded en inglés en la línea 100. Deberías utilizar la función t() para permitir la traducción y soportar múltiples idiomas.

Aplica este cambio:

-            Average CVSS: {averageCVSS}
+            {t('averageCVSS')}: {averageCVSS}

Asegúrate de agregar la clave averageCVSS en tus archivos de traducción.


100-100: El color del texto podría afectar la accesibilidad

El uso de text-green-400 puede no ofrecer suficiente contraste para todos los usuarios. Considera utilizar un tono más oscuro, como text-green-700, para mejorar la legibilidad y cumplir con las pautas de accesibilidad.

Aplica este cambio:

-          <div className="absolute top-0 left-0 w-full text-right pr-4 text-green-400 text-sm">
+          <div className="absolute top-0 left-0 w-full text-right pr-4 text-green-700 text-sm">
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 34223b9 and be063ee.

📒 Files selected for processing (7)
  • frontend/src/components/charts/CVSSChart.tsx (1 hunks)
  • frontend/src/components/dashboard/AverageCVSS.tsx (3 hunks)
  • frontend/src/components/dashboard/CIATriad.tsx (2 hunks)
  • frontend/src/components/dashboard/CVSSScore.tsx (1 hunks)
  • frontend/src/routes/audits/edit/AuditRoot.tsx (1 hunks)
  • frontend/src/routes/dashboard/dashboard.tsx (1 hunks)
  • frontend/src/services/audits.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • frontend/src/components/dashboard/AverageCVSS.tsx
  • frontend/src/components/dashboard/CIATriad.tsx
  • frontend/src/services/audits.ts
🔇 Additional comments (6)
frontend/src/routes/audits/edit/AuditRoot.tsx (1)

112-112: ¡Bien hecho con el manejo de nulos!

La adición del operador de coalescencia nula (??) es una mejora necesaria para prevenir errores cuando cvssv3 es undefined o null.

frontend/src/routes/dashboard/dashboard.tsx (5)

1-21: ¡Las importaciones y definiciones de tipos están correctas!

La estructura de importaciones está bien organizada y el tipo ListItem está bien definido.


36-44: ⚠️ Potential issue

¡El manejo de errores en cvssStringToScore es inadecuado!

La función:

  1. Retorna 0 silenciosamente cuando hay un error
  2. Solo registra el error en consola
  3. No valida la entrada

Implementa esta corrección:

 const cvssStringToScore = (cvssScore: string) => {
+  if (!cvssScore?.trim()) {
+    throw new Error('Vector CVSS no puede estar vacío');
+  }
   try {
     const cvssVector = new Cvss3P1(cvssScore);
     return cvssVector.calculateExactOverallScore();
   } catch (error) {
-    console.error('Invalid CVSS vector:', error);
+    throw new Error(`Vector CVSS inválido: ${error.message}`);
   }
-  return 0;
 };

Likely invalid or redundant comment.


110-121: ⚠️ Potential issue

¡El manejo de errores y estados de carga es deficiente!

La función fetchClients tiene varias deficiencias críticas:

  1. No hay gestión del estado de carga
  2. El manejo de errores es básico
  3. No hay feedback visual para el usuario

Implementa un mejor manejo de estados:

+const [isLoading, setIsLoading] = useState(false);
+const [error, setError] = useState<Error | null>(null);

 const fetchClients = async () => {
+  setIsLoading(true);
+  setError(null);
   try {
     const data = await getCompanies();
     const clientName: ListItem[] = data.datas.map((cliente, index) => ({
       id: index,
       value: cliente.name,
     }));
     setClientName(clientName);
   } catch (error) {
-    console.error('Error fetching audits:', error);
+    setError(error as Error);
+    console.error('Error al obtener clientes:', error);
+  } finally {
+    setIsLoading(false);
   }
 };

Likely invalid or redundant comment.


46-58: 🛠️ Refactor suggestion

¡La función severityByScore necesita una refactorización urgente!

La implementación actual:

  1. Usa números mágicos para los umbrales
  2. Retorna índices numéricos crípticos
  3. No tiene seguridad de tipos para los valores de retorno

Implementa esta mejora:

+enum Severity {
+  Critical = 0,
+  High = 1,
+  Medium = 2,
+  Low = 3,
+  Informative = 4,
+}
+
+const SEVERITY_THRESHOLDS = {
+  CRITICAL: 9.0,
+  HIGH: 7.0,
+  MEDIUM: 4.0,
+  LOW: 0.1,
+} as const;
+
-const severityByScore = (score: number) => {
+const severityByScore = (score: number): Severity => {
   if (score >= 9.0) {
-    return 0;
+    return Severity.Critical;
   } else if (score >= 7.0) {
-    return 1;
+    return Severity.High;
   } else if (score >= 4.0) {
-    return 2;
+    return Severity.Medium;
   } else if (score >= 0.1) {
-    return 3;
+    return Severity.Low;
   } else {
-    return 4;
+    return Severity.Informative;
   }
 };

Likely invalid or redundant comment.


23-34: ⚠️ Potential issue

¡La función cvssStringTo es extremadamente frágil!

La implementación actual es peligrosa porque:

  1. Usa índices hardcodeados que se romperán si cambia el formato del vector CVSS
  2. No valida la entrada
  3. No maneja casos donde el substring está fuera de rango

Implementa esta solución más robusta:

-const cvssStringTo = (
+const cvssStringTo = (
   field: 'integrity' | 'availability' | 'confidentiality',
   cvssVector: string,
 ) => {
+  if (!cvssVector?.trim()) {
+    throw new Error('Vector CVSS no puede estar vacío');
+  }
+
+  const metrics = cvssVector.split('/');
+  const metricMap = {
+    integrity: 'I',
+    availability: 'A',
+    confidentiality: 'C',
+  };
+
   const values: Record<string, number> = { H: 2, L: 1, N: 0 } as const;
-  const substrings = {
-    confidentiality: 35,
-    integrity: 39,
-    availability: 43,
-  } as const;
-  return values[cvssVector.substring(substrings[field], substrings[field] + 1)];
+  
+  const metric = metrics.find(m => m.startsWith(metricMap[field]));
+  if (!metric) {
+    throw new Error(`Métrica ${field} no encontrada en el vector CVSS`);
+  }
+  
+  const value = metric.split(':')[1];
+  if (!(value in values)) {
+    throw new Error(`Valor inválido ${value} para la métrica ${field}`);
+  }
+  
+  return values[value];
 };

Likely invalid or redundant comment.

massi-ponce
massi-ponce previously approved these changes Nov 4, 2024
Copy link
Collaborator

@massi-ponce massi-ponce left a comment

Choose a reason for hiding this comment

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

LGTM

- Added `@radix-ui/react-tooltip` dependency in `package.json`.
- Imported Tooltip components in CWECloud chart.
- Implemented TooltipProvider, Tooltip, TooltipTrigger, and TooltipContent in the CWECloud component for displaying tooltips when hovering over items in the chart.
Copy link
Collaborator

@massi-ponce massi-ponce left a comment

Choose a reason for hiding this comment

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

LGTM x2

Copy link
Collaborator

@Sealra Sealra left a comment

Choose a reason for hiding this comment

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

LGTM !!!

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (3)
frontend/src/components/ui/tooltip.tsx (2)

6-10: ¡Buena implementación de los componentes base!

La re-exportación de los primitivos de Radix UI es una práctica común y correcta. Esto permite:

  • Centralizar la configuración de los componentes
  • Facilitar cambios futuros sin afectar a los consumidores
  • Mantener una API consistente

12-26: ¡La implementación necesita mejor organización!

El componente funciona correctamente, pero la cadena de clases es demasiado larga y difícil de mantener.

Sugiero extraer las clases a una constante separada:

+const tooltipContentStyles = 'z-50 overflow-hidden rounded-md border bg-popover px-3 py-1.5 text-sm text-popover-foreground shadow-md animate-in fade-in-0 zoom-in-95 data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2'

const TooltipContent = React.forwardRef<
  React.ElementRef<typeof TooltipPrimitive.Content>,
  React.ComponentPropsWithoutRef<typeof TooltipPrimitive.Content>
>(({ className, sideOffset = 4, ...props }, ref) => (
  <TooltipPrimitive.Content
    ref={ref}
    sideOffset={sideOffset}
-    className={cn(
-      'z-50 overflow-hidden rounded-md border bg-popover px-3 py-1.5 text-sm text-popover-foreground shadow-md animate-in fade-in-0 zoom-in-95 data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2',
-      className,
-    )}
+    className={cn(tooltipContentStyles, className)}
    {...props}
  />
));
frontend/src/components/charts/CWECloud.tsx (1)

1-10: ¡La organización de las importaciones necesita ser más estructurada!

Las importaciones deberían estar agrupadas de la siguiente manera:

  1. Dependencias externas (React, i18next)
  2. Componentes de UI (Radix)
  3. Iconos
  4. Tipos y utilidades
+// External dependencies
 import { t } from 'i18next';
 import React from 'react';
-import { FaBug } from 'react-icons/fa';
 
+// UI Components
 import {
   Tooltip,
   TooltipContent,
   TooltipProvider,
   TooltipTrigger,
 } from '../ui/tooltip';
+
+// Icons
+import { FaBug } from 'react-icons/fa';
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between be063ee and 24f70b1.

📒 Files selected for processing (3)
  • frontend/package.json (2 hunks)
  • frontend/src/components/charts/CWECloud.tsx (1 hunks)
  • frontend/src/components/ui/tooltip.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • frontend/package.json
🔇 Additional comments (4)
frontend/src/components/ui/tooltip.tsx (2)

1-4: ¡Las importaciones están correctamente organizadas!

La organización de las importaciones es clara y todas son necesarias para la implementación del componente.


28-28: ¡Las exportaciones están correctamente definidas!

Todos los componentes necesarios están disponibles para su uso en la aplicación.

frontend/src/components/charts/CWECloud.tsx (2)

12-20: ¡Referencia a comentario anterior sobre tipos!

El comentario anterior sobre la necesidad de restricciones más estrictas en los tipos y documentación adicional sigue siendo válido.


35-58: ⚠️ Potential issue

¡La implementación no coincide con el nombre del componente!

Este componente se llama CWECloud pero está implementado como una cuadrícula. Esto viola el principio de menor sorpresa y no cumple con los objetivos del PR que mencionan el uso de @visx/wordcloud.

Debes:

  1. Implementar una verdadera nube de palabras usando @visx/wordcloud
  2. O renombrar el componente a CWEGrid para reflejar su implementación actual

frontend/src/components/charts/CWECloud.tsx Show resolved Hide resolved
frontend/src/components/charts/CWECloud.tsx Show resolved Hide resolved
@caverav caverav merged commit f1cbc31 into development Nov 5, 2024
3 of 4 checks passed
@caverav caverav deleted the feature/client-dashboard branch November 5, 2024 01:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants