moodle-an-hochschulen / moodle-theme_boost_union

Theme Boost Union is an enhanced child theme of Boost which is intended, on the one hand, to make Boost simply more configurable and, on the other hand, to provide helpful additional features for the daily Moodle operation of admins, teachers and students.
GNU General Public License v3.0
55 stars 49 forks source link

Issue: Links to pluginfile.php assume that $systemcontext->id = 1 for all sites #627

Open NJahreis opened 2 months ago

NJahreis commented 2 months ago

Describe the bug Then using a moodle installation where the site context id does not equal "1" the url in the list of additional ressources and for customfonts is wrong as it assumes that the sitecontextid always equals one.

To Reproduce Steps to reproduce the behavior:

  1. Have a moodle site where the database field "id" of the context with contextlevel 10 (site context) is not 1 but anything else.
  2. Go to '/admin/settings.php?section=theme_boost_union_look#theme_boost_union_look_resources'
  3. Upload a ressource for the "theme_boost_union | additionalresources" option.
  4. Reload the page.
  5. Try to access the uploaded ressource using either of the urls provided in the list of ressources.
  6. The site will throw a database error: dml_missing_record_exception "Can't find data record in database table context."

The same steps can be used for the customfonts lists.

Expected behavior I expect to be able to access the ressource using the links in the list.

Additional context This most likely only affects extremely few users (maybe only myself) as normaly a clean moodle installation will always have the following entry in its database:

select * from mdl_context where contextlevel=10;
+-----+--------------+------------+------+-------+--------+
| id  | contextlevel | instanceid | path | depth | locked |
+-----+--------------+------------+------+-------+--------+
|   1 |           10 |          0 |   /1 |     1 |      0 |
+-----+--------------+------------+------+-------+--------+

However for me the entry in the database is:

select * from mdl_context where contextlevel=10;
+-----+--------------+------------+------+-------+--------+
| id  | contextlevel | instanceid | path | depth | locked |
+-----+--------------+------------+------+-------+--------+
| 150 |           10 |          0 | /150 |     1 |      0 |
+-----+--------------+------------+------+-------+--------+

I think on my site the installation was not performed on a completely new and clean database back in 2008. As this case should be extemely rare I think the existing test cases do not require expansion or changes to account for this issue. However as moodle core also does not assume that the id of the site context equals 1 for all installations I still believe it should be fixed in the theme.

Fixing the issue only requires minimal changes to locallib.php as can be seen here: https://github.com/moodle-an-hochschulen/moodle-theme_boost_union/compare/master...NJahreis:moodle-theme_boost_union:NJahreis-issue_pluginfile.php_syscontext