Skip to content

Fix optional template setting save without upload#66

Merged
eXeLearningProject merged 1 commit into
mainfrom
fix/optional-template-setting
Jun 28, 2026
Merged

Fix optional template setting save without upload#66
eXeLearningProject merged 1 commit into
mainfrom
fix/optional-template-setting

Conversation

@erseco

@erseco erseco commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

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 whose get_setting() is null) and the save can fail because admin_setting_configstoredfile::write_setting() returns an errorsetting when the filemanager posts no/empty draftitemid — 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

  • Add a plugin-local optional stored-file admin setting class \mod_exeweb\admin\admin_setting_optional_configstoredfile.
  • get_setting() reports '' instead of null for 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 calls file_save_draft_area_files() with an empty draft (which would delete an existing template) and never returns an error.
  • Positive draft item ids are delegated to core, preserving the normal upload / replace / intentional-clear behaviour. File type restrictions (['.zip', '.elpx']) are unchanged.
  • post_write_settings() reports "no change" for an untouched empty save, avoiding a spurious "changes saved" notice.
  • Replace the exeweb/template setting with the new optional stored-file setting (one line in settings.php).
  • Add PHPUnit regression tests for empty saves, empty drafts, uploads, replace, preserving existing files, intentional clear, and change detection.

Testing

  • Moodle phpcs (--standard=moodle, moodle-cs 3.7.0) — clean on the new files
  • PHPUnit mod/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.

Preview in Moodle Playground

ℹ️ 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.

@erseco erseco requested a review from ignaciogros June 25, 2026 12:28
@erseco erseco self-assigned this Jun 25, 2026
@erseco erseco requested a review from pabloamayab June 25, 2026 12:29
@eXeLearningProject eXeLearningProject merged commit 55e752d into main Jun 28, 2026
1 check passed
@eXeLearningProject eXeLearningProject deleted the fix/optional-template-setting branch June 28, 2026 11:27
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.

3 participants