J3 Issue ?
avatar ankush-maherwal
ankush-maherwal
30 Aug 2019

What needs to be fixed

Access specifier of function "validateField()" in class "Form" is protected and which can be changed to public

Why this should be fixed

As of now, we are able to validate the entire form as well as fields of some specific group using the "validate()" method of "Form" class in the controller as the "validate()" method is public but we do not have any method which can validate any single field of the form.

The function "validate()" uses function "validateField()" to validate single field's data but as the function "validateField()" is protected we cant use it from controller to validate single field's data.

How would you fix it

Change the access specifier of the method "validateField()" in class Form to "public" so that the method can be used to validate single field's data

Side Effects expected

There is no harm in changing the access specifier of the method as the method is used to validate fields data and function from which it is called is itself public. Also, there are not any triggers in the method which can lead to misuse.

avatar ankush-maherwal ankush-maherwal - open - 30 Aug 2019
avatar joomla-cms-bot joomla-cms-bot - change - 30 Aug 2019
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - labeled - 30 Aug 2019
avatar franz-wohlkoenig franz-wohlkoenig - change - 30 Aug 2019
Labels Added: J3 Issue
avatar franz-wohlkoenig franz-wohlkoenig - labeled - 30 Aug 2019
avatar mbabker
mbabker - comment - 30 Aug 2019

The API is not designed for validating a single field, hence keeping that method protected. What is the use case where you're attempting to validate a single field in a form without validating the entire form?

avatar franz-wohlkoenig franz-wohlkoenig - change - 30 Aug 2019
Status New Information Required
avatar ankush-maherwal
ankush-maherwal - comment - 31 Aug 2019

@mbabker I am implementing ajax based autosave functionality for the form. I need to save/update the field value when a user adds or updates the field value.

Before adding or updating the field I need to validate its data and hence need a way to validate single field's data.

Changing the access specifier will allow the extension developers to use the validateField() method to validate individual field in the controller or as per there needs OR we can update the signature of the validate() method in the JForm class so that we can pass optional third parameter "fieldname" which is passed then it will validate only the specified field against the data.

avatar mbabker
mbabker - comment - 31 Aug 2019

TBH this doesn’t sound like a workflow a validation API should be
supporting. Generally, you’re giving the validator a set of data to
validate and checking it all in one go, the validator’s internals of
checking each item individually aren’t exposed as the public interface.

If you really need to use this kind of behavior, your best options are to
either do some manipulation in the controller to build a stub Form class
containing only the field you want to validate or to subclass the Form
class and make the method public yourself (which is allowed in PHP). I
don’t think the default core behavior should expose this internal behavior
as a public interface and therefore suggested workflow for all developers.

On Fri, Aug 30, 2019 at 11:57 PM ankush-maherwal notifications@github.com
wrote:

@mbabker https://github.com/mbabker I am implementing ajax based
autosave functionality for the form. I need to save/update the field value
when a user adds or updates the field value.

Before adding or updating the field I need to validate its data and hence
need a way to validate single field's data.

Changing the access specifier will allow the extension developers to use
the validateField() method to validate individual field in the controller
or as per there needs OR we can update the signature of the validate()
method in the JForm class so that we can pass optional third parameter
"fieldname" which is passed then it will validate only the specified field
against the data.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/joomla/joomla-cms/issues/26091?email_source=notifications&email_token=AACZ7IPP5L3N5VIT3DA3UOTQHH2Z5A5CNFSM4ISNHJSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5TFH6A#issuecomment-526799864,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AACZ7ILAMBZBU5CP6JBDTA3QHH2Z5ANCNFSM4ISNHJSA
.

--

  • Michael Please pardon any errors, this message was sent from my iPhone.
avatar coolbung
coolbung - comment - 3 Sep 2019

The Form::validate($data, $group = null) class already allows an optional group name, to validate only fields from that group. https://github.com/joomla/joomla-cms/blob/staging/libraries/src/Form/Form.php#L1182

The proposal is to add an optional third parameter that provides a specific field name to validate. For lengthy forms, this opens up a use case of building a autosave/draft save feature that only validates certain fields.

avatar mbabker
mbabker - comment - 3 Sep 2019

That group parameter is already a code smell to me. The interface on a
proper validation API (see: Symfony and/or Laravel) is checking a data
object or an array, it does not give you the ability to validate only a
single field on those items.

Like I said before, the right way to handle this “I want to validate only a
single field to do X” scenarios is either creating a stub Form instance
containing only the single field you want to validate, or since you want to
use the class internals and ignore the proper interface, subclass the Form
class so you can direct access the validateField method.

Either way, from an engineering perspective validating a single field only
in a dataset feels like an incorrect use of a validation API. This would
potentially allow you to reach a state where part of your dataset
(typically a database row) is in an invalid state yet the row is still
updated in its storage location (most likely database), which can create
more problems later on.

On Tue, Sep 3, 2019 at 7:14 AM Ashwin Date notifications@github.com wrote:

The Form::validate($data, $group = null) class already allows an optional
group name, to validate only fields from that group.
https://github.com/joomla/joomla-cms/blob/staging/libraries/src/Form/Form.php#L1182

The proposal is to add an optional third parameter that provides a
specific field name to validate. For lengthy forms, this opens up a use
case of building a autosave/draft save feature that only validates certain
fields.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/joomla/joomla-cms/issues/26091?email_source=notifications&email_token=AACZ7IMMZ4QAD2LLFFA3HO3QHZIIPA5CNFSM4ISNHJSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5X7QOI#issuecomment-527431737,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AACZ7IOUVD5FZSFXSNU3LA3QHZIIPANCNFSM4ISNHJSA
.

--

  • Michael Please pardon any errors, this message was sent from my iPhone.
avatar coolbung
coolbung - comment - 3 Sep 2019

The form field stub is a good idea. We'll try that out ! Thanks

avatar joomla-cms-bot joomla-cms-bot - change - 22 Dec 2019
Status Information Required Closed
Closed_Date 0000-00-00 00:00:00 2019-12-22 10:53:10
Closed_By joomla-cms-bot
avatar alikon alikon - change - 22 Dec 2019
Closed_By joomla-cms-bot alikon
avatar joomla-cms-bot joomla-cms-bot - close - 22 Dec 2019
avatar joomla-cms-bot
joomla-cms-bot - comment - 22 Dec 2019

Set to "closed" on behalf of @alikon by The JTracker Application at issues.joomla.org/joomla-cms/26091

avatar alikon
alikon - comment - 22 Dec 2019

i'm closing here as per suggested workaround


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/26091.

Add a Comment

Login with GitHub to post a comment