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 - Implementa periodos censables en internacion #3003

Merged
merged 1 commit into from
Jun 26, 2024
Merged

IN - Implementa periodos censables en internacion #3003

merged 1 commit into from
Jun 26, 2024

Conversation

ma7payne
Copy link
Contributor

@ma7payne ma7payne commented Apr 24, 2024

Requerimiento

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

Funcionalidad desarrollada

  1. En el detalle de la internación (Ingreso) puedo verse una opción para agregar periodos censables, solo en capa estadística.
  2. El selector de periodos permite agregar uno o mas periodos a partir del ingreso por fechas.
  3. Los periodos se agregan al campo periodosCensables de la internación.
  4. Si se borra el listado de periodos el estado esCensable en la internación pasa a "false".
  5. Si la internación tiene estado esCensable "false" y se agregan periodos, pasa a ser "true".

UserStory llegó a completarse

  • Si
  • No
  • No corresponde

Requiere actualizaciones en la base de datos

  • Si
  • No

Requiere actualizaciones en la API

Requiere actualizaciones en andes-test-integracion

@ma7payne
Copy link
Contributor Author

@AgosLizzi dejo el PR para revisar la interfaz, si esta ok el lugar donde se coloca el selector o cualquier sugerencia de diseño y comportamiento que creas apropiada. Quedo atento.

Copy link
Contributor

@AgosLizzi AgosLizzi left a comment

Choose a reason for hiding this comment

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

Mati, un par de observaciones:

  1. O editamos o agregamos. SI tenemos un botón que dice "editar" no es necesario un tooltip, el botón ya indica.
    tooltip
  2. Siguiendo en la misma linea que el item anterior. Yo modificaria la acción del botón según lo que tenemos. Si no tenemos nada cargado el botón que da la acción es un "AGREGAR". Y si realmente queremos "EDITAR" lo pondría sobre el listado que se va generando
    boton editar
  3. Para el listado, agregaría un pleex table para diferenciar un periodo de otro. Decime que te parece
    table
  4. Esto resulta muy reiterativo.
    cobertura
  5. Por último, Cuando elimino un concepto se abre un modal de advertencia?

@ma7payne ma7payne force-pushed the IN-594 branch 3 times, most recently from a9fecb4 to 948e59b Compare May 3, 2024 15:36
@negro89 negro89 added the changes requested Se solicitaron cambios label May 6, 2024
@ma7payne ma7payne force-pushed the IN-594 branch 2 times, most recently from 44958f8 to 757c342 Compare May 13, 2024 18:45
@ma7payne ma7payne force-pushed the IN-594 branch 5 times, most recently from 6c83c83 to f5dc29a Compare May 15, 2024 15:36
@ma7payne
Copy link
Contributor Author

@AgosLizzi los cambios charlados fueron realizados.

@negro89
Copy link
Contributor

negro89 commented May 31, 2024

Mati, completando el circuito veo que lo datos se guardan correctamente y todo queda bien entendible desde el detalle en el sidebar. Pero noto algunas dificultades en el circuito de registro de los periodos que hacen a la funcionalidad un poco confusa:

  1. Si completo las fechas de un nuevo periodo que deseo agregar y en lugar de clickear en el boton "check" presiono "guardar" se lanza un cartel de confirmación pero el periodo no queda guardado.
  2. Similar al anterior. Si completo las fechas de un nuevo periodo y clickeo el "chheck" pero en lugar de guardar presiono la "X" no aparece el cartel pero tampoco se registra el periodo (Habiendo presionado previamente el boton "check").
  3. Cada vez que agrego un periodo, este se almacena debajo del todo y se resetean los campos de fecha quedando listos para registrar otro periodo. Esta ok, pero en mi pantalla no se percibe a primera vista que se haya registrado el nuevo periodo, por lo que sugeriría agregar un scrollToEnd cada vez que se registra un periodo.

Como propuesta (Revisar con agos en todo caso) suprimiría el botón "guardar" asi por un lado no mezclamos texto e iconos en los botones, y por el otro eliminamos la ambigüedad de la acción de guardar. Y dejaría el botón check con un tooltip "guardar periodo" encargado de almacenar cada uno. Quizas con la propuesta deja medio descolocado al botón "editar", para lo que se me ocurren dos cosas; que se muestre solo si hay al menos un periodo censable o que cada periodo tenga un boton editar (De esta forma hasta se podría controlar que no se pisen los periodos)

Dejo videito con las observaciones hechas: https://www.loom.com/share/ae2c4734eea14ac385af1a922a3e457b?sid=b2eb58e5-e244-428f-96c3-f806e0e56bd4


P/D: Tres casos recurrentes al dia de hoy de fallo de test (Dos corridas distintas)

  1. https://cloud.cypress.io/projects/xr7gft/runs/10068/test-results?actions=%5B%5D&browsers=%5B%5D&groups=%5B%5D&isFlaky=%5B%5D&modificationDateRange=%7B%22startDate%22%3A%221970-01-01%22%2C%22endDate%22%3A%222038-01-19%22%7D&orderBy=EXECUTION_ORDER&oses=%5B%5D&specs=%5B%5D&statuses=%5B%7B%22value%22%3A%22FAILED%22%2C%22label%22%3A%22FAILED%22%7D%5D&testingTypesEnum=%5B%5D
  2. https://cloud.cypress.io/projects/xr7gft/runs/10067/test-results?actions=%5B%5D&browsers=%5B%5D&groups=%5B%5D&isFlaky=%5B%5D&modificationDateRange=%7B%22startDate%22%3A%221970-01-01%22%2C%22endDate%22%3A%222038-01-19%22%7D&orderBy=EXECUTION_ORDER&oses=%5B%5D&specs=%5B%5D&statuses=%5B%7B%22value%22%3A%22FAILED%22%2C%22label%22%3A%22FAILED%22%7D%5D&testingTypesEnum=%5B%5D

image

@ma7payne ma7payne force-pushed the IN-594 branch 4 times, most recently from eb5b475 to a545afc Compare June 12, 2024 18:21
@ma7payne
Copy link
Contributor Author

ma7payne commented Jun 13, 2024

TEST OK #11090

@ma7payne ma7payne added changes done test ok Los test estan ok and removed changes requested Se solicitaron cambios labels Jun 13, 2024
@ma7payne
Copy link
Contributor Author

Mati, completando el circuito veo que lo datos se guardan correctamente y todo queda bien entendible desde el detalle en el sidebar. Pero noto algunas dificultades en el circuito de registro de los periodos que hacen a la funcionalidad un poco confusa:

  1. Si completo las fechas de un nuevo periodo que deseo agregar y en lugar de clickear en el boton "check" presiono "guardar" se lanza un cartel de confirmación pero el periodo no queda guardado.
  2. Similar al anterior. Si completo las fechas de un nuevo periodo y clickeo el "chheck" pero en lugar de guardar presiono la "X" no aparece el cartel pero tampoco se registra el periodo (Habiendo presionado previamente el boton "check").
  3. Cada vez que agrego un periodo, este se almacena debajo del todo y se resetean los campos de fecha quedando listos para registrar otro periodo. Esta ok, pero en mi pantalla no se percibe a primera vista que se haya registrado el nuevo periodo, por lo que sugeriría agregar un scrollToEnd cada vez que se registra un periodo.

Como propuesta (Revisar con agos en todo caso) suprimiría el botón "guardar" asi por un lado no mezclamos texto e iconos en los botones, y por el otro eliminamos la ambigüedad de la acción de guardar. Y dejaría el botón check con un tooltip "guardar periodo" encargado de almacenar cada uno. Quizas con la propuesta deja medio descolocado al botón "editar", para lo que se me ocurren dos cosas; que se muestre solo si hay al menos un periodo censable o que cada periodo tenga un boton editar (De esta forma hasta se podría controlar que no se pisen los periodos)

Dejo videito con las observaciones hechas: https://www.loom.com/share/ae2c4734eea14ac385af1a922a3e457b?sid=b2eb58e5-e244-428f-96c3-f806e0e56bd4

P/D: Tres casos recurrentes al dia de hoy de fallo de test (Dos corridas distintas)

  1. https://cloud.cypress.io/projects/xr7gft/runs/10068/test-results?actions=%5B%5D&browsers=%5B%5D&groups=%5B%5D&isFlaky=%5B%5D&modificationDateRange=%7B%22startDate%22%3A%221970-01-01%22%2C%22endDate%22%3A%222038-01-19%22%7D&orderBy=EXECUTION_ORDER&oses=%5B%5D&specs=%5B%5D&statuses=%5B%7B%22value%22%3A%22FAILED%22%2C%22label%22%3A%22FAILED%22%7D%5D&testingTypesEnum=%5B%5D
  2. https://cloud.cypress.io/projects/xr7gft/runs/10067/test-results?actions=%5B%5D&browsers=%5B%5D&groups=%5B%5D&isFlaky=%5B%5D&modificationDateRange=%7B%22startDate%22%3A%221970-01-01%22%2C%22endDate%22%3A%222038-01-19%22%7D&orderBy=EXECUTION_ORDER&oses=%5B%5D&specs=%5B%5D&statuses=%5B%7B%22value%22%3A%22FAILED%22%2C%22label%22%3A%22FAILED%22%7D%5D&testingTypesEnum=%5B%5D

image

@negro89 Listo los nuevos cambios para revisar. Se quita el botón de Guardar para aplicar cambios solo con el botón de check. No se aplica el tooltip en el check porque rompe la vista, se podría evaluar con @AgosLizzi una alternativa para este caso.

nuevoPeriodo: Periodo;
periodos: Periodo[] = [];
error = null;
camaEsCensable = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Esta variable no se utiliza en ningún lado. Si se dejó por un posible uso en el futuro, entonces se puede dejar!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MarianoCampetella Listo el cambio y las sugerencias sobre los subscribe deprecados!

@ma7payne ma7payne force-pushed the IN-594 branch 2 times, most recently from b4f4c8e to 4da9c70 Compare June 19, 2024 12:14
@negro89 negro89 merged commit a75abaf into master Jun 26, 2024
2 checks passed
@negro89 negro89 deleted the IN-594 branch June 26, 2024 15:02
liquid36 pushed a commit that referenced this pull request Jul 3, 2024
# [5.142.0](v5.141.0...v5.142.0) (2024-07-03)

### Features

* **IN:** implementa periodos censables en internacion ([#3003](#3003)) ([a75abaf](a75abaf))
* **TOP-183:** fix en ContinuarRegistro de TOP ([#3030](#3030)) ([087b003](087b003))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants