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

CIT - Notificaciones: limpiar código innecesario #3117

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

Conversation

JuanIRamirez
Copy link
Contributor

@JuanIRamirez JuanIRamirez commented Nov 27, 2024

Requerimiento

https://proyectos.andes.gob.ar/browse/CIT-355

Funcionalidad desarrollada

  1. Se elimina código obsoleto 'enviarSms' y sus llamadas.
  2. Se quita if (environment.production === true)
  3. Se modifica también suspender-turno.component para marcar el turno.avisoSuspension = 'enviado' o 'fallido' idem
    suspender-agenda.

UserStory llegó a completarse

  • Si
  • No
  • No corresponde

Requiere actualizaciones en la base de datos

  • Si
  • No

Requiere actualizaciones en la API

  • Si
  • No

Requiere actualizaciones en andes-test-integracion

  • Si
  • No

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.

codigo ok

idTurno: turno.id,
avisoSuspension: aviso
};
this.turnosService.patch(this.agenda.id, idBloque, turno.id, data).subscribe(resultado => {
Copy link
Contributor

Choose a reason for hiding this comment

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

@JuanIRamirez Veo en esta linea dos subscribe anidados, lo que resulta mala practica porque permite la fuga de memoria y dificultad para el manejo de errores. En su lugar se puede optar por pipe + switchMap o similar

@ma7payne ma7payne added the changes requested Se solicitaron cambios label Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changes requested Se solicitaron cambios
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants