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

IN - Eliminar ingresos duplicados #1991

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

IN - Eliminar ingresos duplicados #1991

wants to merge 3 commits into from

Conversation

Fabio-Ramirez
Copy link
Contributor

@Fabio-Ramirez Fabio-Ramirez commented Nov 8, 2024

Requerimiento

https://proyectos.andes.gob.ar/browse/IN-598

Funcionalidad desarrollada

  1. El script elimina las internaciones con egresos duplicados que quedaron en base.
  2. En caso de egreso duplicado, se elige el egreso con la fecha mas reciente.
  3. NOTA colocar como parametros fecha requerida, por ejemplo: node jobs/manual.js scripts/IN-598.js 2021-11-31 2021-12-26

UserStories llegó a completarse

  • Si
  • No

Requiere actualizaciones en la base de datos

  • Si
  • No

@Fabio-Ramirez Fabio-Ramirez requested review from a team as code owners November 8, 2024 17:18
@github-actions github-actions bot added the script Necesidad de aplicar un scripts label Nov 8, 2024
@MarianoCampetella
Copy link
Contributor

MarianoCampetella commented Nov 13, 2024

A modo de comentario, en la BD tengo cargado un egreso duplicado con fecha del 06/11/2024 pero al momento de ejecutar dentro de mi terminar node jobs/manual.js scripts/IN-598.js 2024-11-01 2024-11-31 no me lo modifica. Si a nadie le sucede lo mismo entonces el PR se encontraría aprobado.

@MarianoCampetella MarianoCampetella added the changes requested Se solicitaron cambios label Nov 13, 2024
@Fabio-Ramirez Fabio-Ramirez added changes done and removed changes requested Se solicitaron cambios changes done labels Nov 15, 2024
@Fabio-Ramirez Fabio-Ramirez force-pushed the IN-598 branch 2 times, most recently from 602a515 to 4965c6a Compare November 19, 2024 11:16
@MCele MCele requested review from MarianoCampetella and removed request for AgosLizzi November 19, 2024 15:33
Copy link
Contributor Author

@Fabio-Ramirez Fabio-Ramirez left a comment

Choose a reason for hiding this comment

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

Compa, utilizando la BD de Demo, no encuentro la internación en la fecha que mencionas, en todo caso si me podés compartir el registro que utilizaste, quedo atento.

@Fabio-Ramirez
Copy link
Contributor Author

Fabio-Ramirez commented Nov 19, 2024

A modo de comentario, en la BD tengo cargado un egreso duplicado con fecha del 06/11/2024 pero al momento de ejecutar dentro de mi terminar node jobs/manual.js scripts/IN-598.js 2024-11-01 2024-11-31 no me lo modifica. Si a nadie le sucede lo mismo entonces el PR se encontraría aprobado.
Compa, utilizando la BD de Demo, no encuentro la internación en la fecha que mencionas, en todo caso si me podés compartir el registro que utilizaste, quedo atento.

@MarianoCampetella
Copy link
Contributor

egresoDuplicado.txt
Fabi ahi te dejo un ejemplo donde pongo el comando que dejaste en la descripcion: node jobs/manual.js scripts/IN-598.js 2024-11-05 2024-11-09 y no me esta borrando el egreso duplicado. Cualquier cosa escribime en privado o si alguien mas lo prueba y le funciona entonces estaría bien!

@Fabio-Ramirez
Copy link
Contributor Author

egresoDuplicado.txt Fabi ahi te dejo un ejemplo donde pongo el comando que dejaste en la descripcion: node jobs/manual.js scripts/IN-598.js 2024-11-05 2024-11-09 y no me esta borrando el egreso duplicado. Cualquier cosa escribime en privado o si alguien mas lo prueba y le funciona entonces estaría bien!

Compa, gracias por la observación, tenias toda la razón, en este ejemplo en particular q enviaste, sucedía que no estaba contamplando el caso de egresos duplicados que además de la misma fecha, tengan el mismo horario. Además, me sirvio para agregar una modificación donde el script abarque todo el dia que ingresa en el parametro de "fecha fin".

@Fabio-Ramirez Fabio-Ramirez added changes done and removed En revisión Se está probando labels Dec 6, 2024
Copy link
Contributor

@negro89 negro89 left a comment

Choose a reason for hiding this comment

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

Fabi, estan bien detectados los movimientos duplicados pero dejo un par de sugerencias en el código. Además recordar que el movimiento que debería prevalecer es el que tiene misma fecha que la prestación. Con respecto a esto ultimo..

  1. La prestación debería existir, de lo contrario hay un error.
  2. Si ninguno de los dos movimientos coincide con la prestación, modificar alguno para que coincida.

❗IMPORTANTE: Al recorrer los estados/movimientos buscando duplicados, tener en cuenta que pueden existir anulados (deletedAt: Date) que deben ser omitidos.

moment(new Date(paramInicio).setHours(0, 0, 0, 0)).format('YYYY-MM-DD HH:mm:ss') :
moment(new Date().setHours(0, 0, 0, 0)).subtract(1, 'day').format('YYYY-MM-DD HH:mm:ss');
const end = moment(paramFin, 'YYYY-MM-DD', true).isValid()
? moment(paramFin).endOf('day').format('YYYY-MM-DD HH:mm:ss')
Copy link
Contributor

Choose a reason for hiding this comment

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

Tanto para start como para end la inicialización la dejaría como moment(paramInicio).startOf('day').toDate() y moment(paramFin).endOf('day').toDate() respectivamente. Y para validar los parametros, previo a la instanciación de las fechas sugiero..

if(!moment(paramInicio, 'YYYY-MM-DD', true).isValid() || !moment(paramFin, 'YYYY-MM-DD', true).isValid()) {
     done();
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agregado

if (fecha && idInternacion) {
const existeEstadoIndex = estadoSinDuplicar.findIndex(e => {
const fechaConvertida = moment(e.fecha).format('YYYY-MM-DD');
return (fechaConvertida === fecha && e.extras?.idInternacion?.toString() === idInternacion.toString()
Copy link
Contributor

@negro89 negro89 Dec 19, 2024

Choose a reason for hiding this comment

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

Con respecto a la comparación entre fechas, sugiero usar las funciones de moment para hacer el codigo mas conciso:
const fecha = moment(estado.fecha);
.
.
.
const fechaConvertida = moment(e.fecha);
return (fechaConvertida.isSame(fecha) && ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agregado.

@negro89 negro89 added changes requested Se solicitaron cambios and removed changes done labels Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changes done script Necesidad de aplicar un scripts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants