Feedback en curso para las acciones de imagen hero

Tomé un ticket de baja prioridad “para después” sobre la tarjeta de asignación de imagen hero que no daba ningún feedback al hacer clic en cualquier cosa. Se había registrado después de notarlo durante pruebas manuales de un issue anterior, se hace clic en “Commit to live article” y no pasa nada en pantalla hasta que, eventualmente, esperando, la tarjeta se actualiza. Sin spinner, sin botón deshabilitado, nada que indique que el clic siquiera se registró.

Entré esperando un pequeño ajuste de CSS/JS. Se convirtió en un problema de diseño más interesante de lo que sugería el título del ticket.

Dos problemas distintos disfrazados de un solo ticket

Una vez que realmente leí el código, esto se dividió en dos arreglos sin relación. Search-load, Select y Remove son síncronos, un solo viaje de ida y vuelta HTTP, listo. Esos solo necesitaban el atributo integrado de Turbo data-turbo-submits-with, que deshabilita el botón e intercambia su etiqueta hasta que llega la respuesta. Nada de JS para escribir, solo un atributo.

Commit era el problema real. El controlador encola un job en segundo plano y retorna casi al instante, el job es el que hace el trabajo real (buscar el archivo en GitHub, quizás descargar y re-codificar una imagen, volver a escribirla). Un truco de “deshabilitar hasta que llegue la respuesta” no cubre esa ventana en absoluto, porque la respuesta vuelve antes de que ocurra cualquier trabajo real.

El diseño que casi publiqué, y por qué no lo hice

Mi primer plan era un booleano persistido hero_commit_in_progress: se pone en true cuando se encola el job, y el propio job lo limpia cuando termina (con éxito o con error). Directo, y habría funcionado para el camino feliz.

Pasé el plan por una segunda opinión antes de escribir código, y detectó algo que se me había escapado: si esa bandera alguna vez quedaba trabada en true, digamos, un deploy reinicia el worker en segundo plano a mitad de un job, la tarjeta mostraría un spinner permanente sin botón de reintentar y sin botón de quitar. Peor que el error que estaba arreglando, ya que al menos hoy siempre hay un botón accionable disponible.

Esta app despliega varias veces al día. “El worker se muere a mitad de un job” no es un caso extremo, es un martes cualquiera.

Así que eliminé por completo la columna de la base de datos. En su lugar, el controlador pasa un valor transitorio committing: true a la misma llamada de render que ya hace justo después de encolar el job, se muestra por exactamente una respuesta, y nunca se persiste en ningún lado. Recargar la página a mitad de un job simplemente vuelve a mostrar el botón normal; el propio broadcast del job igual corrige la tarjeta una vez que efectivamente termina. Nada que se pueda trabar, porque no hay estado en el que trabarse.

La contrapartida: una segunda pestaña del navegador, o una persona distinta, mirando el mismo post no va a ver el estado en curso, solo lo ve la pestaña que hizo el clic. Para una herramienta interna de un solo operador es un intercambio razonable. Para un producto multiusuario no lo sería.

El error que publiqué de todas formas (y que la revisión de código detectó)

Esta es una app de Rails con dos vistas independientes que muestran el estado de la imagen hero, una lista por lotes de “hero faltante” y la página de detalle del post, y con el tiempo habían terminado duplicando la misma lógica de commit/reintentar/quitar en vez de compartir un partial. Encontré esto mientras investigaba el ticket; el issue solo nombraba una de las dos vistas.

Escribí el mismo condicional de tres ramas en ambos archivos: “¿ya está en vivo?” / “¿se está haciendo commit?” / “¿hubo error?” / “mostrar el botón normal.” Puse la verificación de “en vivo” primero, siguiendo el código preexistente. Eso estaba mal. Si un post ya tiene un hero en vivo y se hace commit de una imagen nueva encima, la bandera de “en vivo” no se limpia hasta que el job termina, así que mi nueva rama de “committing” nunca ganaba la carrera del if/elsif. El caso de re-commit, que es posiblemente el uso real más común (reemplazar el hero, no solo asignarlo una vez), mostraba en silencio el texto obsoleto “Applied to the live article” sin spinner durante toda la corrida del job.

La revisión automatizada multiángulo que corro antes de cada merge, ocho pasadas independientes sobre el diff, cada una desde un ángulo distinto, señaló este mismo error de precedencia desde tres direcciones diferentes. Es el tipo de cosa obvia una vez que se ve, y muy fácil de no ver mientras se está escribiendo, porque el test del camino feliz (el primer commit de la historia, sin hero en vivo todavía) pasa limpio y parece terminado.

El arreglo fue reordenar dos líneas: verificar committing primero, luego live, luego error. Agregué un test de regresión específicamente para el caso de re-commit sobre un hero en vivo, ya que es exactamente el tipo de cosa que regresiona en silencio si alguien vuelve a reordenar las ramas más adelante.

Un segundo error, más escurridizo, de mi propio código de test

Mientras escribía un test para el nuevo atributo del botón Select, necesitaba hacer stub de una llamada externa de búsqueda de imágenes sin golpear APIs reales. Hay un patrón existente en este archivo de test para hacer stub de un método de clase sin ninguna gema de mocking instalada: definir un método singleton falso, y luego remove_method en un bloque ensure para limpiar.

Lo copié. Se veía idéntico. Rompió un archivo de test completamente distinto, y solo en CI, no localmente.

El patrón existente hace stub de Github::ContentClient.new, que se hereda de Class#new, quitar el stub simplemente vuelve a exponer el método heredado, inofensivo. Mi caso, ImageSearch.search, es un método que el propio módulo define. remove_method no expone nada debajo, elimina la única implementación que existió jamás, para el resto de ese proceso de test.

Localmente, con 10 workers de test en paralelo, mi test de stubbing y el test que llama al método real casi nunca caían en el mismo worker, así que se veía bien a través de varias corridas locales completas. CI corre con 2 workers. Probabilidad de colisión mucho más alta, y colisionó: un archivo de test completamente sin relación falló con NoMethodError: undefined method 'search' for module ImageSearch, varios archivos lejos de cualquier cosa que hubiera tocado. Me tomó un minuto ubicar por qué una suite que pasaba localmente fallaría en CI en un test que ni siquiera había modificado.

Arreglo: capturar el objeto Method real antes de hacer el stub, y restaurar ese objeto en vez de borrar nada. Lo anoté como nota de base de conocimiento ya que es una trampa general de Ruby, no específica de esta app, el error exacto que se obtiene al copiar y pegar un helper de stubbing sin verificar si el método original es heredado o definido explícitamente.

Qué sigue

Nada más planeado para este, se dimensionó como un ticket pequeño de pulido y se mantuvo en ese tamaño una vez que el diseño se asentó. Si un issue futuro quiere que el estado en curso sea visible entre pestañas o espectadores, eso implicaría retomar el enfoque de la bandera persistida con un camino de recuperación adecuado (por ejemplo, que el botón de quitar siga siempre disponible incluso a mitad de un commit) en vez del atajo transitorio-local que usó esto.