Skip to content

[#152] Remove pluginfile tokens - #153

Open
owenherbert-catalyst wants to merge 1 commit into
catalyst:MOODLE_404_STABLEfrom
owenherbert-catalyst:issue-152-fix
Open

[#152] Remove pluginfile tokens#153
owenherbert-catalyst wants to merge 1 commit into
catalyst:MOODLE_404_STABLEfrom
owenherbert-catalyst:issue-152-fix

Conversation

@owenherbert-catalyst

@owenherbert-catalyst owenherbert-catalyst commented Mar 4, 2026

Copy link
Copy Markdown

Issue: #152

This PR improves handling of @@PLUGINFILE@@ tokens in search snippets and updates the associated PHPUnit tests.

Steps to reproduce and test:

  1. Install a 4.5 Moodle locally
git clone -b MOODLE_405_STABLE git@github.com:moodle/moodle.git moodle405stable
cd $DOCKER_DEV_ROOT/sites/moodle405stable
git clean -ffd && git submodule sync && git submodule update --init --recursive --force --jobs 16
control add moodle405stable --db pgsql --php 8.3 --elasticsearch
control install moodle405stable --adminuser admin --adminpass admin
  1. Install elastic search plugin https://github.com/mattporritt/moodle-search_elastic (MOODLE_404_STABLE)
  2. Create a new course named "test"
  3. Add a file to the course named "test", set description to "@@PluginFile@@/image.png", select a random file
  4. Save and display
  5. Go to /admin/searchareas.php, reindex courses search area
  6. Go to /admin/searchareas.php, reindex images search area
  7. Go to /admin/tool/task/scheduledtasks.php, run "Global search indexing" task
  8. Go to /search/index.php, search for "test". Obersve the warning
  9. Update the moodle-search_elastic plugin to use the feature branch owenherbert-catalyst:issue-152-fix
  10. Repeat step 9, observe the warning is gone.

Comment thread classes/document.php
* @return string HTML text to be renderer
*/
protected function format_text($text) {
// Remove @@PLUGINFILE@@ tokens and associated file paths from search snippets.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@owenherbert-catalyst , do I understand this correctly? We remove the token because there is no way to reconstruct the full plugin file URL?

I'm also wondering what the real-world use case is for reproducing this issue. The testing instructions seem to rely on a fairly artificial scenario.

@owenherbert-catalyst owenherbert-catalyst Aug 3, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @dmitriim , I created a core tracker for this a while ago. This can be seen here. This issue can be reproduced in all search plugins not just Elastic search.

I believe the more correct approach is to address it in core.

@dmitriim dmitriim Aug 3, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. That was my thought. I think we should close this PR and instead fix it in core.

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