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

FEAT-Modificar esquema especialidad profesionales #1801

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JuanIRamirez
Copy link
Contributor

@JuanIRamirez JuanIRamirez commented Apr 24, 2023

Requerimiento

https://proyectos.andes.gob.ar/browse/MAT-119

Funcionalidad desarrollada

  1. Script para corregir la colección "profesional" de acuerdo al nuevo esquema.
  2. Se modificó esquema "profesional"

UserStories llegó a completarse

  • Si
  • No

Requiere actualizaciones en la base de datos

  • Si
  • No

@JuanIRamirez JuanIRamirez requested review from a team as code owners April 24, 2023 16:09
@github-actions github-actions bot added the script Necesidad de aplicar un scripts label Apr 24, 2023
@silviroa
Copy link
Contributor

USUARIO: jramirez
BUILD NUMBER: 8761
CYPRESS RUN: 7934
TEST START: 2023-04-26T13:27:24.145Z
TOTAL: 406
SUCCESS: 403
FAIL: 1
SKIPPED: 2

@silviroa
Copy link
Contributor

USUARIO: jramirez
BUILD NUMBER: 8762
CYPRESS RUN: 7935
TEST START: 2023-04-26T13:58:29.544Z
TOTAL: 406
SUCCESS: 402
FAIL: 2
SKIPPED: 2

@silviroa
Copy link
Contributor

USUARIO: jramirez
BUILD NUMBER: 8763
CYPRESS RUN: 7936
TEST START: 2023-04-26T14:09:26.053Z
TOTAL: 406
SUCCESS: 402
FAIL: 2
SKIPPED: 2

@silviroa
Copy link
Contributor

silviroa commented May 5, 2023

USUARIO: mcampetella
BUILD NUMBER: 8845
CYPRESS RUN: 8013
TEST START: 2023-05-05T15:35:53.352Z
TOTAL: 406
SUCCESS: 401
FAIL: 3
SKIPPED: 2


const actualizar = true;
const diasAño = 365.25;
const añosRevalida = 5 + 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

No seria lo mismo poner const añosRevalida = 6 y aclarar en un comentario que son 5 años de vigencia mas 1 de gracia??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Si, serìa lo mismo, pero como son Cinco años lo correcto y pueden tener un año de gracia. Es para indicar eso.

Copy link
Contributor

Choose a reason for hiding this comment

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

En todo caso eso se puede especificar mediante un comentario con los // o /* ... */
Por ejemplo: // Los años de vigencia son 5 años mas 1 año de gracia los cual sumados dan 6


for (const matriculacion of profMatriculacion) {

if (matriculacion.periodos.length === 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

esto se puede remplazar por if(!matriculacion.periodos.length) { ... }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hecho

Comment on lines 64 to 68
function replSpecialChr(texto: string) {
let textoReplace = texto.replace(/\(/g, '.');
textoReplace = textoReplace.replace(/\)/g, '.');
return textoReplace;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

No entiendo por qué haces dos veces el .replace a la misma cadena de caracteres.
Esto se podría optimizar de la siguiente manera:

function replSpecialChr(texto: string) { return texto.replace(/(/g, '.'); }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No es lo mismo, primero reemplaza '(' y luego ')' por '.' los dos.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

igual esa funciòn se descartò

@MarianoCampetella
Copy link
Contributor

Como se modifico el esquema del profesional, seguramente genere problemas los cuales el siguiente test no este pasando.
image

@github-actions github-actions bot added the has_conflicts Tiene conlfictos label May 5, 2023
@JuanIRamirez JuanIRamirez force-pushed the MAT-119 branch 2 times, most recently from 1f8dd5d to b66a2ed Compare May 22, 2023 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changes requested Se solicitaron cambios script Necesidad de aplicar un scripts test fail
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants