diff --git a/AGENTS.md b/AGENTS.md index 3ff7ad2..e6d6e43 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -182,7 +182,7 @@ Cerradas: **TAREA-012 / RIE-001** investigación (DEC-0019); **TAREA-009 / RIE-0 | DEC-0048 | **Aceptada** (2026-06-12) | Estrategia de cobertura de tests: mockear la red con `\curl::mock_response()` + mock parcial de `download_to_temp()` en vez de excluir; no excluir del scope código testeable (`excludelistfiles` vacío); xdebug/Codecov es la medida autoritativa (pcov local subacredita llamadas anidadas — artefacto, no límite); gate `codecov project: target: auto` (trinquete). Cobertura honesta 85.71%→87.2% (PR #65) | | DEC-0049 | **Aceptada** (2026-06-12) | Auditoría estándar de repositorio (2026-06-11, tras DEC-0016/DEC-0044): 9 mejoras P1–P3 implementadas (PRs #46–#54: hardening XML de estilos, thirdpartylibs en el ZIP, fidelidad backup/restore, lock de intentos, participación vs grademethod, recálculo de notas en lote, `zip_utils`, descarga del informe, Behat) + registro de **hallazgos descartados** y opciones de dirección para no re-auditar | | DEC-0050 | **Aceptada** (2026-06-12) | La herramienta de migración exeweb/exescorm vive en `mod_exelearning` (destino, dueño de los internals); orígenes como fuentes legacy de solo lectura tras `source_interface`. Endurecimiento de la rama issue #13: fix `mod_exeweb` itemid=revision (antes leía 0 → todo `nosource`); clasificación `mod_exescorm` (`.elpx` directo / 1 embebido / 0=nosource / >1=ambiguous / external+aiccurl+localsync=unsupported, `localsync` excluido por sincronización aunque tenga snapshot local); limpieza compensatoria con `course_delete_module` ante fallo parcial (sin transacción, caveat recycle bin); preservación de metadatos del cm (idnumber **nunca** se copia); validación post-extracción anti shell-vacío (`migrateextractfailed`); eventos (started/migrated/skipped/failed, patrón DEC-0041); columnas `userid`/`timemodified` (upgrade 2026061201); preflight + `\core\progress\display`. Refactor a `classes/local/migration/` (elimina `import_service`). CLI diferido | -| DEC-0051..0063 | (varias, 2026-06-12 → 2026-06-17) | **Ver índice completo en `research/docs/indices/adrs.yaml`.** Resumen: DEC-0051 eventos selectivos · DEC-0052 completion por estado · DEC-0053 búsqueda global · DEC-0054 refactor `lib.php` (extracción a clases) · DEC-0055 auditoría post-refactor · DEC-0056 tests JS (Vitest) · DEC-0057 extracción no-destructiva (BETA→STABLE) · DEC-0058 fijar tag del editor en release · DEC-0063 validación canónica del endpoint xAPI + política de versión (1.0.3 tolerante a 2.0) · DEC-0064 implementación ingesta xAPI · DEC-0065 editor solo empaquetado en release (sin instalador runtime) · DEC-0066 interruptor global del editor (modo reproductor puro). *(DEC-0059..0062 = iframe seguro en rama `feature/secure-iframe-scorm-bridge`, aún no en `main`.)* | +| DEC-0051..0063 | (varias, 2026-06-12 → 2026-06-17) | **Ver índice completo en `research/docs/indices/adrs.yaml`.** Resumen: DEC-0051 eventos selectivos · DEC-0052 completion por estado · DEC-0053 búsqueda global · DEC-0054 refactor `lib.php` (extracción a clases) · DEC-0055 auditoría post-refactor · DEC-0056 tests JS (Vitest) · DEC-0057 extracción no-destructiva (BETA→STABLE) · DEC-0058 fijar tag del editor en release · DEC-0063 validación canónica del endpoint xAPI + política de versión (1.0.3 tolerante a 2.0) · DEC-0064 implementación ingesta xAPI · DEC-0065 editor solo empaquetado en release (sin instalador runtime) · DEC-0066 interruptor global del editor (modo reproductor puro) · DEC-0067 página de estilos solo-endpoint (cierra UX-01). *(DEC-0059..0062 = iframe seguro en rama `feature/secure-iframe-scorm-bridge`, aún no en `main`.)* | ## Restricciones inmutables diff --git a/README.md b/README.md index 0b53b3b..34de1fd 100644 --- a/README.md +++ b/README.md @@ -154,10 +154,9 @@ editor remains): into a pure `.elpx` player — uploads and playback keep working, but the "Edit with eXeLearning" button is hidden and the editor endpoints refuse requests. -* **Styles**: upload eXeLearning style packages (`.zip`), enable/disable the - editor's built-in styles, and optionally block users from importing styles - bundled inside an `.elpx`. A dedicated _Styles_ admin page lists and manages - them. +* **Styles**: upload eXeLearning style packages (`.zip`), list and + enable/disable the uploaded and built-in styles, and optionally block users + from importing styles bundled inside an `.elpx` — all on this page. * **xAPI**: master switch for the xAPI-primary grading channel. ## The embedded editor is a release artifact diff --git a/admin/styles.php b/admin/styles.php index 79fc73f..8b11b8d 100644 --- a/admin/styles.php +++ b/admin/styles.php @@ -15,7 +15,20 @@ // along with Moodle. If not, see . /** - * Admin page to manage uploaded eXeLearning styles. + * Action endpoint for the eXeLearning styles managed on the plugin settings page. + * + * This is deliberately NOT a management page (DEC-0067). Styles are managed on + * the plugin settings page, where the upload control sits inside the settings + * form and actually saves; a second visible manager here duplicated the widgets + * — including an upload control with no form around it, which silently discarded + * dropped files (UX-01 in the external validation of release 4.0.2). + * + * The script itself must survive: the per-row enable/disable/delete buttons are + * sesskey-protected GET links because a
nested inside the settings-page + * form is invalid HTML and used to break the whole settings save. Those links + * need a handler outside the settings form — this one — which processes the + * action (with a server-side confirmation for the destructive delete) and + * redirects back to the settings page. Do not add rendering back here. * * @package mod_exelearning * @copyright 2025 eXeLearning @@ -39,7 +52,8 @@ $PAGE->set_title(get_string('stylesmanager', 'mod_exelearning')); $PAGE->set_heading(get_string('stylesmanager', 'mod_exelearning')); -$returnurl = new moodle_url('/mod/exelearning/admin/styles.php'); +// Every outcome lands back on the settings page, the single place styles are managed. +$returnurl = new moodle_url('/admin/settings.php', ['section' => 'modsettingexelearning']); if ($action !== '' && confirm_sesskey()) { if ($action === 'enable' || $action === 'disable') { @@ -53,7 +67,7 @@ // server-side: a first hit (or a link prefetch) only shows the confirmation; the // actual delete needs the confirmed POST that $OUTPUT->confirm() generates. if (!optional_param('confirm', 0, PARAM_BOOL)) { - $confirmurl = new moodle_url($returnurl, [ + $confirmurl = new moodle_url('/mod/exelearning/admin/styles.php', [ 'action' => 'delete', 'slug' => $slug, 'confirm' => 1, @@ -79,37 +93,5 @@ } } -echo $OUTPUT->header(); -echo $OUTPUT->heading(get_string('stylesmanager', 'mod_exelearning')); - -echo \html_writer::tag('p', get_string('stylesmanager_intro', 'mod_exelearning')); - -// Upload form (native filemanager; auto-installs the dropped ZIP on save). -$upload = new \mod_exelearning\admin\admin_setting_stylesupload( - 'exelearning/styles_drops', - get_string('stylesupload_label', 'mod_exelearning'), - get_string( - 'stylesupload_hint', - 'mod_exelearning', - display_size(styles_service::get_max_zip_size()) - ), - 'styles_drops', - 0, - [ - 'accepted_types' => ['.zip'], - 'maxbytes' => styles_service::get_max_zip_size(), - 'maxfiles' => -1, - 'subdirs' => 0, - ] -); -echo $upload->output_html(''); - -// Uploaded styles list. -$uploaded = new \mod_exelearning\admin\admin_setting_stylesuploaded(); -echo $uploaded->output_html(''); - -// Built-in themes list. -$builtins = new \mod_exelearning\admin\admin_setting_stylesbuiltins(); -echo $builtins->output_html(''); - -echo $OUTPUT->footer(); +// No action to process (a direct visit): there is nothing to show here. +redirect($returnurl); diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index 5f7b13d..daeb0ab 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -265,8 +265,8 @@ embedded editor: `.elpx`. Authors may then only choose from the admin-approved list. Use this to keep a consistent, approved look across your site. -A dedicated **Styles** management page is also reachable directly under -*Site administration > Plugins > Activity modules*. +All style management happens on this settings page (the per-row buttons +confirm destructive actions before applying them). --- diff --git a/research/decisiones/adr/DEC-0067-pagina-estilos-solo-endpoint.md b/research/decisiones/adr/DEC-0067-pagina-estilos-solo-endpoint.md new file mode 100644 index 0000000..5104338 --- /dev/null +++ b/research/decisiones/adr/DEC-0067-pagina-estilos-solo-endpoint.md @@ -0,0 +1,74 @@ +--- +id: DEC-0067 +titulo: "La página de estilos queda como endpoint de acciones: se elimina el gestor visible duplicado" +estado: Aceptada +fecha: 2026-07-24 +agentes: + - erseco + - claude-code +fuentes: + - REPO-004 +relacionados: + - DEC-0049 + - DEC-0066 +herramienta_ia: + interfaz: claude-code + modelo: claude-fable-5 +--- + +## Contexto + +La gestión de estilos de eXeLearning vivía en **dos sitios** a la vez: + +1. **La página de ajustes del plugin**, donde los tres widgets (subida, subidos, + integrados) funcionan completos: el control de subida va dentro del formulario de + ajustes y se instala al pulsar "Guardar cambios". +2. **`admin/styles.php`**, registrada como página visible del menú de administración, que + volvía a pintar los mismos tres widgets con `output_html('')`. + +La segunda vida era una trampa: el widget de subida pintado a pelo **no tiene formulario +ni botón que lo envíe**, así que un ZIP arrastrado ahí parece aceptado, nunca se instala y +desaparece al recargar sin mensaje alguno. Es el hallazgo **UX-01** de la validación +externa de la release 4.0.2 (Tresipunt, que lo sufrió durante sus pruebas), y el informe +ofrecía dos salidas: envolver el control en su propio formulario, o retirar la vista y +dejar la gestión solo en ajustes. + +El fichero, sin embargo, **no puede desaparecer**: los botones activar/desactivar/borrar +de cada fila —también los de la página de ajustes— son enlaces GET con sesskey que +apuntan a `admin/styles.php`, porque un `` anidado dentro del formulario de la +página de ajustes es HTML inválido y llegó a romper el guardado completo de esa página +(corregido en el PR 81 convirtiendo los botones en enlaces; ver el registro de la +auditoría [[DEC-0049]]). Esas acciones necesitan un manejador fuera del formulario de +ajustes, y ese manejador es este script. + +## Decisión + +`admin/styles.php` queda como **controlador puro**, sin interfaz propia: + +- Conserva el procesado de acciones (enable/disable/delete de subidos, toggles de + integrados) y la **confirmación server-side del borrado** (la única pantalla que + renderiza, necesaria porque el borrado llega por GET y un prefetch no debe borrar). +- Tras cada acción **redirige a la página de ajustes**, que pasa a ser el único lugar de + gestión; una visita directa sin acción también redirige allí. +- El registro `admin_externalpage` se mantiene (lo exige `admin_externalpage_setup()` y + el árbol de administración) pero **oculto del menú** (`hidden = true`). +- Se elimina el render duplicado de los tres widgets, y con él la subida huérfana: + **UX-01 queda resuelto por eliminación de la superficie**, la opción (b) del informe. +- El docblock del script advierte explícitamente que no se le vuelva a añadir interfaz. + +## Consecuencias + +- Positivas: un único lugar de gestión (donde todo funciona); desaparece la subida que + descartaba ficheros en silencio; menos superficie duplicada que mantener y revisar; + los enlaces de acción existentes no cambian de URL (sin migración). +- Negativas (asumidas): quien tuviera `admin/styles.php` en marcadores aterriza en la + página de ajustes (redirección, no error); la "página de estilos" desaparece de la + documentación de usuario (actualizada en este cambio). + +## Alternativas consideradas + +| Alternativa | Por qué se rechaza | +|---|---| +| Envolver el control de subida de `styles.php` en su propio formulario (opción a del informe) | Mantiene dos gestores paralelos del mismo estado: más código, más superficie de revisión, y la duplicidad seguía confundiendo. | +| Dejarlo como estaba | La trampa de la subida silenciosa está confirmada por un tercero; "no romper nada" no justifica conservar una vista que descarta trabajo del administrador. | +| Mover las acciones a formularios POST propios dentro de ajustes | Imposible sin anidar formularios (el origen del problema que arregló el PR 81). | diff --git a/research/docs/indices/adrs.yaml b/research/docs/indices/adrs.yaml index ba11318..b03139e 100644 --- a/research/docs/indices/adrs.yaml +++ b/research/docs/indices/adrs.yaml @@ -62,3 +62,4 @@ items: - id: 'DEC-0064', titulo: 'Implementación de la ingesta xAPI (TAREA-015): xAPI-primary para paquetes nuevos, SCORM inerte, overall desde el statement de paquete, siempre activo', estado: 'Aceptada', fecha: '2026-06-18', ruta: 'decisiones/adr/DEC-0064-implementacion-ingesta-xapi.md' - id: 'DEC-0065', titulo: 'Editor embebido exclusivamente empaquetado en la release: eliminar el instalador/actualizador en runtime', estado: 'Aceptada', fecha: '2026-07-24', ruta: 'decisiones/adr/DEC-0065-editor-empaquetado-solo-en-release.md' - id: 'DEC-0066', titulo: 'Interruptor global del editor embebido: modo reproductor puro vía ajuste de sitio', estado: 'Aceptada', fecha: '2026-07-24', ruta: 'decisiones/adr/DEC-0066-interruptor-global-editor-embebido.md' + - id: 'DEC-0067', titulo: 'La página de estilos queda como endpoint de acciones: se elimina el gestor visible duplicado', estado: 'Aceptada', fecha: '2026-07-24', ruta: 'decisiones/adr/DEC-0067-pagina-estilos-solo-endpoint.md' diff --git a/settings.php b/settings.php index 2a06bce..4cab6cd 100644 --- a/settings.php +++ b/settings.php @@ -30,14 +30,18 @@ defined('MOODLE_INTERNAL') || die(); -// Register the styles management external page so it can be linked from the -// settings page and reached directly. Must be added before the $fulltree -// guard so it is always registered in the admin tree. +// Register the styles action endpoint. Hidden from the admin menu (DEC-0067): +// styles are managed on this settings page; admin/styles.php only processes the +// enable/disable/delete links (they cannot be forms nested inside the settings +// form) and hosts the delete confirmation. The registration must stay — it is +// what admin_externalpage_setup() resolves — and must be added before the +// $fulltree guard so it is always present in the admin tree. $ADMIN->add('modsettings', new admin_externalpage( 'mod_exelearning_styles', get_string('stylesmanager', 'mod_exelearning'), new moodle_url('/mod/exelearning/admin/styles.php'), - 'mod/exelearning:manageembeddededitor' + 'mod/exelearning:manageembeddededitor', + true )); // Register the site-wide migration tool only when a sibling plugin (mod_exeweb / diff --git a/tests/admin_setting_styles_render_test.php b/tests/admin_setting_styles_render_test.php index a7704c4..6937e7f 100644 --- a/tests/admin_setting_styles_render_test.php +++ b/tests/admin_setting_styles_render_test.php @@ -93,4 +93,22 @@ private function make_style_zip(string $name): string { $zip->close(); return $zippath; } + + /** + * The styles admin page stays registered (it is the action endpoint the row + * buttons post to, and admin_externalpage_setup() needs it) but hidden from + * the admin menu: managing styles happens on the plugin settings page, and a + * second visible manager page duplicated the widgets — including an upload + * control that silently discarded files (UX-01, DEC-0067). + */ + public function test_styles_admin_page_is_registered_but_hidden(): void { + global $CFG; + $this->resetAfterTest(); + $this->setAdminUser(); + require_once($CFG->libdir . '/adminlib.php'); + + $page = admin_get_root(true, false)->locate('mod_exelearning_styles'); + $this->assertInstanceOf(\admin_externalpage::class, $page); + $this->assertTrue($page->hidden); + } }