Fix optional template setting save without upload#66
Merged
Conversation
ignaciogros
approved these changes
Jun 28, 2026
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
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
Fixes the optional "New package template" admin setting so the settings page can be saved when no template file is uploaded or when the filemanager is left untouched.
Problem
The template setting uses Moodle's
admin_setting_configstoredfile. During the plugin upgrade / new-settings flow, Moodle treats the setting as new (admin_output_new_settings_by_page()flags any setting whoseget_setting()isnull) and the save can fail becauseadmin_setting_configstoredfile::write_setting()returns anerrorsettingwhen the filemanager posts no/emptydraftitemid— even though the template is optional.This blocks admins on the "New settings" page unless they upload a template file (issue exelearning/exelearning#2000).
Changes
\mod_exeweb\admin\admin_setting_optional_configstoredfile.get_setting()reports''instead ofnullfor an unset optional file, so it is never treated as a new/unconfigured setting.write_setting()accepts an empty/missing/'0'/non-positive draft as a successful no-op: it never callsfile_save_draft_area_files()with an empty draft (which would delete an existing template) and never returns an error.['.zip', '.elpx']) are unchanged.post_write_settings()reports "no change" for an untouched empty save, avoiding a spurious "changes saved" notice.exeweb/templatesetting with the new optional stored-file setting (one line insettings.php).Testing
--standard=moodle, moodle-cs 3.7.0) — clean on the new filesmod/exeweb/tests/admin/admin_setting_optional_configstoredfile_test.php— 8 tests, 28 assertions, OK (Moodle 5.0.5, PHPUnit 11.5)Run in a Docker Moodle 5.0.5 environment (
erseco/alpine-moodle).Moodle Playground Preview
The changes in this pull request can be previewed and tested using a Moodle Playground instance.
ℹ️ The eXeLearning editor is fetched from the shared release and unpacked into the plugin when the playground boots, so the first load may take a few extra seconds. ELPX upload, viewer and preview work normally.