Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
7 changes: 3 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
56 changes: 19 additions & 37 deletions admin/styles.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,20 @@
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.

/**
* 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 <form> 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
Expand All @@ -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') {
Expand All @@ -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,
Expand All @@ -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);
4 changes: 2 additions & 2 deletions docs/USER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).

---

Expand Down
74 changes: 74 additions & 0 deletions research/decisiones/adr/DEC-0067-pagina-estilos-solo-endpoint.md
Original file line number Diff line number Diff line change
@@ -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 `<form>` 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). |
1 change: 1 addition & 0 deletions research/docs/indices/adrs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
12 changes: 8 additions & 4 deletions settings.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 /
Expand Down
18 changes: 18 additions & 0 deletions tests/admin_setting_styles_render_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Loading