Hi Roberto,
Thanks for this improvement (and of course for all your other contributions).

Shouldn't this PR be made against 3.3-dev branch as it seems to introduce potential backwards compatibility issues ?

Shouldn't this PR be made against 3.3-dev branch as it seems to introduce potential backwards compatibility issues ?

Why do you think there could be BC issues? All it does is

• deprecate the existing field. Extensions can still used it as before. Nothing changes here.
• Change the use in com_users to the other field. I don't see how that could affect 3rd party extensions. Especially as it does the same thing (as I understand).

However according to SemVer which we will use starting with 3.4, it would indeed have to go into the new minor. But only because new deprecations aren't allowed to go into a patch (see point 7).

Hi Thomas,
It is a backwards-compatibility issue because:

1. Any extension extending that JForm will not find that field anymore (ok, not a lot of extensions affected but still a BC issue)
2. Potentially filling logs with suddenly deprecate messages is a DoS/DDsS vulnerability issue.

Those are the exact reasons why SemVer requests deprectations to go into minor release increment.

Best Regards

Any extension extending that JForm will not find that field anymore (ok, not a lot of extensions affected but still a BC issue)

Please note that the field is still there. We don't remove it. We just deprecate it.
It will then be removed with 4.0, not earlier.

Potentially filling logs with suddenly deprecate messages is a DoS/DDsS vulnerability issue.

The log message is only written if the admin has that message logging ("Log deprecated API" in the "Logging" tab) enabled in the debug plugin.
It's disabled by default for the exact reason you noted. Try it once yourself, the log will already be filled fast, I promise

Thanks @Bakual & @beat for the feedback.

My initial thought was to merge it for 3.3 because it's a simple change B/C. But I'm ok with 3.4 too, I created UserGroupList to avoid dealing with the UserGroup field. So anybody that needs it can use the new field.

@phproberto: 3.3 is fine. Now wondering: is "staging" branch 3.3 or 3.2.next ? There is still a "3.3-dev" branch, thus I supposed that staging was 3.2.next. Thus looks like i got confused ?

Currently, staging is still Joomla 3.2. 3.3-dev holds the code of the current 3.3 beta.
This will change once 3.3 is released.
Then staging will become 3.2.x, 3.3-dev becomes the new staging and there will be a new branch 3.4-dev.
However I have no clue what happens with PRs targeted at staging. I guess they will change their target automatically to 3.2.x but we will see.

Everything clear?

Thanks Thomas, then this PR should not go to staging but to 3.3-dev for various good reasons listed above ;-)

We don't accept PRs against 3.3-dev branch. All PRs have to be done vs staging and then (for 3.3 ) they are picked manually by @mbabker

So even not being done against 3.3-dev that doesn't mean it cannot be merged into 3.3. I'll try to talk about this with Michael and if not replace the deprecated tag with 3.4.

Thanks!

ok, even more different than I thought, learning every day. As said it's 100% fine for 3.3.0.

We can take PRs to 3.3-dev just fine if that's what version the PR is
intended for. Or if it's against staging, I can just manually grab it like
you said. Either or works.

On Wed, Apr 9, 2014 at 9:36 AM, Roberto Segura notifications@github.comwrote:

We don't accept PRs against 3.3-dev branch. All PRs have to be done vs
staging and then (for 3.3 ) they are picked manually by @mbabkerhttps://github.com/mbabker

So even not being done against 3.3-dev that doesn't mean it cannot be
merged into 3.3. I'll try to talk about this with Michael and if not
replace the deprecated tag with 3.4.

Thanks!

Reply to this email directly or view it on GitHub#3421 (comment)
.

It would be nice if that form field is deprecated to put it in the legacy folder as well - try and start getting this stuff reorganized

It would be nice if that form field is deprecated to put it in the legacy folder as well - try and start getting this stuff reorganized

There is not really a point in moving that file to legacy. Legacy doesn't exactly mean "deprecated".
The deprecation tag and the log notice is all which is needed.
Also moving files could potentially break someones code if he load it manually (which is bad behaviour, but still). So we would have no gain but a low risk.

How to test:

• 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 code

• Apply 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!

@test
Works

How to test:

• 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 code

• Apply 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.}

I have updated this PR but I'm closing it and create a new PR against 3.5-dev branch.

Thanks everybody for your comments / tests!

Reopened so this is merged into staging branch.