Vulnerability Types#175
Conversation
|
Caution Review failedThe pull request is closed. 📝 Walkthrough📝 WalkthroughWalkthroughEste pull request introduce cambios significativos en varios componentes de la aplicación, principalmente relacionados con la gestión de tipos de vulnerabilidades y la internacionalización. Se añaden nuevos mensajes de éxito y error en el archivo de localización para inglés, se mejora la lógica de actualización en el componente Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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/VulnerabilityTypes.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. 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 (
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (7)
frontend/src/routes/data/CustomData/VulnerabilityTypeList.tsx (1)
61-63: ¡Atención! Hay más texto que necesita traducciónHas traducido correctamente el placeholder, pero hay otros textos estáticos en el componente que también deberían traducirse, como el botón "X" para eliminar elementos.
Considera aplicar estas traducciones adicionales:
- <PrimaryButton color="red" onClick={() => handleRemoveRow(row.id)}> - X - </PrimaryButton> + <PrimaryButton color="red" onClick={() => handleRemoveRow(row.id)}> + {t('remove')} + </PrimaryButton>frontend/src/i18n/en-US/index.ts (1)
207-209: ¡Hay que mantener consistencia en los nombres de los mensajes de error!Los mensajes de error son claros, pero deberíamos mantener un patrón consistente en los nombres de las claves.
Sugiero este cambio para mantener la consistencia con otros mensajes de error:
- failedCreatingVulnerabilityType: 'Failed to create vulnerability type', - failedUpdatingVulnerabilityTypes: 'Failed to update vulnerability types', + errorCreatingVulnerabilityType: 'Failed to create vulnerability type', + errorUpdatingVulnerabilityTypes: 'Failed to update vulnerability types',frontend/src/routes/data/CustomData/VulnerabilityTypes.tsx (5)
70-71: Agregue retroalimentación al usuario en caso de error al cargar los idiomas.Actualmente, si ocurre un error al obtener los idiomas en
fetchLanguages, solo se registra en la consola conconsole.error(err). Debería notificar al usuario utilizandotoast.erroru otro mecanismo para informar que no se pudieron cargar los idiomas.
95-95: Agregue manejo de errores para informar al usuario si falla la carga de tipos de vulnerabilidad.En el bloque
catchdefetchVulnerabilityTypes, simplemente se registra el error en la consola. Sería recomendable informar al usuario sobre el problema.
160-164: EliminesetNewVulnerabilityTypeListde las dependencias deuseCallback.Las funciones setter de
useStateson estables y no cambian entre renders, por lo que no es necesario incluirlas en las dependencias deuseCallback.Aplicar este cambio para limpiar las dependencias:
const handleUpdateVulnerabilityType = useCallback( (data: { name: string; locale: string }[]) => { setNewVulnerabilityTypeList(data); }, - [setNewVulnerabilityTypeList], + [], );
204-204: Simplifique el manejadoronChangeenSelectDropdown.Puede pasar la función directamente sin necesidad de una función anónima intermedia.
Aplicar este cambio para simplificar el código:
<SelectDropdown items={languages} - onChange={value => onChangeLanguage(value)} + onChange={onChangeLanguage} selected={currentLanguage} title={t('language')} />
234-238: Considere extraer el componente de cargando a un componente reutilizable.El código para mostrar el estado de carga se repite. Sería beneficioso extraerlo en un componente separado para mejorar la reutilización y la legibilidad.
Por ejemplo:
// Crear un componente LoadingSpinner.jsx const LoadingSpinner = () => ( <div className="flex items-center justify-center h-min"> <span className="flex"> <ArrowPathIcon className="h-8 w-8 animate-spin text-blue-500" /> </span> </div> ); // Luego, utilizarlo en el código {loadingVulnerabilityTypes ? ( <LoadingSpinner /> ) : ( <VulnerabilityTypeList data={filteredVulnerabilityTypes} isDisabled={!isEditing} onUpdateList={handleUpdateVulnerabilityType} /> )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
frontend/src/i18n/en-US/index.ts(3 hunks)frontend/src/routes/data/CustomData/VulnerabilityTypeList.tsx(3 hunks)frontend/src/routes/data/CustomData/VulnerabilityTypes.tsx(5 hunks)frontend/src/services/data.ts(2 hunks)
🔇 Additional comments (5)
frontend/src/routes/data/CustomData/VulnerabilityTypeList.tsx (1)
2-2: ¡Bien hecho con la implementación de i18next!
La importación de la función de traducción está correctamente implementada y se alinea con los objetivos de internacionalización del proyecto.
frontend/src/services/data.ts (1)
749-749: Cambio de formato, sin impacto funcional
frontend/src/i18n/en-US/index.ts (2)
165-166: ¡Los mensajes de éxito están bien implementados!
Los mensajes son claros y siguen el patrón de nomenclatura existente para mensajes de éxito.
741-741: ¡La nueva clave de traducción está correctamente implementada!
La clave vulnerabilityType sigue el patrón de nomenclatura existente y su valor es apropiado.
frontend/src/routes/data/CustomData/VulnerabilityTypes.tsx (1)
168-175: Verifique la lógica al combinar los tipos de vulnerabilidad actualizados.
Asegúrese de que al combinar newVulnerabilityTypeList con differentLanguageTypes, no se pierdan datos y los tipos de vulnerabilidad se actualicen correctamente.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (2)
frontend/src/routes/data/CustomData/VulnerabilityTypes.tsx (2)
189-194: ¡Optimiza el rendimiento con useCallback!La función
onClickCanceldebería estar memoizada para evitar recreaciones innecesarias.Aplica este cambio:
- const onClickCancel = () => { + const onClickCancel = useCallback(() => { setFilteredVulnerabilityTypes( vulnerabilityTypes.filter(type => type.locale === currentLanguage?.value), ); setIsEditing(false); - }; + }, [vulnerabilityTypes, currentLanguage?.value]);
233-237: ¡Extrae el componente de carga!El componente de carga debería estar separado para mejorar la reutilización y mantenibilidad.
Crea un nuevo componente:
const LoadingSpinner = () => ( <div className="flex items-center justify-center h-min"> <span className="flex"> <ArrowPathIcon className="h-8 w-8 animate-spin text-blue-500" /> </span> </div> );Y úsalo así:
- <div className="flex items-center justify-center h-min"> - <span className="flex"> - <ArrowPathIcon className="h-8 w-8 animate-spin text-blue-500" /> - </span> - </div> + <LoadingSpinner />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
frontend/src/routes/data/CustomData/VulnerabilityTypeList.tsx(3 hunks)frontend/src/routes/data/CustomData/VulnerabilityTypes.tsx(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/routes/data/CustomData/VulnerabilityTypeList.tsx
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
frontend/src/routes/data/CustomData/VulnerabilityTypes.tsx (1)
89-91: Uniformiza el uso de tipos en el filtroEn el método
filter, estás tipandotypecomo{ locale: string }, pero deberías utilizar el tipoVulnerabilityTypepara mantener consistencia y aprovechar la definición del tipo.Aplica este cambio:
setFilteredVulnerabilityTypes( types.filter( - (type: { locale: string }) => + (type: VulnerabilityType) => type.locale === languageSelected?.value, ), );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
frontend/src/routes/data/CustomData/VulnerabilityTypes.tsx(4 hunks)
🔇 Additional comments (3)
frontend/src/routes/data/CustomData/VulnerabilityTypes.tsx (3)
134-143: Elimina la duplicación de logs y mejora el manejo de errores
Este comentario ya fue realizado en una revisión anterior y sigue siendo válido.
69-71: El manejo de errores es insuficiente al cargar idiomas
Este comentario ya fue realizado en una revisión anterior y sigue siendo válido.
98-99: Corrige las dependencias del useCallback
Este comentario ya fue realizado en una revisión anterior y sigue siendo válido.
Descripción
Inicialmente la branch estaba destinada para arreglar algunos bugs, pero durante el desarrollo se terminó agregando bastante de la funcionalidad faltante en Vulnerability Type en Custom Data.
Cambios realizados:
Motivación y Contexto
Se realiza para completar la funcionalidad requerida para la presentación del viernes (y apoyar al massi).
¿Cómo ha sido probado?
Para probar se debe:
Capturas de pantalla (si es apropiado):
No aplica
Tipos de cambios
Lista de verificación:
Summary by CodeRabbit
Nuevas características
Correcciones de errores
Documentación
VulnerabilityTypepara mejorar la seguridad de tipos en la gestión de vulnerabilidades.