? Failure

User tests: Successful: Unsuccessful:

avatar Harmageddon
Harmageddon
27 Mar 2019

Summary of Changes

The test method of validation rules has a $group argument that should contain the group name / path of the field inside the form as a string. For example, a field like

<fields name="params">
	<fieldset name="basic">
		<field
			name="text"
			type="text"
			label="Text"
		/>
	</fieldset>
</fields>

can be found inside the group "params" and is thus addressed as "params.text" instead of only "text".

Before #12414, in the Form class, the variable $group, originally a parameter of the function, but later overwritten, was passed as the parameter. #12414 separated parameter from variable, but continued to pass the, now unchanged, parameter. IMHO, we should rather pass the variable here, meaning $attrGroup. As far as I see, the function Form::validate is never given a value for $group inside the Joomla! core.
If you have an extension making use of the $group argument, please check if this PR breaks anything for you!
The problem occurs, when we validate a field that is inside a fields group with a rule that uses $group. In core, this applies for the rules "equals" and "options". "equals" compares the value of the current field with that of another field inside the same group (in fact, as I'm writing this, I'm not sure what is the reason behind this. Shouldn't we also allow the equal rule to be applied to two fields of different groups?).
"options" tries to load the FormField instance. If it fails, it only uses the <option> tags inside the XML declaration for validation. In core, this works because this validation rule is only applied to pure list fields that have all their options inside the XML. If we now use a more complex FormField like "category", this no longer works.

Testing Instructions

  1. Create a form with two fields using validation="equal" and one complex list field using validation="options". All fields should be inside a fields group, meaning nested inside a <fields name="something"> tag.
    You can see an example at https://github.com/Harmageddon/mod_demovalidation/tree/form_validation_group. Feel free to test different settings.
  2. Correctly fill the form values, e.g. enter the same value into the two "equal" fields and choose one from the values of the list field.
  3. Try to save the form.

Expected result

The form should be correctly saved.

Actual result

The form is rejected because the validation rules fail.

Apply this PR and test again. The form should now be validated correctly.

Other possible solutions

The alternative would be to change the mentioned form rules to get the group identifier using the XML of the fields. However, I'd say that we would have a misleading documentation for the $group parameter of FormRule::test.

avatar Harmageddon Harmageddon - open - 27 Mar 2019
avatar Harmageddon Harmageddon - change - 27 Mar 2019
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 27 Mar 2019
Category Libraries
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 27 Mar 2019

can you please add "[4.0] " at Start of Title?

avatar Harmageddon Harmageddon - change - 27 Mar 2019
Title
Pass correct group attribute to form validation.
[4.0] Pass correct group attribute to form validation.
avatar Harmageddon Harmageddon - edited - 27 Mar 2019
avatar Harmageddon
Harmageddon - comment - 27 Mar 2019

Done, thank you for spotting this!

How can I access the screenshot and page source that Drone stored after failing? Or restart the build?

avatar Harmageddon Harmageddon - change - 5 May 2019
Labels Removed: J4 Issue
avatar Harmageddon
Harmageddon - comment - 5 May 2019

Closing because this has apparently been fixed with #24633.

avatar Harmageddon Harmageddon - change - 5 May 2019
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2019-05-05 14:02:27
Closed_By Harmageddon
avatar Harmageddon Harmageddon - close - 5 May 2019

Add a Comment

Login with GitHub to post a comment