[#6229] - [imp] deprecate buggy usergroup field and replace ocurrences with usergrouplist. Fixes #3419
- Closed
- 5 Mar 2015
- Medium
- Build: 3.5-dev
- # 6229
- Diff
- phproberto:usergroup-list
- Error The Travis CI build could not complete due to an error Details
User tests: Successful: Unsuccessful:
See #3421
Steps to reproduce the issue
- Open this file /plugins/authentication/joomla/joomla.xml
- Before
</extension>add the following code:
<config>
<fields name="params">
<fieldset name="basic">
<field
name="myfield"
type="usergroup"
label="User Group">
<option value="">No Group</option>
</field>
</fieldset>
</fields>
</config>
- Go to Extension > Plugin Manager search for "authentication"
- Open the "Authentication - Joomla"
You should see a dropdown with default option "Show all groups" which is bad because you set
<option value="">No Group</option>in the above codeApply the patch
- Go back to /plugins/authentication/joomla/joomla.xml
- Change type="usergroup" to type="usergrouplist"
- Go the "Authentication - Joomla" plugin again
- You should now see the dropdown shows "No group"
- If you do see "No group" in the dropdown, it works!
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/3421.
Expected result
A dropdown with 'No Group' as the initially selected value.
Actual result
A dropdown with 'Show All Groups' as the initially selected value
System information (as much as possible)
JFormFieldUsergroup::getInput() returns with:
return JHtml::_('access.usergroup', $this->name, $this->value, $attr, $options, $this->id);
But the class is defined as:
JHtmlAccess::usergroup($name, $selected, $attribs = '', $allowAll = true)
Note that $options is being passed to $allowAll. On a slightly related note, the call is passing a fifth argument the method doesn't accept at all.
In addition, there is another form field JFormFieldUsergrouplist which does work as expected. It would seem both form fields are intended to serve the same function and so are duplicates. JFormFieldUsergroup should either be fixed or deprecated.
Additional comments
I have a plugin in which I want to have a user group optionally selected. Since it isn't required, JFormFieldUsergroup won't work. JFormFieldUsergrouplist does exactly what I need.
joomla-cms-bot
- | Labels |
Added:
?
|
||
zero-24
- | Category | ⇒ | Administration Libraries |
@phproberto The branch staging would actually be fine indeed.
3.5-dev is only needed in special cases where you want to change something which is already changed in 3.5-dev. Like for example if you want to fix a new feature.
We will commit into the correct branch when merging.
@phproberto You're correct that this PR will end up in the 3.5 branch. But we usually keep the PRs against staging and decide on the branch when it's merged. It's easier to test and also easier if the PR for some reason doesn't get accepted into 3.5.
In your case that is very unlikely as you're the release leader 
I have a small XML syntax improvement proposal here: instead of adding new type="usergrouplist", we could have this syntax:
<field type="usergroup" multiple="true" ... />
That way, if we want to apply multi-selects to other field types later, we can make it work too later without doubling potentially number of field types, without copy-pasting lots of code.
And if implemented in a SOLID way, it could just be a new active observer class that can be applied to any field-type without any change of code, which could be a big software architecture win starting renovating the JForms.
@Bakual all the fields will require that we migrate them to use JLayout so I guess it's better to have it in v3.5. The only issue for this is if someone submits a PR for the same against staging. I'm ok with either way so you decide but I think having a staging that can be released anytime and keeping v3.5 branch for features / deprecations is better.
@beat the issue here is not the multiple attribute but the field not accepting parent options. JFormFieldUsergroup is not extending JFormFieldList so you cannot use multiple there. That's why I created the JFormFieldUserGroupList field that already accepts the multiple attribute. About field flexibility I think you will love when fields are able to use layouts because that allows you to completely change a field behavior just changing the layout attribute.
I'm ok with either way so you decide but I think having a staging that can be released anytime and keeping v3.5 branch for features / deprecations is better.
It doesn't really matter much. Com_patchtester handles PRs against staging and 3.5-dev just fine.
Personally I prefer PRs being against staging for the sole reason that they don't need to change when 3.5 got released and the PR didn't get merged.
It doesn't change where the PR gets merged in the end. Features always will go into 3.5-dev. We just squash and commit them manually and can't directly merge them using the GitHub UI. But that's not a big issue and preferred anyway.
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-03-05 21:47:41 |
Why did you do the PR against 3.5-dev and not staging?
We can't test it at the Dutch PBF in Dordrecht if its against 3.5-dev.