Deuda técnica, parte 2: "mover no era mejorar", así que esta vez lo desmonté de verdad
ContenidoContents
En la primera parte os dejé en el peor momento posible: con una lección a medio cumplir. La moraleja de aquel post era “mover no es mejorar”, y la ilustré con mi peor pieza de código —una función de casi 924 líneas que genera la videollamada— a la que hice justo eso: moverla de sitio sin tocarla por dentro. Seguía ahí, intacta, escupiendo HTML y unas 600 líneas de JavaScript concatenado a mano. Prometido quedaba arreglarla “más adelante”.
Pues bienvenidos a “más adelante”. Esta es la tanda donde el monstruo se desmonta de verdad. Sigue todo en la rama dev, en pruebas, pero ya os puedo contar cómo ha ido. Y, como siempre, los sustos que han salido por el camino.
El plan: partir el monstruo en tres
Una función que mezcla qué datos mostrar, cómo se ven y qué hacen es imposible de mantener (y de testear). Así que la idea era separarla en tres capas limpias:
- Datos: funciones PHP puras y testeables que construyen lo que hay que mostrar.
- Presentación: el HTML, fuera del PHP, en plantillas (Mustache, el sistema de Moodle).
- Comportamiento: el JavaScript, fuera del PHP, en módulos modernos (AMD, ES6, con su linter).
Y de paso, rematar lib.php, que en el post 1 dejé a medio vaciar.
lib.php: de 1.086 a 363 líneas
Empecé por el remate fácil. Los últimos helpers que quedaban en lib.php salieron a sus clases, los renderizadores de gráficos (un mapa de calor, una barra de segmentos) pasaron a plantillas Mustache, y la versión “privada” de la función de la videollamada —que en el post 1 ni siquiera había movido— por fin se fue a su sitio.
Resultado: lib.php pasó de 1.086 a 363 líneas. Y lo bonito es qué son esas 363: ya solo callbacks estándar de Moodle. Nada de lógica de negocio, nada de HTML, nada de JavaScript. El cajón de sastre dejó de ser un cajón de sastre.
Desmontando la función: de generar comportamiento a generar datos
El cambio de fondo se ve en un botón cualquiera —el de retransmitir en directo—. Antes, su lógica se “imprimía” como texto desde PHP, escapando comillas y cosiendo valores a mano:
1// ANTES: JavaScript construido a base de echo dentro del PHP
2echo "function handleStreamBtn() {\n";
3echo " btn.disabled = true;\n";
4echo " require(['jquery','core/ajax'], function($, ajax) {\n"; // ← la mecha del 'require'
5echo " ajax.call([{ methodname: 'mod_jitsi_press_record_button',\n";
6echo " args: {jitsi:'" . $jitsi->id . "', user:'" . $USER->id . "'} }]);\n"; // ← datos PHP cosidos
7echo " });\n";
Comillas escapadas, el texto traducible metido con addslashes(get_string(...)), IDs de PHP interpolados en mitad del JavaScript, una variable muerta que nadie veía, y un require inline (la mecha del ReferenceError de la que hablo luego). Imposible de testear, imposible de pasar por un linter.
Después, ese botón vive en un módulo ES6 de verdad, y el PHP solo le pasa datos:
1// DESPUÉS: amd/src/session_recording.js (ES6, con linter e imports reales)
2const handleStreamBtn = () => {
3 const btn = document.getElementById('streamBtn');
4 if (!btn) { return; }
5 btn.disabled = true;
6 fire('mod_jitsi_press_record_button', {jitsi: config.jitsiid, user: config.userid});
7 btn.classList.contains('btn-warning') ? stopStream() : stream();
8};
9streamBtn.addEventListener('click', handleStreamBtn); // antes: onclick="..." en el HTML
Y el lado PHP, que eran 600 líneas de echo, queda en una llamada que entrega datos, no comportamiento:
1$PAGE->requires->js_call_amd('mod_jitsi/session_recording', 'init', [[
2 'jitsiid' => (int) $jitsi->id, 'userid' => (int) $USER->id, 'cmid' => (int) $cmid,
3]]);
Y esto no es estética: el PHP deja de fabricar comportamiento y pasa a entregar datos que el módulo consume. El onclick desaparece (lo sustituye addEventListener), las traducciones salen por el sistema de idiomas en vez de incrustarse, y el JavaScript queda bajo el mismo linter que cualquier módulo de Moodle. Que es, exactamente, el contrato que la interfaz reactiva de Moodle (la que va hacia React) espera: datos por servicio, comportamiento en módulos.
La batallita: “require no existe todavía”
Aquí está el susto más instructivo de esta tanda, y tiene una moraleja que no me esperaba.
Cuando saqué el primer módulo de JavaScript, lo cargué de la forma “obvia”: un require([...]) al principio del script. Petó en producción con un críptico ReferenceError: Can't find variable: require. ¿Cómo? Si ese JavaScript llevaba años funcionando…
La clave: el código original nunca llamaba a require directamente. Lo hacía siempre dentro de temporizadores y manejadores de eventos, que se ejecutan más tarde —cuando el cargador de módulos de Moodle ya está listo—. Mi require nuevo, en cambio, corría al instante, antes de que ese cargador existiera. La solución idiomática: exponer el objeto vivo en window y dejar que Moodle cargue el módulo en el pie de página, ya con todo listo.
La lección, que da título a una de las de hoy: sacar JavaScript de PHP a un módulo no es mover código, es cambiar el momento en que se ejecuta. El código inline corre al instante; un módulo corre cuando el sistema lo resuelve. Ese desfase de tiempo es justo donde te estrellas. Y de propina, un segundo bug encadenado: como la redirección de “colgar la llamada” vivía ahora en ese módulo que no cargaba, al colgar te quedabas atrapado en la pantalla de la videollamada. Si mueves la salida a un módulo y el módulo no carga, te quedas sin salida.
Lo que un refactor destapa
Reescribir a mano, línea a línea, es la mejor herramienta de auditoría que conozco. Traduciendo el JavaScript de grabación encontré ~80 líneas que no se ejecutaban nunca: una función sin una sola llamada y bloques que comprobaban un botón que ya no existe en ninguna plantilla. Solo lo ves si te obligas a leer cada línea.
Y dos trampas que evité por los pelos:
- Código casi idéntico que NO debía unificar. El token de seguridad de las salas privadas parece calcado al de las normales… salvo que fuerza la grabación a desactivada. “Deduplicarlo porque es igual” habría sido un agujero de seguridad: habilitar grabación en sesiones privadas. La duplicación con intención no se toca; se comenta.
- Otro cabo suelto del post 1. Validando, al abrir la página de admin para borrar grabaciones, saltó un
Call to undefined function isDeletable(). Era un call site huérfano del refactor anterior, escondido en una página poco visitada. El primo delgrepengañoso de la primera parte: los rincones que ni tus tests ni tú visitáis son donde se esconden estas minas.
Las lecciones de esta tanda
- Sacar JS inline a un módulo no es mover, es cambiar el modelo de ejecución. Copiar y pegar no basta: hay que exponer el estado vivo y dejar que el framework cargue el módulo en su momento.
- Cuando tu CI no puede ejecutar la función, el test runner eres tú. Los tests automáticos no entran en una videollamada. Así que troceé el trabajo en pasos minúsculos (presencia, botones, contraseña, grabación) y validé cada uno en una llamada real —entrar, grabar, colgar, ver el vídeo— antes de hacer commit. El tamaño del paso no lo decide lo que cabe en un commit, sino lo que puedes verificar de una vez.
- No todo lo que se repite se deduplica. El código casi-idéntico que diverge a propósito es una trampa disfrazada de oportunidad.
Dónde estoy y qué queda
Con esto, el refactor del trío external.php / lib.php / la función de la videollamada queda cerrado (todo en dev, validado en llamadas reales, aún sin llevar a master). En métricas de esta segunda tanda: 14 commits, lib.php a 363 líneas, de 105 a 130 tests, y unas 570 líneas de JavaScript que han salido de dentro del PHP a módulos con linter.
¿El siguiente objetivo? Ya lo tengo fichado: view.php, otras ~970 líneas con el mismo vicio (JavaScript incrustado, SQL construido a mano, todo mezclado). Misma medicina, otra tanda.
Y cerrando el círculo del post 1: todo esto —separar datos de presentación y de comportamiento, que el JavaScript consuma servicios en vez de venir cosido desde PHP— es exactamente el cimiento que la nueva interfaz de Moodle, la que va hacia React, necesita. Así que sí: mover no era mejorar. Por eso esta vez, en lugar de mover, he desmontado.
Continúa en la tercera parte: refactorizo la vista principal y, debajo, aparece una consulta escaneando toda la tabla de logs.