fix: stop upgradesettings.php loop on styles upload widget (#47)#53
Merged
Conversation
`admin_setting_stylesupload` extends `admin_setting_configstoredfile`, whose `get_setting()` reads the stored file id from plugin config. Our `write_setting()` consumes the dropped ZIP and removes it, never persisting anything in `config_plugins`, so `get_setting()` kept returning `null`. Moodle's `admin_output_new_settings_by_page()` flags any setting whose `get_setting()` is null as "new", redirecting every admin login to `admin/upgradesettings.php` and re-rendering the widget no matter how many times "Save changes" was clicked. Override `get_setting()` to return an empty string so the setting is never reported as new. The widget always builds a fresh draft area, so no stored value is needed for rendering, and `write_setting()` keeps behaving exactly as before.
This was referenced May 9, 2026
ignaciogros
approved these changes
May 11, 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
3 participants
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 #47 (and the duplicate #51).
After upgrading to v4.0.0, every admin login was being redirected to
admin/upgradesettings.phpwith the Styles upload widget shown as a "new setting" — and clicking Save changes never cleared it, even when a style was selected.Root cause
admin_setting_stylesuploadextendsadmin_setting_configstoredfile, whoseget_setting()reads the stored file id from plugin config. Ourwrite_setting()extracts the uploaded ZIP and immediately deletes it, so nothing is ever persisted inconfig_plugins. As a result,get_setting()always returnednull. Moodle'sadmin_output_new_settings_by_page()(inlib/adminlib.php, line 9231) flags any setting whoseget_setting()is null as new, so the upload widget reappeared on every visit and the admin was trapped in the loop.Fix
Override
get_setting()inadmin_setting_stylesuploadto return an empty string. The widget itself does not need a stored value — it always starts from a fresh draft area — so this is functionally equivalent to "nothing pending" while telling Moodle the setting has been seen.write_setting()keeps behaving exactly as before.This mirrors the pattern already used in
admin_setting_embeddededitor(the embedded-editor management widget), which solved the same problem the same way.Test plan
/admin/upgradesettings.php?return=admin— the Style packages (.zip) row no longer appears as a new setting.\core\notification.Sibling PR
Same fix on the SCORM side: exelearning/mod_exescorm#68 (issue exelearning/mod_exescorm#66).
Closes #47. Closes #51 as duplicate.