Skip to content

Check for the real default media source value and don't use a fixed v…#16962

Closed
Jako wants to merge 1 commit into
modxcms:3.xfrom
Jako:check-default-media-source-value
Closed

Check for the real default media source value and don't use a fixed v…#16962
Jako wants to merge 1 commit into
modxcms:3.xfrom
Jako:check-default-media-source-value

Conversation

@Jako

@Jako Jako commented May 31, 2026

Copy link
Copy Markdown
Contributor

What changed and why

Check for the real default media source value and don't use a fixed value.

@Jako Jako requested review from Mark-H and opengeek as code owners May 31, 2026 18:18
@smg6511

smg6511 commented Jun 1, 2026

Copy link
Copy Markdown
Collaborator

@Jako @Ibochkarev: You're misinterpreting what the purpose of this line is—It's to prevent the built-in Filesystem Source (which should always be id 1) from being deleted. That source is not necessarily the default source in a multi media source setup. As such, no change is needed. Perhaps a short doc block above the line in question explaining this could be helpful though.

@Jako

Jako commented Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

@smg6511 After I have seen the installations of one client, I think a bit different about this: the media source 1 of several installations was changed into a 'Client Media Source' and pointed to something like {base_path}user/whatever. The filesystem media source was created as a second media source with a custom name.

If we don't want that media source 1 to be deleted, we also have to make sure that it can't be edited, too. Since this would be a bit too restrictive to me, the check for the default_media_source setting is more suitable.

@smg6511

smg6511 commented Jun 1, 2026

Copy link
Copy Markdown
Collaborator

@Jako - We actually already more clearly established the Filesystem media source as being protected a couple or so releases ago. While it cannot be removed or renamed, its settings can be edited. Although this change may eventually cause some confusion for the likely small percentage of folks who renamed this source (and subsequently try to rename it again), their setups would be otherwise unaffected. Remember that it's possible to have no source as the default media source, so there's not fool-proof protection by disallowing the deletion of the default source as you propose.

@Jako

Jako commented Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

Then at least the default_media_source system/context/user setting check should be added there to disallow these sources too.

@smg6511

smg6511 commented Jun 2, 2026

Copy link
Copy Markdown
Collaborator

@Jako You raise a good point and the UX can definitely be improved. I think the best way, rather than disallow the removal of a non-built-in source that's specified as a default, would be to mark the current default in the Sources grid (maybe via an icon or asterisk after the name) and maybe add a variation on the current warning/confirmation dialog that addresses the implications of removing that source.

@Jako

Jako commented Jun 6, 2026

Copy link
Copy Markdown
Contributor Author

The getlist processor can check for the default media sources in the system/context/user settings and return these values. The warning message can be extended with the value of the processor.

@Jako Jako closed this Jun 6, 2026
@Jako Jako deleted the check-default-media-source-value branch June 6, 2026 10:14
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