[#47366] - [6.2] Default new user's assigned group to active group filter
- Ready to Commit
- Medium
- Build: 6.1-dev
- # 47366
- Diff
- krishnagandhicode:feature-default-usergrp
User tests: Successful: Unsuccessful:
Pull Request resolves #47357
- I read the Generative AI policy and my contribution is either not created with the help of AI or is compatible with the policy and GNU/GPL 2 or later.
Summary of Changes
Updated the getAssignedGroups method in the userModel to check for an active list fillter before it fallls back to the default group. I added logic to get the group_id from the session state using the com_users.users.default.filter path so that if a filter is active , it become the preselected group for the new user.
Testing Instructions
go to users -> manage
Use the filter dropdown to select a group (for eg Manager).
now create a new user
click Assigned User Groups
Actual result BEFORE applying this Pull Request
the form ignores the active filter and always defaults to checking the Registered group.
Expected result AFTER applying this Pull Request
The group you choose to filtered by in the list (e.g Manager) is already checked for you (as default).
Link to documentations
Please select:
-
Documentation link for guide.joomla.org:
-
No documentation changes for guide.joomla.org needed
-
Pull Request link for manual.joomla.org:
-
No documentation changes for manual.joomla.org needed
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_users |
| Labels |
Added:
PR-6.1-dev
|
||
I have tested this item ✅ successfully on d391dce
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47366.
Restored test results
| Labels |
Added:
Feature
|
||
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47366.
This pull request has been automatically rebased to 6.2-dev.
| Title |
|
||||||
Look at the changes in the PR, I see few problems here:
- You always return user group selected in the user group filter for new user. Imagine you are creating a new user, select some user groups for that new user but there is error with saving (because you enter an existing username or email of an existing user for example), the selected user groups will be lost when form reloaded.
- This change a very long time behavior. Imagine super user user group is selected in the filter before admin press New button in the toolbar to create a new user. A long time admin user will think that that new user will be added to the default user group (usually Registered) and do not change it while the new user will actually be added as a Super User which could be dangerous. Note that the user groups selection for new account is displayed in a separate tab and admin might not know that he is added as a Super User. @brianteeman Do you think it is OK with this change, or I'm worry too much?
- There code is also not very nice. But we will discuss further if we really want to go with this change.
@joomdonation your concern is valid
Thanks @brianteeman
should also point out that this testing across multiple components this is not the only filter that doesnt get applied when you click on new
should also point out that testing this across multiple components this is not the only filter that doesnt get applied when you click on new
| Labels |
Added:
RTC
RMDQ
PR-6.2-dev
Removed: PR-6.1-dev |
||
Thanks @joomdonation and @brianteeman for the feedback.
@joomdonation I fixed the data loss issue - form values are now checked first, so selected groups are preserved on savee errors.
For the ux, security concerns, this feature was accepted by maintainers in [discussion #47412] maybe we gotta discuss it there ?
@krishnagandhicode Thanks for pointing out the discussion. Maintainers decided to accept this change, so we can continue. I checked the code with your last commit and it looks good to me now. @exlemor @adarshdubey03 Could you please re-test? Thanks
Seems there is a failing test? @krishnagandhicode? is it fixable?
Seems there is a failing test? @krishnagandhicode? is it fixable?
@exlemor the failing test is unrelated to this PR - Looks like a CI run issue. @richard67 could you re trigger the tests when you get a chance? (Thanks in advance)
PR is ready for Human testing.
I have tested this item ✅ successfully on d391dce
I have successfully tested this! Thanks @krishnagandhicode!
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47366.