Skip to content

Remove unreachable refresh_site controller#16947

Open
opengeek wants to merge 1 commit into
modxcms:3.xfrom
opengeek:retire/refresh_site
Open

Remove unreachable refresh_site controller#16947
opengeek wants to merge 1 commit into
modxcms:3.xfrom
opengeek:retire/refresh_site

Conversation

@opengeek
Copy link
Copy Markdown
Member

@opengeek opengeek commented Apr 28, 2026

What does it do?

Removes the unused system/refresh_site manager controller path:

  • Deletes manager/controllers/default/system/refresh_site.php
  • Deletes manager/templates/default/system/refresh_site.tpl

Lexicon entries refresh_site / refresh_site_desc (in core/lexicon/en/topmenu.inc.php) are retained — they're used as the Clear Cache topmenu item's label and description, not as path references.

Why is it needed?

The controller is dead code, unreachable in-tree:

  • The topmenu Clear Cache entry in _build/data/transport.core.menus.php uses 'action' => '' with 'handler' => 'MODx.clearCache(); return false;' — it routes to the System/ClearCache connector processor, not to this controller.
  • No PHP include/require references the file, no JS or URL constructions reference ?a=system/refresh_site, and no other code references the would-be SystemRefreshSiteManagerController class.
  • The only entrypoint was direct URL navigation to /manager/?a=system/refresh_site, which has been broken since the modManagerController loader started requiring .class.php files.

PR #16827 proposed converting the script-style file into a controller class to fix the broken URL. Triage determined the maintenance surface isn't worth preserving an entrypoint with no callers and no menu route. Closing #16827 in favor of retirement.

How to test

  1. On a fresh 3.x install, click Manage → Clear Cache from the topmenu — confirm the cache is cleared (OnBeforeCacheUpdate / OnSiteRefresh still fire from the connector processor path).
  2. Visit /manager/?a=system/refresh_site directly — confirm the response is a clean "unknown action" from modManagerResponse rather than a fatal.
  3. Confirm no other manager screens regress (search the manager UI for any incidental references — none expected).

Related issue(s)/PR(s)

Resolves #16282 (by retirement rather than fix). Supersedes #16827.

@opengeek opengeek added this to the v3.2.1 milestone Apr 28, 2026
@opengeek opengeek requested a review from Mark-H as a code owner April 28, 2026 21:51
Copy link
Copy Markdown
Collaborator

@smg6511 smg6511 left a comment

Choose a reason for hiding this comment

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

Confirmed all works as expected. No found references to controller.

@opengeek opengeek modified the milestones: v3.2.1, v3.3.0 May 13, 2026
@JoshuaLuckers
Copy link
Copy Markdown
Contributor

Shouldn’t there be an upgrade script that removes these files as well?

@opengeek
Copy link
Copy Markdown
Member Author

Shouldn’t there be an upgrade script that removes these files as well?

Maybe? IDK. I'm torn since I generally use with git and removing files in a script feels redundant.

@JoshuaLuckers
Copy link
Copy Markdown
Contributor

Shouldn’t there be an upgrade script that removes these files as well?

Maybe? IDK. I'm torn since I generally use with git and removing files in a script feels redundant.

If they were part of a release, we used to remove them during setup.
E.g: https://github.com/modxcms/revolution/blob/3.x/setup/includes/upgrades/common/3.0.0-cleanup-files.php

@opengeek
Copy link
Copy Markdown
Member Author

Shouldn’t there be an upgrade script that removes these files as well?

Maybe? IDK. I'm torn since I generally use with git and removing files in a script feels redundant.

If they were part of a release, we used to remove them during setup. E.g: https://github.com/modxcms/revolution/blob/3.x/setup/includes/upgrades/common/3.0.0-cleanup-files.php

I recall — though that was specifically cleaning up old 2.x paths abandoned in 3.x. 🤔

@smg6511
Copy link
Copy Markdown
Collaborator

smg6511 commented May 15, 2026

But, there's nothing that would remove them in an typical upgrade situation otherwise, right?

@opengeek
Copy link
Copy Markdown
Member Author

But, there's nothing that would remove them in an typical upgrade situation otherwise, right?

There's not, but we've never established a standard for this other than the one cleanup script for 3.0.0 release that I can find.

@opengeek
Copy link
Copy Markdown
Member Author

Further, it looks like the way it was handled in that script would not work if you used the advanced installer to put the manager in a different location (or manually moved it to a different location).

@JoshuaLuckers
Copy link
Copy Markdown
Contributor

I wasn’t totally sure what we used to do with left over files in the past, it has been a while for me!

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.

Revo 3: the action /manager/?a=system/refresh_site does not work properly

4 participants