Adds Vulnerability Types and Categories to Custom Data#177
Adds Vulnerability Types and Categories to Custom Data#177caverav merged 60 commits intodevelopmentfrom
Conversation
This commit adds the `VulnerabilityCategories` component along with its associated functionality for managing categories and sorting options. The component allows users to create, update, and delete vulnerability categories. It includes features such as adding new categories, updating category lists, and saving changes. Additionally, a `CategoryList` component is introduced to display and manage the list of vulnerability categories, including drag-and-drop functionality for reordering items. The `VulnerabilityCategories` component integrates with translation functionality, error handling, and toast notifications for user feedback. Users can input category names, specify sorting options, and interact with the category list interface. This enhancement provides a comprehensive solution for managing vulnerability categories within the application.
… component in frontend/src/components/ui/switch.tsx
…ate in VulnerabilityCategoriesList
…e in VulnerabilityCategories component.
📝 Walkthrough📝 WalkthroughWalkthroughSe han realizado cambios en el proyecto Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Warning Rate limit exceeded@massi-ponce has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 2 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
frontend/src/routes/data/CustomData/VulnerabilityCategories.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "@typescript-eslint/eslint-plugin". (The package "@typescript-eslint/eslint-plugin" was not found when loaded as a Node module from the directory "/frontend".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "@typescript-eslint/eslint-plugin" was referenced from the config file in "frontend/.eslintrc.cjs". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. 📜 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 (
|
massi-ponce
requested review from
Icksir,
caverav,
iTzGooDLife,
jllanosg and
sealra
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Outside diff range and nitpick comments (18)
frontend/src/components/ui/switch.tsx (3)
1-4: ¡Optimiza las importaciones de tipos!Deberías separar las importaciones de tipos para mejorar el rendimiento en tiempo de ejecución. Es una práctica recomendada en TypeScript.
Aplica este cambio:
-import * as React from 'react'; +import { forwardRef, type ElementRef, type ComponentPropsWithoutRef } from 'react';
6-9: ¡La documentación es obligatoria!El componente carece de documentación JSDoc. En un componente de UI reutilizable, es crucial documentar su propósito y props.
Agrega esta documentación:
+/** + * Un componente Switch accesible basado en Radix UI. + * Proporciona una interfaz toggle con estados checked/unchecked. + * + * @component + * @example + * <Switch + * checked={isEnabled} + * onCheckedChange={setIsEnabled} + * /> + */ const Switch = React.forwardRef<
25-27: ¡Exporta el tipo del componente!Deberías exportar el tipo del componente para facilitar su uso en otros archivos.
Agrega esta exportación de tipo:
+export type SwitchProps = React.ComponentPropsWithoutRef<typeof SwitchPrimitives.Root>; export { Switch };frontend/src/routes/data/CustomData.tsx (1)
Line range hint
82-88: ¡La lógica de renderizado es inconsistente!El componente
CustomFieldstiene un tratamiento especial en el renderizado que rompe con el patrón establecido para los demás componentes. Esto dificulta el mantenimiento y reduce la consistencia del código.Sugerencia: Eliminar el renderizado condicional y tratar
CustomFieldscomo los demás componentes:<div className="mt-4"> - {cardContent.id === 6 ? ( - <CustomFields /> - ) : ( <Card title={cardContent.cardTitle}>{cardContent.cardChildren}</Card> - )} </div>frontend/src/i18n/en-US/index.ts (1)
759-759: ¡La nueva entrada debería estar agrupada con las demás traducciones relacionadas!La entrada
vulnerabilityTypedebería estar agrupada con las demás traducciones relacionadas con vulnerabilidades para mantener una mejor organización del código.Considera mover esta entrada cerca de las otras traducciones relacionadas con vulnerabilidades, alrededor de la línea 170.
frontend/src/routes/data/CustomData/VulnerabilityCategoriesList.tsx (4)
167-173: Mejorar la eficiencia al comparar objetosUsar
JSON.stringifypara comparar objetos es ineficiente y puede causar problemas si las propiedades no están en el mismo orden. Esto puede afectar el rendimiento de la aplicación.Reemplaza la comparación por una función de comparación profunda:
+ import isEqual from 'lodash/isEqual'; useEffect(() => { const updatedData = rows.map(({ id, ...rest }) => rest); - if (JSON.stringify(updatedData) !== JSON.stringify(data)) { + if (!isEqual(updatedData, data)) { onUpdateList(updatedData); } }, [rows, data, onUpdateList]);Esto garantiza una comparación más confiable y eficiente.
89-92: Mejorar la accesibilidad del icono de arrastreEl icono de arrastre carece de una etiqueta aria, lo que puede dificultar su uso para personas que utilizan lectores de pantalla.
Agrega un atributo
aria-labelpara describir la función del botón:<div className="cursor-grab col-span-1 flex items-center justify-center"> - <Bars2Icon className="h-6 w-6 text-gray-500" /> + <Bars2Icon className="h-6 w-6 text-gray-500" aria-label="Reordenar categoría" /> </div>
95-95: Eliminar comentario innecesarioEl comentario
eslint-disable-next-line sonarjs/no-duplicate-stringpuede ser redundante si ya no es necesario desactivar esa regla.Si es posible, elimina el comentario para mantener el código limpio.
81-165: Dividir 'renderRow' en componentes más pequeñosEl método
renderRowes extenso y abarca múltiples responsabilidades, lo que dificulta su mantenimiento y legibilidad.Considera separar partes del renderizado en componentes individuales:
- Crear un componente para los campos de entrada.
- Crear un componente para los selectores de ordenamiento.
- Crear un componente para los botones de acción.
Esto mejorará la organización y facilitará futuras modificaciones.
frontend/src/routes/data/CustomData/VulnerabilityCategories.tsx (3)
88-90: Manejo de errores insuficiente al crear una categoríaAl atrapar un error al crear una categoría, solo muestras un mensaje genérico. Sería mejor proporcionar detalles adicionales o permitir que el usuario intente nuevamente.
Considera mostrar más información o recomendaciones al usuario.
95-105: Falta manejar el estado de carga al actualizar categoríasDurante la actualización de categorías en
onClickSave, no estás manejando un estado de carga. Esto puede llevar a problemas de interacción si el usuario intenta realizar otras acciones mientras la actualización está en curso.Añade un estado de carga para mejorar la usabilidad.
120-163: Mejora de accesibilidad y semántica en el formularioPara mejorar la accesibilidad, considera usar etiquetas
<label>asociadas con los campos de entrada y agrupar los elementos dentro de un<form>.Esto ayudará a usuarios con lectores de pantalla y mejorará la estructura del HTML.
frontend/src/routes/data/CustomData/VulnerabilityTypes.tsx (4)
69-73: Notifica al usuario en caso de error al cargar los idiomasActualmente, si ocurre un error al cargar los idiomas, solo se registra en la consola, pero el usuario no es notificado. Deberías mostrar un mensaje de error para informar al usuario.
Puedes agregar un mensaje de error así:
} catch (err) { console.error(err); + toast.error(t('err.failedLoadingLanguages')); }Asegúrate de tener una clave de traducción para
'err.failedLoadingLanguages'.
94-97: Notifica al usuario en caso de error al cargar los tipos de vulnerabilidadSi ocurre un error al obtener los tipos de vulnerabilidad, el usuario no es informado. Deberías mostrar un mensaje de error para mejorar la experiencia del usuario.
Agrega un mensaje de error como este:
} catch (err) { console.error(err); + toast.error(t('err.failedLoadingVulnerabilityTypes')); }Recuerda agregar la traducción correspondiente para
'err.failedLoadingVulnerabilityTypes'.
148-152: Evita la duplicación de código al filtrar los tipos de vulnerabilidadEl filtrado de
vulnerabilityTypesporlocalese repite en varias partes del código. Deberías extraer esta lógica en una función reutilizable para mejorar la mantenibilidad.Crea una función para filtrar:
const filterVulnerabilityTypesByLanguage = (languageValue: string) => { return vulnerabilityTypes.filter(type => type.locale === languageValue); };Luego, úsala así:
const onChangeLanguage = (value: ListItem) => { - setFilteredVulnerabilityTypes( - vulnerabilityTypes.filter(type => type.locale === value.value), - ); + setFilteredVulnerabilityTypes(filterVulnerabilityTypesByLanguage(value.value)); setCurrentLanguage(value); };Aplica lo mismo en otras partes donde filtras por idioma.
165-187: Mejora el manejo de errores al actualizar los tipos de vulnerabilidadActualmente, si ocurre un error durante la actualización, solo se muestra un mensaje genérico. Deberías manejar casos específicos, como conflictos o problemas de validación, para proporcionar información más detallada al usuario.
Considera verificar el código de estado de la respuesta de error:
} catch (error) { console.error(error); - toast.error(t('err.failedUpdatingVulnerabilityTypes')); + if (error.response && error.response.status === 400) { + toast.error(t('err.validationError')); + } else { + toast.error(t('err.failedUpdatingVulnerabilityTypes')); + } setIsEditing(false); return; }Asegúrate de agregar las claves de traducción necesarias.
frontend/src/services/data.ts (2)
791-793: Inconsistencia en el nombre de la funcióncreateVulnerabilityCategoriesLa función
createVulnerabilityCategoriessugiere que crea múltiples categorías, pero actualmente solo acepta un único objetoVulnerabilityCategory. Para mayor claridad y coherencia, sería recomendable renombrar la función acreateVulnerabilityCategory.Aplica este diff para renombrar la función:
-export const createVulnerabilityCategories = async ( +export const createVulnerabilityCategory = async ( vulnerabilityCategory: VulnerabilityCategory,Asegúrate de actualizar todas las llamadas a esta función en el código.
772-837: Unificar el manejo de errores para reducir código duplicadoNoté que el manejo de errores en las funciones relacionadas con vulnerabilidades es repetitivo. Sería beneficioso extraer este comportamiento en una función auxiliar para mejorar la mantenibilidad y reducir la duplicación de código.
Por ejemplo, podrías crear una función
handleResponseError:function handleResponseError(response: Response): never { throw new Error(response.statusText || networkErrorMsg); }Y utilizarla en tus funciones:
if (!response.ok) { - throw new Error(networkErrorMsg); + handleResponseError(response); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (10)
frontend/package.json(1 hunks)frontend/src/components/ui/switch.tsx(1 hunks)frontend/src/i18n/en-US/index.ts(3 hunks)frontend/src/routes/data/CustomData.tsx(2 hunks)frontend/src/routes/data/CustomData/VulnerabilityCategories.tsx(1 hunks)frontend/src/routes/data/CustomData/VulnerabilityCategoriesList.tsx(1 hunks)frontend/src/routes/data/CustomData/VulnerabilityTypeList.tsx(1 hunks)frontend/src/routes/data/CustomData/VulnerabilityTypes.tsx(1 hunks)frontend/src/routes/data/CustomData/custom-components/PrimarySwitch.tsx(1 hunks)frontend/src/services/data.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/package.json
🔇 Additional comments (6)
frontend/src/routes/data/CustomData/custom-components/PrimarySwitch.tsx (2)
1-1: ¡Buena elección de librería!
La utilización de @headlessui/react es una decisión acertada ya que proporciona componentes accesibles y bien mantenidos.
31-31: Exportación correcta
La exportación por defecto es apropiada para un componente único en el archivo.
frontend/src/routes/data/CustomData.tsx (2)
9-10: ¡Las importaciones están correctamente estructuradas!
La adición de los nuevos componentes sigue el patrón de importación establecido y mantiene una organización coherente con el resto del código.
Line range hint 16-40: ¡Atención! La secuencia de IDs no es consecutiva
La secuencia de IDs salta del 2 al 4, lo que podría causar confusión en el mantenimiento futuro del código. Es necesario mantener una secuencia numérica coherente.
Sugerencia de corrección:
{
- id: 4,
+ id: 3,
title: t('vulnerabilityTypes'),
cardTitle: t('createVulnerabilityTypes'),
cardChildren: <VulnerabilityTypes />,
},
{
- id: 5,
+ id: 4,
title: t('vulnerabilityCategories'),
cardTitle: t('createVulnerabilityCategories'),
cardChildren: <VulnerabilityCategories />,
},Verificar las nuevas claves de traducción
Es necesario verificar que las nuevas claves de traducción existan en todos los archivos de idiomas.
frontend/src/routes/data/CustomData/VulnerabilityCategories.tsx (1)
47-50: Valores iniciales de los estados podrían no ser los deseados
Estás inicializando sortOption y sortOrder con el primer elemento de sus respectivas listas. Verifica que estos valores coincidan con las preferencias predeterminadas o requerimientos del negocio.
Asegura que los valores iniciales sean los correctos para el caso de uso previsto.
frontend/src/services/data.ts (1)
795-795:
Corregir el tipo de dato devuelto en createVulnerabilityCategory
El tipo de dato devuelto en la promesa es incorrecto; debería ser VulnerabilityCategory en lugar de VulnerabilityType.
Aplica este diff para corregir el tipo de dato:
datas: VulnerabilityType;
+ datas: VulnerabilityCategory;Likely invalid or redundant comment.
caverav
left a comment
caverav
left a comment
There was a problem hiding this comment.
Se utilizan componentes propios en vez de utilizar los de shadcn, acordamos que preferiríamos estos (solo indiqué uno pero hay varios)
|
Creo que este PR si requiere nuevos tests (descripción del PR) |
- use shadcn switch component instead of primary switch - center cross button - change label position to be up - expand card size
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (2)
frontend/src/routes/data/CustomData/VulnerabilityCategoriesList.tsx (2)
30-45: Falta seguridad de tipos en las traduccionesLas claves de traducción se utilizan directamente como strings, lo que podría causar errores en tiempo de ejecución si las traducciones no existen.
Considera usar un tipo para las claves de traducción:
type TranslationKey = | 'cvssScore' | 'cvssTemporalScore' | 'cvssEnvironmentalScore' | 'priority' | 'remediationComplexity' | 'descending' | 'ascending'; const t = (key: TranslationKey): string => { // implementación existente };
79-79: El comentario debe estar en inglésEl comentario en español debe ser traducido al inglés para mantener la consistencia del código.
-//disabled true -> no se está editando +//disabled true -> not in edit mode
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
frontend/src/routes/data/CustomData/VulnerabilityCategories.tsx(1 hunks)frontend/src/routes/data/CustomData/VulnerabilityCategoriesList.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/routes/data/CustomData/VulnerabilityCategories.tsx
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
frontend/src/routes/data/CustomData/VulnerabilityCategoriesList.tsx(1 hunks)
🔇 Additional comments (5)
frontend/src/routes/data/CustomData/VulnerabilityCategoriesList.tsx (5)
57-62:
La comparación de datos es ineficiente y poco fiable
La comparación de referencias (!==) no detecta cambios en el contenido de los arrays, lo que puede causar que el componente no se actualice correctamente.
Implementa una comparación profunda:
+import { isEqual } from 'lodash';
useEffect(() => {
- if (prevDataRef.current !== data) {
+ if (!isEqual(prevDataRef.current, data)) {
setRows(data.map((row, index) => ({ ...row, id: index.toString() })));
prevDataRef.current = data;
}
}, [data]);Likely invalid or redundant comment.
52-54:
El uso del índice como clave es una mala práctica
Usar el índice como identificador en React puede causar problemas de renderizado y comportamiento inesperado, especialmente al reordenar o eliminar elementos.
Implementa un identificador único:
const [rows, setRows] = useState(
- data.map((row, index) => ({ ...row, id: index.toString() })),
+ data.map((row) => ({ ...row, id: crypto.randomUUID() })),
);Likely invalid or redundant comment.
176-182:
La comparación de datos es extremadamente ineficiente
Usar JSON.stringify para comparar objetos es una práctica muy ineficiente que puede causar problemas de rendimiento significativos.
Implementa una comparación más eficiente:
- useEffect(() => {
- const updatedData = rows.map(({ id, ...rest }) => rest);
- if (JSON.stringify(updatedData) !== JSON.stringify(data)) {
- onUpdateList(updatedData);
- }
- }, [rows, data, onUpdateList]);
+ useEffect(() => {
+ const updatedData = rows.map(({ id, ...rest }) => rest);
+ const hasChanges = updatedData.some((item, index) => (
+ !data[index] ||
+ item.name !== data[index].name ||
+ item.sortValue !== data[index].sortValue ||
+ item.sortOrder !== data[index].sortOrder ||
+ item.sortAuto !== data[index].sortAuto
+ ));
+ if (hasChanges) {
+ onUpdateList(updatedData);
+ }
+ }, [rows, data, onUpdateList]);Likely invalid or redundant comment.
64-72:
Falta validación de entrada en el manejador
No se está validando ningún valor antes de actualizar el estado, lo que podría permitir datos inválidos en la aplicación.
Implementa las validaciones necesarias:
const handleInputChange = (
id: string,
field: keyof VulnerabilityCategoryItem,
value: string | boolean,
) => {
+ if (field === 'name' && typeof value === 'string') {
+ if (value.length === 0 || value.length > 100) {
+ console.error('Nombre inválido');
+ return;
+ }
+ }
+ if (field === 'sortValue' && typeof value === 'string') {
+ if (!sortOptions.some(opt => opt.value === value)) {
+ console.error('Valor de ordenamiento inválido');
+ return;
+ }
+ }
setRows(prevRows =>
prevRows.map(row => (row.id === id ? { ...row, [field]: value } : row)),
);
};Likely invalid or redundant comment.
11-16:
Los tipos son demasiado permisivos y necesitan ser más estrictos
La definición actual permite valores que no deberían ser válidos. Es necesario reforzar los tipos para prevenir errores en tiempo de ejecución.
Implementa estos cambios:
+type SortValueType = 'cvssScore' | 'cvssTemporalScore' | 'cvssEnvironmentalScore' | 'priority' | 'remediationComplexity';
+type SortOrderType = 'asc' | 'desc';
type VulnerabilityCategoryItem = {
- name: string;
+ name: string & { minLength: 1, maxLength: 100 };
- sortValue: string;
+ sortValue: SortValueType;
- sortOrder: string;
+ sortOrder: SortOrderType;
sortAuto: boolean;
};Likely invalid or redundant comment.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
4 participants
Descripción
Se agregan la vistas "Vulnerability Types" y "Vulnerability Categories" a "Custom Data", junto a toda su funcionalidad.
Motivación y Contexto
Se realiza para completar las secciones faltantes en "Custom Data".
¿Cómo probar?
Vulnerability Types
EditCardtras una creación o edición de vulnerability types.EditCardvuelve al estado inicial al "cancelar" la edición.Vulnerability Categories
EditCardtras una creación o edición de vulnerability categories.EditCardvuelve al estado inicial al "cancelar" la edición.Tipos de cambios
Lista de verificación:
Summary by CodeRabbit
Nuevas Funciones
Switchpara una interfaz de usuario personalizable.Mejoras en la Localización
Actualizaciones de Dependencias
@radix-ui/react-switch.