Data UI required fixes#94
Conversation
…e into data-ui-required-fixes
iTzGooDLife
left a comment
iTzGooDLife
left a comment
There was a problem hiding this comment.
La regex implementada en la creación de colaboradores no es correcta, debería coincidir con la del backend:
/(?=.*\d)(?=.*[a-z])(?=.*[A-Z]).{8,}/
Se muestra un ejemplo en el screeenshot adjunto de como una contraseña que deberia ser valida, no lo es, mientras que al usar la regex mencionada pasa sin problemas (no pongo un screenshot de esto pq sería solamente el toast xd).
Regex actual:
|
No se verifican los campos requeridos durante la creación del cliente. En la primera imagen se ven los campos requeridos, pero al editar, estos ya no lo son (segunda imagen) y en la tercera imagen se puede ver que los cambios se guardan aunque no se tengan esos campos (excepto el email).
|
Está actualizado, hay que arreglar eso. Se hará issue.
|
El problema era que development estaba desactualizado en comparación con main, el problema ya no está presente. |
…e into data-ui-required-fixes
En efecto, únicamente gestioné los errores del back en el caso de editar, dado que el resto de acciones sí era posible, podemos discutir en la próxima daily el comportamiento deseado. |
|
Pedí un nuevo review @iTzGooDLife dado que se corrigió el Regex ! |
Pasa exactamente lo mismo con Collaborators. Primero cree uno, y luego lo edité limpiando todos los campos que antes eran required, y me dejó hacerlo. Adjunto fotos:
|
|
Encontré otro "bug" en Clients pero no sé que tanta importancia deberíamos darle ya que igual es un poco rebuscado y, además, está muy relacionado a los ya comentados más arriba. Pero básicamente se pueden pasar por alto los campos requeridos incluso a partir desde la creación del cliente como tal, es decir, no es necesario crear uno, luego editarlo y limpiar los campos como se indica en comentarios más arriba. Adjunto fotos de casos, donde la primera imagen muestra como, al rellenar los campos requeridos con un espacio (menos el correo), permite crear un cliente "vacío" pasando por alto los campos requeridos. Esto se puede ver reflejado en la segunda imagen.
Intenté replicar lo mismo en Collaborators pero, por alguna razón, ahí no lo permite y sale el siguiente mensaje en el toast:
|
sealra
dismissed
caverav’s stale review
Se agregó un <span/ > en vez de null dado que genera conflictos con los hooks de UITable useSortableTable y useTableFiltering, revisar si esto es un cambio adecuado o va fuera de este scope porfavor!
|
No se iba a eliminar alguno de los dos asteriscos que indican obligatoriedad, de una vez que hablamos de ello? Encuentro que es algo redundante que esté tanto en el título como en la caja para rellenar
|
|
Y para la creación de la contraseña en Collaborators, podría ser útil que exista una verificación de esta misma en el momento de la creación. Onda lo típico de Confirmar contraseña, por si existen errores de tipeo (q puede q pase) |
No es de este PR |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| const arrayBufferToBase64 = (buffer: ArrayBuffer): string => { | ||
| let binary = ''; | ||
| const bytes = new Uint8Array(buffer); | ||
| const len = bytes.byteLength; | ||
| for (let i = 0; i < len; i++) { | ||
| binary += String.fromCharCode(bytes[i]); | ||
| } | ||
| return btoa(binary); | ||
| }; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplificar la conversión a Base64 utilizando readAsDataURL
Actualmente, estás utilizando una función personalizada arrayBufferToBase64 y readAsArrayBuffer para convertir el archivo a una cadena Base64. Puedes simplificar este proceso utilizando el método readAsDataURL de FileReader, que convierte directamente el archivo en una cadena Base64 sin necesidad de pasos intermedios.
Aplica este cambio para simplificar el código:
-const arrayBufferToBase64 = (buffer: ArrayBuffer): string => {
- let binary = '';
- const bytes = new Uint8Array(buffer);
- const len = bytes.byteLength;
- for (let i = 0; i < len; i++) {
- binary += String.fromCharCode(bytes[i]);
- }
- return btoa(binary);
-};
...
reader.onloadend = () => {
- // eslint-disable-next-line @typescript-eslint/consistent-type-assertions
- const arrayBuffer = reader.result as ArrayBuffer;
- const base64String = arrayBufferToBase64(arrayBuffer);
+ const base64String = reader.result as string;
onFileSelect({ name: file.name, content: base64String });
};
-reader.readAsArrayBuffer(file);
+reader.readAsDataURL(file);Also applies to: 41-44
| }; | ||
| reader.readAsArrayBuffer(file); | ||
| } else { | ||
| setFileName(''); |
There was a problem hiding this comment.
Notificar la deselección de archivos al componente padre
Cuando el usuario deselecciona un archivo, la función onFileSelect no se llama. Esto puede provocar que el componente padre no sea consciente de que ya no hay un archivo seleccionado.
Aplica este cambio para notificar al componente padre cuando no hay un archivo seleccionado:
} else {
setFileName('');
+ onFileSelect({ name: '', content: '' });
}Committable suggestion was skipped due to low confidence.
|
@sealra por favor revisar los comentarios de coderabbit y resolver/aceptar c/u |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range comments (2)
frontend/src/i18n/en-US/index.ts (2)
Line range hint
1-141: Estandarice el uso de comillas en todo el archivoHe observado inconsistencias en el uso de comillas simples y dobles en todo el archivo. Esto es inaceptable y debe ser corregido de inmediato. Es crucial mantener un estilo coherente en todo el código.
Ejemplos de inconsistencias:
- Línea 5:
changeDisplayLanguageInfo: "Choose your preferred default language",- Línea 135:
clientEmailError: 'Client email already exists',Debe elegir un estilo (comillas simples o dobles) y aplicarlo consistentemente en todo el archivo. Recomiendo encarecidamente usar comillas simples para todas las cadenas, ya que es el estilo predominante en el archivo.
Aplique este cambio en todo el archivo:
- changeDisplayLanguageInfo: "Choose your preferred default language", + changeDisplayLanguageInfo: 'Choose your preferred default language',No hay razón para mantener esta inconsistencia. Corríjala inmediatamente para mejorar la calidad y mantenibilidad del código.
Line range hint
1-141: Reestructure el archivo de traducciones y revise el uso de HTML en las cadenasEste archivo es excesivamente largo y difícil de mantener. Además, el uso de etiquetas HTML dentro de las cadenas de traducción es una práctica peligrosa. Estas son las acciones que debe tomar de inmediato:
Divida este archivo en módulos más pequeños y manejables. Por ejemplo, agrupe las traducciones por secciones lógicas de la aplicación (autenticación, configuración, auditorías, etc.).
Elimine las etiquetas HTML de las cadenas de traducción. Estas deben manejarse en los componentes de la interfaz de usuario, no en los archivos de traducción. Por ejemplo:
- importVulnerabilitiesInfo: `Import Vulnerabilities from a .yml or .json file. (Serpico format accepted)<br /> - This allows you to easily import a batch of vulnerabilities templates.`, + importVulnerabilitiesInfo: 'Import Vulnerabilities from a .yml or .json file. (Serpico format accepted). This allows you to easily import a batch of vulnerabilities templates.',
- Considere utilizar una biblioteca de internacionalización más robusta que permita el manejo de plurales y la interpolación de variables de manera más segura.
Estas modificaciones son cruciales para mejorar la mantenibilidad y la seguridad de su código. No hay excusa para mantener estas prácticas deficientes. Implemente estos cambios lo antes posible.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (3)
- frontend/src/components/input/FileInput.tsx (2 hunks)
- frontend/src/i18n/en-US/index.ts (1 hunks)
- frontend/src/routes/data/Collaborators.tsx (15 hunks)
iTzGooDLife
left a comment
iTzGooDLife
left a comment
There was a problem hiding this comment.
Al agregar y editar colaboradores, si la solicitud falla por conexión o por no rellenar algún campo requerido, si se cierra y abre el modal, la alerta de Confirmar contraseña queda activada hasta que se recargue la página (aunque se envíe una solicitud correctamente).
Editar colaborador
Añadir colaborador
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (1)
- frontend/src/routes/data/Collaborators.tsx (15 hunks)
🔇 Additional comments (2)
frontend/src/routes/data/Collaborators.tsx (2)
465-468: Verificar la visualización de los campos requeridos en el formularioEn los componentes de entrada, estás utilizando las propiedades
requiredFieldyrequiredAlert. Asegúrate de que el indicador de campo requerido y las alertas se muestren correctamente al usuario para mejorar la experiencia de uso.Comprueba visualmente que los asteriscos de campo requerido y los mensajes de alerta aparecen cuando corresponden. Si hay duplicación de asteriscos o mensajes confusos, considera ajustarlos para mayor claridad.
Also applies to: 476-479, 487-490, 513-514, 524-528, 530-538
240-242: Unificar la validación de contraseñas con el backendLa función
validatePasswordutiliza una expresión regular para verificar la complejidad de la contraseña. Es importante asegurarse de que esta validación coincida exactamente con la que utiliza el backend para evitar inconsistencias.Por favor, confirma que la expresión regular
/^(?=.*\d)(?=.*[a-z])(?=.*[A-Z]).{8,}$/coincide con la del backend. Puedes ejecutar el siguiente script para verificarlo:
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (1)
- frontend/src/routes/data/Collaborators.tsx (15 hunks)
🔇 Additional comments (6)
frontend/src/routes/data/Collaborators.tsx (6)
240-242: Excelente implementación de la función de validación de contraseñasLa función
validatePasswordutiliza una expresión regular adecuada para asegurar que las contraseñas cumplan con los requisitos de complejidad.
246-307: Validaciones de campos requeridos implementadas correctamenteLas validaciones para los campos obligatorios en
handleSubmitAddCollabestán bien implementadas, garantizando que los datos esenciales sean proporcionados antes de crear un colaborador.
334-373: Validaciones adecuadas al editar un colaboradorEn la función
handleSubmitEditCollab, las validaciones se realizan correctamente, permitiendo la edición segura de colaboradores y garantizando la integridad de los datos.
104-133: Uso efectivo de 'useMemo' para optimizar el renderizadoLa utilización de
useMemopara definir las columnas de la tabla es apropiada y ayuda a optimizar el rendimiento del componente.
390-399: 🛠️ Refactor suggestionSimplificación posible en las funciones de manejo de estado
Dado que
newCollaboratorse inicializa y nunca esnull, las comprobaciones deprevStatepodrían simplificarse eliminando la verificación denull. Esto hará el código más limpio y fácil de mantener.Aplicar este cambio en
handleInputChange:const handleInputChange = (name: string, value: string) => { - setNewCollaborator(prevState => { - if (!prevState) { - return null; - } else { - return { - ...prevState, - [name]: value, - }; - } - }); + setNewCollaborator(prevState => ({ + ...prevState, + [name]: value, + })); };Y en
handleRoleChange:const handleRoleChange = (role: RoleOption) => { setSelectedRole(role); - setNewCollaborator(prevState => { - if (!prevState) { - return null; - } else { - return { - ...prevState, - role: role.value, - }; - } - }); + setNewCollaborator(prevState => ({ + ...prevState, + role: role.value, + })); };Likely invalid or redundant comment.
241-241: Mantener la consistencia de validación de contraseñas con el backendAsegúrate de que la expresión regular utilizada en
validatePasswordcoincida con la que se utiliza en el backend para mantener la consistencia en la validación de contraseñas.Verifica si la expresión regular coincide con la del backend:
6 participants
Descripción
Se agregan required en CRUD de Data, y se modifica el componente FileInput para que también sea capaz de gestionar required. Además, se arregla comportamiento buggy en Clients.
Destacar que no soluciona problemas de ESLint warns del issue #52
Motivación y Contexto
Mejora UI/UX, soluciona issue #63
¿Cómo ha sido probado?
A través del uso de los modales, no afecta otras áreas.
Capturas de pantalla (si es apropiado):
Tipos de cambios
Lista de verificación:
Summary by CodeRabbit
Nuevas Funciones
Mejoras en la Experiencia del Usuario