Validate the ID target for symlinks

[OptimusCrime]

What does it do?

This PR attempts to introduce validation for symlink targets. The symlinks needs to specify an existing resource in the system, and it is fairly easy to check if the user misspelled their target value.

The check is implemented for both update and create processors.

Note: I have left out the validation when you change the class type. If you have a modDocument, and then decide to change it into a modSymLink, we would attempt to validate the value of the content field. This can be anything, and would most likely produce a lot of errors and problems for the developers. Because of that, we only validate resources that are NEW modSymLinks, or when the user attempts to save an existing modSymLink. Also note that we do not validate that the field has any values, as this has previously not been required.

Why is it needed?

It adds a sanity check that I think only improves usability.

Related issue(s)/PR(s)

#10141

[JoshuaLuckers]

The current symlink_help lexicon entry needs to be modified to reflect this change:

The address of the object you wish to reference with this Symlink. If you want to point to an existing MODX Resource, enter the ID here.

Another thing I noticed. If I have an existing resource with id 2 and I enter 2 0 (resource with ID 20 does not exist and notice the space in the field) I'm able to save the resource without an error.

[OptimusCrime]

Thank you for feedback @JoshuaLuckers. I agree, the text needs to be adjusted. I am still a bit unsure if this is an enhancement we want though. I'll wait to see what Jason and Mark says :)

I can also expand the validation by adding a check that validates that the input is a number.

[JoshuaLuckers]

Maybe it should be a warning instead of an error.

[Jako]

A few thoughts:

• modWeblink could be regarded too.
• Maybe it is an option to check the target id with filter_var($target, FILTER_VALIDATE_INT) before passing it to $this->modx->getObject('modResource', $target).
• Is the symlink content parsed and could it contain a snippet? Then the validation has to be completely changed.
• The validation during changing a modResource to modSymLink/modWebLink should run, but it should just log an error for an invalid content.

[OptimusCrime]

Good points, I'll try to implement this!

[Mark-H]

Minor tweak - as @Jako also mentioned, this is an alternative way to require the provided content to be an integer ID. Makes it a bit more explicit than the filter_var approach imo.

[alroniks]

I wondering is it possible to use traits here to avoid code duplication?

[Jako]

My issues with this PR should be fixed with these changes.

[Mark-H]

👍

[OptimusCrime]

@Jako awesome work man :) Thanks for taking the time to fix it.

[opengeek]

This is a problem unfortunately as this PR causes a significant BC break. The target of a symlink or weblink can be a MODX tag that is evaluated at runtime and this destroys the ability to maintain those.

[OptimusCrime]

@opengeek What do you mean? @Jako added a filter, so it only tries to validate integer values. It will skip any MODX tags.

[opengeek]

I mean that the target value can be [[SnippetToReturnAnIntegerValueAtRuntime]], or any other valid tag expression that can be parsed by MODX.

[OptimusCrime]

Yes, and those will be ignored, because there is not really any good way to validate them. This PR only validates if the user enters an integer value.

[opengeek]

Not according to my testing…

[OptimusCrime]

I tried to look at the code, but I do not have the time to properly investigate this now. I suspect one of the filters is incorrect.

[Jako]

The weblink code checks for an integer and validates afterwards, since it could be an external link. The Symlink code validates only, if it is an integer. Thats the issue here.