Skip to content

Add a site-wide switch to disable embedded editing (DEC-0066) - #108

Merged
erseco merged 3 commits into
mainfrom
feature/editor-enable-setting
Jul 24, 2026
Merged

Add a site-wide switch to disable embedded editing (DEC-0066)#108
erseco merged 3 commits into
mainfrom
feature/editor-enable-setting

Conversation

@erseco

@erseco erseco commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

What

A new admin setting, Disable the embedded editor (exelearning/editordisabled, unticked by default), lets a site use the plugin as a pure .elpx player: uploading and serving packages keeps working exactly as before, but the "Edit with eXeLearning" button is hidden and the editor endpoints refuse requests.

Why a negative checkbox

The first iteration used a positive "Enable the embedded editor" with default Yes — and rendered unticked next to "Default: Yes" on any site where upgradesettings had not yet materialised the default, contradicting the real behaviour. With the negative toggle, the unset config and the unticked box always mean the same thing: editing on. Same pattern as the plugin's existing stylesblockimport setting. The default behaviour is unchanged: the editor stays enabled unless an administrator ticks the box.

Why a setting and not a capability

Recorded in ADR DEC-0066 (included in this PR). The who of editing is already governed by moodle/course:manageactivities, consistent with the rest of the plugin; the real use case is binary and site-wide ("this site does not edit in place"), which is what activity plugins normally express as a feature switch in admin settings. A per-role capability would force sites to edit role definitions for a yes/no decision — and can still be added on top later without breaking anything, with the setting remaining the master switch.

How

  • exelearning_embedded_editor_enabled() now combines the toggle with bundle validation (DEC-0065), so everything that already consulted it reacts at once: the edit button in view.php, the asset router editor/static.php, and the create-from-scratch CTA (which degrades to the "upload a package" hint).
  • Hiding a button is not an access control, so direct requests are refused too: editor/index.php shows an explanatory error page and editor/save.php — the state-changing half — aborts through the new exelearning_require_embedded_editor_enabled() guard.
  • Strings added to the five language packs (machine translations carry the ~ review marker, per repo convention); README, docs/EMBEDDED_EDITOR.md and the user guide document the switch.

Tests

Written first and watched failing: with a valid bundle and the toggle ticked, the helper reports false and the endpoint guard throws moodle_exception; with no config at all, editing stays on and the guard passes. Suite: 7/7 locally on the helper file (Moodle 5.0.7 / PHP 8.3 / MariaDB), phpcs --standard=moodle 0/0 on every touched file.

Some sites use the plugin as a pure player: teachers upload .elpx packages
authored elsewhere and the site does not want to offer in-place editing. Until
now the "Edit with eXeLearning" button appeared for anyone with
moodle/course:manageactivities whenever the bundled editor was valid, with no
supported way to switch it off.

Add the exelearning/editorenabled admin setting (checkbox, default on; an unset
config counts as on so upgrades keep today's behaviour).
exelearning_embedded_editor_enabled() now combines the toggle with bundle
validation, so the edit button, editor/static.php and the create-from-scratch
CTA all react to it. Because hiding a button is not an access control, the
editor endpoints refuse direct requests too: editor/index.php shows an
explanatory error page and editor/save.php aborts through the new
exelearning_require_embedded_editor_enabled() guard. Uploading and serving
packages is unaffected either way.

A site-wide setting was chosen over a new capability (ADR DEC-0066): the "who"
is already governed by moodle/course:manageactivities, the real use case is
binary and site-wide, and feature switches in admin settings are how activity
plugins usually disable whole features. A per-role capability can still be
added on top later without breaking anything.

Strings added to the five language packs (machine translations carry the ~
review marker). Tests cover the default-on behaviour, the toggle disabling the
helper, and the endpoint guard throwing.
@codecov-commenter

codecov-commenter commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.31%. Comparing base (f39f7e7) to head (7c4a140).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff            @@
##               main     #108   +/-   ##
=========================================
  Coverage     92.30%   92.31%           
  Complexity      824      824           
=========================================
  Files            51       51           
  Lines          3730     3734    +4     
=========================================
+ Hits           3443     3447    +4     
  Misses          287      287           
Flag Coverage Δ
javascript 94.11% <ø> (ø)
php 92.23% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
PHP (server-side) 92.23% <100.00%> (+<0.01%) ⬆️
JavaScript (SCORM tracker) 94.11% <ø> (ø)
Files with missing lines Coverage Δ
lib.php 80.13% <100.00%> (+0.27%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

erseco added 2 commits July 24, 2026 12:49
The checkbox's own description already explains the switch; repeating a
condensed version in the heading added noise without information.
Review caught the classic new-setting confusion: "Enable the embedded editor"
rendered UNTICKED next to "Default: Yes", because a positive default only
materialises when upgradesettings runs — until then the stored value is unset
and the box contradicts the actual behaviour.

Replace editorenabled with editordisabled (default 0, unticked): now the unset
config and the unticked checkbox both mean "editing on", on every site and at
every moment, with no dependency on when settings were saved. Same pattern as
the plugin's existing stylesblockimport toggle. Default behaviour is unchanged:
the embedded editor stays enabled unless an admin ticks the box.
@erseco
erseco merged commit 7f45737 into main Jul 24, 2026
25 checks passed
@erseco
erseco deleted the feature/editor-enable-setting branch July 24, 2026 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants