feat(styles): administrator-managed style registry for the embedded editor#13
Open
feat(styles): administrator-managed style registry for the embedded editor#13
Conversation
…ditor
Adds a **Module settings → eXeLearning → Styles** admin page where site
admins upload eXeLearning style .zip packages, enable/disable built-in
styles, and enable/disable/delete uploaded ones — without rebuilding
the static editor bundle or editing the module source tree.
Architecture
- `ExeLearning\Service\StylesService` owns the pure logic: ZIP
validator (path traversal, absolute paths, size cap, extension
allow-list, single config.xml), slug allocation with collision
suffix, registry persistence in Omeka settings, and the
`buildThemeRegistryOverride()` payload the editor consumes.
- `ExeLearning\Controller\Admin\StylesController` renders the
admin page and handles upload/toggle/delete via POST + CSRF.
- `ExeLearning\Controller\StylesServeController` serves extracted
style assets via the `/exelearning/styles/:slug/:file` public route
with traversal + registry gating.
- `EditorController` injects
`window.eXeLearning.config.themeRegistryOverride` and mirrors
`blockImportInstall` onto the pre-existing `userStyles`
(ONLINE_THEMES_INSTALL) flag so the 'Install this project style?'
modal is suppressed end-to-end.
- `Module.php::getConfigForm` adds a "Styles" link fieldset so
admins can jump from the module config page to the management UI.
Storage
Uploaded style bundles extract to
`{OMEKA_PATH}/files/exelearning-styles/{slug}/` — a sibling of the
ELP extraction directory, so reinstalling the embedded editor never
destroys admin-managed styles.
Admin toggle
'Block user-imported styles' defaults to **off** (imports allowed),
matching the upstream ONLINE_THEMES_INSTALL=true default. Admins opt
in to the lockdown from the Styles page.
Tests
`test/ExeLearningTest/Service/StylesServiceTest.php` covers the
validator, registry shape, install, unique-slug on collision, delete,
override payload, the import-blocked contract and the
double-nested / flat bundle.json shapes.
Minor
`.gitignore`: anchor `exelearning/` to the repo root so it does
not also ignore `view/exelearning/admin/styles/`.
Depends on
- exelearning/exelearning#1722 — runtime `themeRegistryOverride` hook (merged).
- exelearning/exelearning#1724 — hides the 'Imported' tab when blocked.
Contributor
omeka-s Playground Preview
Try this PR in your browser
|
Adds Spanish msgstr entries for every user-facing string introduced by the styles management feature: page title, upload form, table headers, import policy toggle labels, and delete confirmations. Recompiled es.mo.
CI reported 69.84% after the styles PR landed with three new classes at 0%. This commit adds: - test/ExeLearningTest/Form/StylesUploadFormTest.php — file, csrf, submit element presence and attributes. - test/ExeLearningTest/Controller/StylesServeControllerTest.php — guessMimeType() covers the full allowed extension list, notFound() returns 404, and serveStyle() exercises the happy path + every refusal branch (unregistered slug, traversal, missing file). - test/ExeLearningTest/Controller/Admin/StylesControllerTest.php — processUploads() covers empty input, single-file + multi-file normalization, broken upload error codes, and downstream service failures. - test/ExeLearningTest/Service/StylesServiceTest.php — extra cases: bundle.json reader (present/missing/malformed), allocateUniqueSlug with collisions, normalizeSlug edge cases, isUnsafeZipEntry / isAllowedFilename matrices, parseConfigXml errors, oversize ZIP rejection, single-root-folder extraction, multi-config archive, CSS-less archive, buildThemeRegistryOverride URL variants, recursiveDelete on missing path. Also: - Refactor StylesServeController: extract the routing-independent body into a public serveStyle($slug, $file) so tests don't need to wire a full Laminas MVC plugin manager. - Add test/Stubs/Laminas/Router/RouteMatch.php and test/Stubs/Laminas/Mvc/MvcEvent.php for any future tests that exercise routed actions.
First coverage pass raised the suite to 85% but CI still rejected it (90% threshold). This round closes the remaining gaps: StylesController - Expose allowed() as protected so a test subclass can override it. - test/ExeLearningTest/Controller/Admin/TestableStylesController.php: lightweight MVC plugin doubles for params(), getRequest(), redirect(), messenger(). - StylesControllerTest: index denied / allowed, toggleUploaded / toggleBuiltin / delete / toggleBlockImport actions verified for POST-only guards, service delegation, flash messages. StylesService - Tests for setUploaded/delete on unknown slug, getStyleUrlPath with non-slug input, listUploadedStyles skipping scalar entries, buildThemeRegistryOverride ignoring scalar/disabled entries, getRegistry surviving garbage JSON, validateZip on empty/missing/ non-zip files, install paths with minimal config.xml, checksum sha256 prefix, CSS discovery with/without style.css, archive whose config is not at the prefix, recursiveDelete on deep trees.
Three integration fixes to bring omeka-s-exelearning in line with wp-exelearning after end-to-end testing in WordPress: - Trap reassignments of window.eXeLearning and window.eXeLearning.config. The static editor's boot sequence reassigns both (the inline script in index.html resets the whole object, and app.bundle.js later parses 'config' from JSON back into an object), so a plain assignment of themeRegistryOverride is wiped before the editor reads it. Wrap the injection in a self-restoring defineProperty getter/setter so the override and the userStyles mirror survive every reset. - Switch the uploaded-style type from 'uploaded' to 'admin' in the override payload. The editor's navbar tabs filter strictly: 'base' | 'site' | 'admin' render in Sistema, 'user' in Imported, and any other value is silently dropped. Admin-uploaded styles match 'admin' semantically and land on the Sistema tab alongside built-ins. - Publish a 'files' manifest alongside each uploaded entry, enumerated from the extracted storage directory. The embedded editor's ResourceFetcher consumes it via themeRegistryOverride.uploaded[].files to fetch admin-approved styles file by file instead of expecting a zip under /bundles/themes/, so preview and HTML5 export pack the real theme assets under theme/* rather than falling back to the placeholder theme/content.css + theme/default.js.
Four integration fixes discovered while uploading and selecting an
admin-approved style end-to-end:
- Accept Laminas-transposed multi-file uploads. The Laminas File
element with `multiple` delivers $_FILES through params()->fromFiles()
as a list of per-file dicts ([{name,tmp_name,error}, ...]) rather
than the native PHP multi-shape ({name: [...], tmp_name: [...]}),
so the controller was falling through to a code path that read
undefined array keys and reported every upload as "Upload failed:
unknown / no file was selected".
- Report the underlying PHP upload error code. Mapping UPLOAD_ERR_*
constants to admin-facing strings makes misconfigurations (php.ini
upload_max_filesize, missing tmp dir, etc.) diagnosable from the
flash message alone.
- Point the admin stylesheet at the real asset. index.phtml was
pulling css/exelearning-admin.css, which doesn't exist; the module
actually ships asset/css/exelearning.css.
- Route /exelearning/styles/* to PHP in nginx. Omeka's base vhost
intercepts .css/.js/.png via a regex location that short-circuits
to the filesystem, so requests for admin-uploaded style assets
never reached StylesServeController. Add a prefix `^~
/exelearning/styles/` location that matches before the regex and
defers to index.php.
…ti-file paths
- Update testProcessUploadsRecordsErrorsForBrokenUploads: the error
string changed to 'Upload failed for <name>: <reason>' after the
UPLOAD_ERR_* mapping landed. Assert the new prefix.
- Add testProcessUploadsHandlesTransposedListShape — covers the
Laminas multi-file shape ([{tmp_name,...}, {tmp_name,...}])
that slipped the 0% coverage net.
- Add testUploadErrorMessageCoversEveryPhpErrCode — iterate every
PHP UPLOAD_ERR_* constant plus an unknown code to pin down the
mapping helper.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a Module settings → eXeLearning → Styles admin page where site admins upload eXeLearning style
.zippackages, enable/disable built-in styles individually, and enable/disable/delete uploaded ones — all without rebuilding the static editor bundle or editing module source files.Consumed by the runtime hook in the core editor (merged as
exelearning/exelearning#1722). Adds a tab-hide follow-up as exelearning/exelearning#1724.
Changes
src/Service/StylesService.php+ factory — pure logic: ZIP validator(traversal / absolute paths / size cap / extension allow-list /
single config.xml), slug allocation with collision suffix, registry
persistence via
Omeka\Settings, and thebuildThemeRegistryOverride()payload the editor consumes.src/Controller/Admin/StylesController.php+ factory — admin page:upload form (native multi-file
<input type="file">), built-in +uploaded lists with toggle/delete. ACL:
Omeka\Entity\Moduleupdate.src/Controller/StylesServeController.php+ factory — public route/exelearning/styles/:slug/:filethat serves extracted style assetswith traversal + registry gating.
src/Form/StylesUploadForm.php— multi-file upload form + CSRF.view/exelearning/admin/styles/index.phtml— styles admin UI.src/Controller/EditorController.php— injectswindow.eXeLearning.config.themeRegistryOverrideand mirrorsblockImportInstallonto the pre-existinguserStyles(ONLINE_THEMES_INSTALL) flag so the "Install this project style?"
modal is suppressed end-to-end.
Module.php::getConfigForm— adds a "Styles" link fieldset to themodule config page.
config/module.config.php— routes, services, controllers, formand a navigation entry.
test/ExeLearningTest/Service/StylesServiceTest.php— 12 testscovering validator edge cases, registry, install, unique-slug,
delete, override payload, import-blocked contract, both bundle
shapes.
test/Stubs/Omeka/Settings/Settings.php— in-memory stub..gitignore— anchorexelearning/to the repo root so it doesn'talso ignore
view/exelearning/admin/styles/.Storage
Uploaded style bundles extract to
{OMEKA_PATH}/files/exelearning-styles/{slug}/— a sibling of theELP extraction directory, so reinstalling the embedded editor never
destroys admin-managed styles.
Admin toggle: "Block user-imported styles"
Defaults to off (imports allowed), matching upstream
ONLINE_THEMES_INSTALL=true. Admins opt in to the lockdown from theStyles page.
Test plan
phpunit --filter StylesServiceTest→ 12/12 pass, 34 assertions.php -lclean on all new/modified files.they show in the editor's style selector.
editor's style selector.
"Imported" tab appears / disappears.
editor falls back to
basewithout errors.Depends on
themeRegistryOverridehook (merged).