-
-
Notifications
You must be signed in to change notification settings - Fork 61
fix for Admin users search #1554
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR renames all instances of the “username” parameter to “uname” to align with the database field, and updates related form inputs, validation checks, and search query parameters.
- Updated form fields and request handlers to use ‘uname’ instead of ‘username’
- Adjusted validation and error messages to pull from Request::getString('uname')
- Revised user search logic and URL parameters for the admin users listing
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| htdocs/modules/system/admin/users/users/users.php | Changed form input name from ‘username’ to ‘uname’ |
| htdocs/modules/system/admin/users/main.php | Replaced request keys, validation, and search filters to use ‘uname’ and adjusted related comments/strings |
Comments suppressed due to low confidence (1)
htdocs/modules/system/admin/users/main.php:512
- [nitpick] The URL parameters mix
unameanduser_uname_match. To improve clarity, consider renaminguser_uname_matchtouname_matchso the prefix is consistent.
$requete_pagenav .= '&user_uname_match=' . htmlspecialchars($user_uname_match, ENT_QUOTES | ENT_HTML5);
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…XoopsCore25 into feature/fix_admin_users
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR standardizes the user identifier from “username” to “uname” across the admin user form and search handlers to match the database column.
- Renamed form fields and request parameters from “username”/“user_uname” to “uname”
- Switched from
$myts->addSlashes()to$xoopsDB->escape()in search criteria - Refactored group parameter handling and updated various comment strings
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| htdocs/modules/system/admin/users/users.php | Changed the form input name for nickname from 'username' to 'uname' |
| htdocs/modules/system/admin/users/main.php | Updated request keys, criteria building, escaping, and comments to use 'uname' and xoopsDB->escape() |
Comments suppressed due to low confidence (1)
htdocs/modules/system/admin/users/main.php:829
- Retrieving
selgroupswithgetStringwill always return a string, but later code treats it as an array. Consider usingRequest::getArray('selgroups', [])for consistent input handling.
$selgroups = Request::getString('selgroups'); //TODO should it be an array?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR renames the “username” form and request parameters to “uname” for database consistency and adjusts related defaults and escaping in the admin user search and edit flows.
- Renamed form element and request keys from
usernametouname - Defaulted
attachsig/user_viewemailto0instead ofnull - Switched from
addSlashesto$xoopsDB->escapeand updated search query and result strings to English
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| htdocs/modules/system/admin/users/users.php | Change form field name from 'username' to 'uname' |
| htdocs/modules/system/admin/users/main.php | Update all username references to uname, default flag values, SQL escaping, and search query building |
Comments suppressed due to low confidence (3)
htdocs/modules/system/admin/users/main.php:473
- [nitpick] The variable
$groupIduses camelCase, while the surrounding code follows snake_case (e.g.,$user_uname). Consider renaming it to$group_idfor consistency.
$groupId = Request::getInt('group', 0, 'GET');
htdocs/modules/system/admin/users/users.php:124
- [nitpick] Since the form field name changed from
usernametouname, add or update automated tests to cover both the form rendering and submission to ensure the backend correctly handles the newunameparameter.
$form->addElement(new XoopsFormText(_AM_SYSTEM_USERS_NICKNAME, 'uname', 25, 25, $uname_value), true);
htdocs/modules/system/admin/users/main.php:647
- It looks like the
$requete_pagenavupdate for theuser_msnmfilter was removed by mistake when updating to English. You should re-add the corresponding&user_msnm=parameter to$requete_pagenavto ensure pagination retains this filter.
$requete_search .= 'msn: ' . $user_msnm . ' and user_msnm_match=' . $user_msnm_match . '<br>';
montuy337513
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading this, it seems correct to me.
replace 'location' with 'occupation'
changing "username" to "uname" to make consistent with the name in the database