Skip to content

Conversation

@BrainforgeUK
Copy link
Contributor

@BrainforgeUK BrainforgeUK commented Jun 3, 2019

Pull Request for Issue #24901.

Summary of Changes

Added event onContentValidateData
Event onUserBeforeDataValidation now depracted
Testing Instructions

Create a field plugin which implements onContentValidateData function like this...
public function onContentValidateData($form, &$data) {
// Change the data in some way
}
Expected result

The data get's changed.
Actual result

The data get's changed.
Documentation Changes Required

Add documentation for onContentValidateData event.
Add documentation for the existing, but undocumented and now deprecated, onUserBeforeDataValidation event.

For Joomla issue 19584
@BrainforgeUK
Copy link
Contributor Author

BrainforgeUK commented Jun 3, 2019

Replaces pull request #24925

@ghost ghost changed the title Added event onContentValidateData [4.0] Added event onContentValidateData Jun 3, 2019
@sakiss
Copy link
Contributor

sakiss commented Feb 13, 2020

I cannot understand the usefulness of that change.
Seems like a rename of an event for the shake of renaming it.

@BrainforgeUK
Copy link
Contributor Author

The event name onUserBeforeDataValidation implies it is only for user validation.
Changing to a generic event means reading the code makes a lot more sense when using the event in other extensions.

Factory::getApplication()->triggerEvent('onUserBeforeDataValidation', array($form, &$data));
}

Factory::getApplication()->triggerEvent('onContentValidateData', array($form, &$data));
Copy link
Contributor

@sakiss sakiss Feb 13, 2020

Choose a reason for hiding this comment

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

To be consistent with the other events (using before/after) something like onBeforeDataValidation possibly would be better

@BrainforgeUK
Copy link
Contributor Author

My main point in raising / proposing this is to replace the event name with something more meaningful when using it outside of user validation - code easier to read / search.

It occurs during validation so an On... name places it within validation while OnBefore... would imply it is outside validation.

A quick code search shows other OnContent... events including onContentBeforeDelete and onContentAfterDelete so naming it onContentValidateData looks like it fits in with previously used naming conventions.

@sakiss
Copy link
Contributor

sakiss commented Feb 13, 2020

My main point in raising / proposing this is to replace the event name with something more meaningful when using it outside of user validation - code easier to read / search.

It occurs during validation so an On... name places it within validation while OnBefore... would imply it is outside validation.

A quick code search shows other OnContent... events including onContentBeforeDelete and onContentAfterDelete so naming it onContentValidateData looks like it fits in with previously used naming conventions.

The event is triggered before the validation logic. It is important to clarify if it is called before or after, to be clear what actions could be carried out through that. Also onContentValidateData implies that the data are validated by that, which is not the case.

@wilsonge wilsonge merged commit cb3e5ec into joomla:4.0-dev Apr 1, 2020
@wilsonge
Copy link
Contributor

wilsonge commented Apr 1, 2020

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants