diff --git a/public/main/admin/add_users_to_usergroup.php b/public/main/admin/add_users_to_usergroup.php index 98d6f2b04c0..1cac26726d9 100644 --- a/public/main/admin/add_users_to_usergroup.php +++ b/public/main/admin/add_users_to_usergroup.php @@ -16,11 +16,33 @@ $this_section = SECTION_PLATFORM_ADMIN; $id = isset($_REQUEST['id']) ? (int) $_REQUEST['id'] : 0; -$relation = isset($_REQUEST['relation']) && is_numeric($_REQUEST['relation']) ? (int) $_REQUEST['relation'] : null; + $usergroup = new UserGroupModel(); $groupInfo = $usergroup->get($id); $usergroup->protectScript($groupInfo); +if (empty($id)) { + api_not_allowed(true); +} + +// Resolve relation from GET and POST (POST wins on submit) +$relation = null; +if (isset($_GET['relation']) && is_numeric($_GET['relation'])) { + $relation = (int) $_GET['relation']; +} +if (isset($_POST['relation']) && '' !== $_POST['relation'] && is_numeric($_POST['relation'])) { + // When the form is submitted, trust the POST value + $relation = (int) $_POST['relation']; +} + +// Group type helpers +$isSocialGroup = isset($groupInfo['group_type']) && (int) $groupInfo['group_type'] === Usergroup::SOCIAL_CLASS; +$isClassGroup = isset($groupInfo['group_type']) && (int) $groupInfo['group_type'] === Usergroup::NORMAL_CLASS; + +// When dealing with social groups, we conceptually lock until a relation is chosen +// (UI is still usable, but submit will be blocked on client + server). +$lockAssignments = $isSocialGroup && (null === $relation); + // setting breadcrumbs $interbreadcrumb[] = ['url' => 'index.php', 'name' => get_lang('Administration')]; $interbreadcrumb[] = ['url' => 'usergroups.php', 'name' => get_lang('Classes')]; @@ -32,57 +54,59 @@ '; $form_sent = 0; @@ -90,46 +114,73 @@ function change_select(val) { $new_field_list = []; if (is_array($extra_field_list)) { foreach ($extra_field_list as $extra_field) { - //if is enabled to filter and is a "" field type if (1 == $extra_field[8] && 4 == $extra_field[2]) { $new_field_list[] = [ 'name' => $extra_field[3], - 'variable' => $extra_field[1], 'data' => $extra_field[9], + 'variable' => $extra_field[1], + 'data' => $extra_field[9], ]; } } } -if (empty($id)) { - api_not_allowed(true); -} - $first_letter_user = ''; if (isset($_POST['form_sent']) && $_POST['form_sent']) { - $form_sent = $_POST['form_sent']; - $elements_posted = isset($_POST['elements_in_name']) ? $_POST['elements_in_name'] : null; - $first_letter_user = $_POST['firstLetterUser']; + $form_sent = (int) $_POST['form_sent']; + $elements_posted = isset($_POST['elements_in_name']) ? $_POST['elements_in_name'] : []; + $first_letter_user = $_POST['firstLetterUser'] ?? ''; if (!is_array($elements_posted)) { - $elements_posted = []; + $elements_posted = ('' === (string) $elements_posted) + ? [] + : [(int) $elements_posted]; + } else { + $elements_posted = array_map('intval', $elements_posted); } - // If "social group" you need to select a role - if (1 == $groupInfo['group_type'] && empty($relation)) { - $_SESSION['usergroup_flash_message'] = get_lang('Select role'); - $_SESSION['usergroup_flash_type'] = 'warning'; - header('Location: '.api_get_self().'?id='.$id); - exit; + // Normalize relation depending on group type + // - Social group: role is required + // - Class group: always default reader role (0) + $normalizedRelation = GROUP_USER_PERMISSION_READER; + if ($isSocialGroup) { + // Ensure a role is selected for social groups + if (null === $relation || '' === $relation) { + $_SESSION['usergroup_flash_message'] = get_lang('Select role'); + $_SESSION['usergroup_flash_type'] = 'warning'; + header('Location: '.api_get_self().'?id='.$id); + exit; + } + + $normalizedRelation = (int) $relation; + + // make sure it is one of the known roles + $allowedRoles = [ + GROUP_USER_PERMISSION_ADMIN, + GROUP_USER_PERMISSION_READER, + GROUP_USER_PERMISSION_PENDING_INVITATION, + GROUP_USER_PERMISSION_MODERATOR, + GROUP_USER_PERMISSION_HRM, + ]; + + if (!in_array($normalizedRelation, $allowedRoles, true)) { + $_SESSION['usergroup_flash_message'] = get_lang('Invalid role selection'); + $_SESSION['usergroup_flash_type'] = 'warning'; + header('Location: '.api_get_self().'?id='.$id); + exit; + } + } else { + // For normal classes, silently force reader role (students) + $normalizedRelation = GROUP_USER_PERMISSION_READER; } - if (1 == $form_sent) { - // Added a parameter to send emails when registering a user + if (1 === $form_sent) { $usergroup->subscribe_users_to_usergroup( $id, $elements_posted, true, - $relation + (int) $normalizedRelation ); $_SESSION['usergroup_flash_message'] = get_lang('Update successful'); $_SESSION['usergroup_flash_type'] = 'success'; @@ -210,7 +261,16 @@ function change_select(val) { } $data = $usergroup->get($id); -$list_in = $usergroup->getUsersByUsergroupAndRelation($id, $relation); + +// Decide which relation to use to list existing users +$listRelationFilter = null; +if ($isSocialGroup) { + $listRelationFilter = $relation; +} else { + $listRelationFilter = GROUP_USER_PERMISSION_READER; +} + +$list_in = $usergroup->getUsersByUsergroupAndRelation($id, $listRelationFilter); $list_all = $usergroup->get_users_by_usergroup(); $order = ['lastname']; @@ -251,25 +311,25 @@ function change_select(val) { } // Avoid anonymous users - if (6 == $item['status']) { + if (ANONYMOUS == $item['status']) { continue; } $list_in_map = array_flip($list_in); if (isset($list_in_map[$item['id']])) { - $officialCode = !empty($item['official_code']) ? ' - ' . $item['official_code'] : null; + $officialCode = !empty($item['official_code']) ? ' - '.$item['official_code'] : null; $person_name = api_get_person_name( $item['firstname'], $item['lastname'] - ) . ' (' . $item['username'] . ') ' . $officialCode; + ).' ('.$item['username'].') '.$officialCode; $orderListByOfficialCode = api_get_setting('display.order_user_list_by_official_code'); if ('true' === $orderListByOfficialCode) { - $officialCode = !empty($item['official_code']) ? $item['official_code'] . ' - ' : '? - '; - $person_name = $officialCode . api_get_person_name( + $officialCode = !empty($item['official_code']) ? $item['official_code'].' - ' : '? - '; + $person_name = $officialCode.api_get_person_name( $item['firstname'], $item['lastname'] - ) . ' (' . $item['username'] . ') '; + ).' ('.$item['username'].') '; } $elements_in[$item['id']] = $person_name; @@ -306,17 +366,17 @@ function change_select(val) { $officialCode = !empty($item['official_code']) ? ' - '.$item['official_code'] : null; $person_name = api_get_person_name( - $item['firstname'], - $item['lastname'] - ).' ('.$item['username'].') '.$officialCode; + $item['firstname'], + $item['lastname'] + ).' ('.$item['username'].') '.$officialCode; $orderListByOfficialCode = api_get_setting('display.order_user_list_by_official_code'); if ('true' === $orderListByOfficialCode) { $officialCode = !empty($item['official_code']) ? $item['official_code'].' - ' : '? - '; $person_name = $officialCode.api_get_person_name( - $item['firstname'], - $item['lastname'] - ).' ('.$item['username'].') '; + $item['firstname'], + $item['lastname'] + ).' ('.$item['username'].') '; } if (!in_array($item['id'], $list_in)) { @@ -356,175 +416,197 @@ function change_select(val) { ); unset($_SESSION['usergroup_flash_message'], $_SESSION['usergroup_flash_type']); } + +// Build form action keeping relation +$action = api_get_self().'?id='.$id; +if (!empty($_GET['add'])) { + $action .= '&add=true'; +} +if (null !== $relation && '' !== $relation) { + $action .= '&relation='.(int) $relation; +} ?> -
- 0) { - echo '

'.get_lang('Filter by user').'

'; - foreach ($new_field_list as $new_field) { - echo $new_field['name']; - $varname = 'field_'.$new_field['variable']; - echo ' '; + echo ''; + foreach ($new_field['data'] as $option) { + $checked = ''; + if (isset($_POST[$varname])) { + if ($_POST[$varname] == $option[1]) { + $checked = 'selected="true"'; + } + } + echo ''; } + echo ''; + echo '  '; } - echo ''; + echo ''; + echo '

'; } - echo ''; - echo '  '; } - echo ''; - echo '

'; - } -} -echo Display::input('hidden', 'id', $id); -echo Display::input('hidden', 'form_sent', '1'); -echo Display::input('hidden', 'add_type', null); + echo Display::input('hidden', 'id', $id); + echo Display::input('hidden', 'form_sent', '1'); + echo Display::input('hidden', 'add_type', null); + ?> +
+
+ + + +

+ +

+ +

+ + +
+
+ + +
+ 'form-multiselect block w-full mt-1 border-gray-300 rounded-md shadow-sm focus:ring-indigo-500 focus:border-indigo-500 sm:text-sm', + 'multiple' => 'multiple', + 'id' => 'elements_not_in', + 'size' => '15', + ], + false + ); ?> + + +
-?> -
-
- - -

- - -
-
- - +
+ + +
+ +
+ + + + %s'), UserGroupModel::getRoleName($relation)); ?> + + + 'form-multiselect block w-full mt-1 border-gray-300 rounded-md shadow-sm focus:ring-indigo-500 focus:border-indigo-500 sm:text-sm', + 'multiple' => 'multiple', + 'id' => 'elements_in', + 'size' => '15', + ], + false + ); ?>
- 'form-multiselect block w-full mt-1 border-gray-300 rounded-md shadow-sm focus:ring-indigo-500 focus:border-indigo-500 sm:text-sm', - 'multiple' => 'multiple', - 'id' => 'elements_not_in', - 'size' => '15', - ], - false - ); ?> - -
-
- -
-
- - - - - - - %s'), UserGroupModel::getRoleName($relation)); ?> - - - 'form-multiselect block w-full mt-1 border-gray-300 rounded-md shadow-sm focus:ring-indigo-500 focus:border-indigo-500 sm:text-sm', - 'multiple' => 'multiple', - 'id' => 'elements_in', - 'size' => '15', - ], - false - ); ?> -
-
- -
- -
- - + function valide() { + // Client-side guard: for social groups, a relation is required before submit + if (isSocialGroup) { + const relationSelect = document.getElementById("relation"); + const relationValue = relationSelect ? relationSelect.value : ""; + + if (!relationValue) { + alert("Please select a relation type before saving users."); + return; + } + } + + const options = document.getElementById("elements_in").options; + for (let i = 0; i < options.length; i++) { + options[i].selected = true; + } + document.forms.formulaire.submit(); + } + ['usergroup_id = ? AND (relation_type = 0 OR relation_type IS NULL OR relation_type = "") ' => [$id]]]; + + if (0 === $relation) { + // Default reader role: include canonical (0) + legacy (2) + null / empty + $conditions = [ + 'where' => [ + 'usergroup_id = ? AND (relation_type = ? OR relation_type = 2 OR relation_type IS NULL OR relation_type = \'\') ' => [ + $id, + GROUP_USER_PERMISSION_READER, + ], + ], + ]; } else { - $conditions = ['where' => ['usergroup_id = ? AND relation_type = ?' => [$id, $relation]]]; + $conditions = [ + 'where' => [ + 'usergroup_id = ? AND relation_type = ?' => [$id, $relation], + ], + ]; } $results = Database::select( @@ -1224,15 +1239,28 @@ public function subscribe_users_to_usergroup( $delete_users_not_present_in_list = true, $relationType = 0 ) { - $current_list = $this->get_users_by_usergroup($usergroup_id); - $course_list = $this->get_courses_by_usergroup($usergroup_id); + // Normalize relation type + $relationType = (int) $relationType; + + // Detect group type to keep legacy behaviour for classic classes + $groupInfo = $this->get($usergroup_id); + $isSocialGroup = isset($groupInfo['group_type']) && (int) $groupInfo['group_type'] === Usergroup::SOCIAL_CLASS; + $isClassGroup = isset($groupInfo['group_type']) && (int) $groupInfo['group_type'] === Usergroup::NORMAL_CLASS; + + if ($isSocialGroup) { + $current_list = $this->getUsersByUsergroupAndRelation($usergroup_id, $relationType); + } else { + $current_list = $this->get_users_by_usergroup($usergroup_id); + } + + $course_list = $this->get_courses_by_usergroup($usergroup_id); $session_list = $this->get_sessions_by_usergroup($usergroup_id); $session_list = array_filter($session_list); - $relationType = (int) $relationType; $delete_items = []; - $new_items = []; + $new_items = []; + // Compute new users to add if (!empty($list)) { foreach ($list as $user_id) { if (!in_array($user_id, $current_list)) { @@ -1241,6 +1269,7 @@ public function subscribe_users_to_usergroup( } } + // Compute users to delete (present before, not present now) if (!empty($current_list)) { foreach ($current_list as $user_id) { if (!in_array($user_id, $list)) { @@ -1252,38 +1281,58 @@ public function subscribe_users_to_usergroup( // Deleting items if (!empty($delete_items) && $delete_users_not_present_in_list) { foreach ($delete_items as $user_id) { - // Removing courses + // Remove course subscriptions if (!empty($course_list)) { foreach ($course_list as $course_id) { $course_info = api_get_course_info_by_id($course_id); CourseManager::unsubscribe_user($user_id, $course_info['code']); } } - // Removing sessions + + // Remove session subscriptions if (!empty($session_list)) { foreach ($session_list as $session_id) { SessionManager::unsubscribe_user_from_session($session_id, $user_id); } } - if (empty($relationType)) { - Database::delete( - $this->usergroup_rel_user_table, - [ - 'usergroup_id = ? AND user_id = ? AND (relation_type = "0" OR relation_type IS NULL OR relation_type = "")' => [ - $usergroup_id, - $user_id, - ], - ] - ); + // Remove from usergroup_rel_user: + // - Social groups: delete exactly this role (even if it is 0 = reader). + // - Normal classes: keep legacy default behaviour. + if ($isSocialGroup) { + // Strict per-role deletion, with reader-role legacy tolerance + if ($relationType === 0) { + // Default reader role: treat legacy 2 as equivalent + Database::delete( + $this->usergroup_rel_user_table, + [ + 'usergroup_id = ? AND user_id = ? AND (relation_type = "0" OR relation_type = "2" OR relation_type IS NULL OR relation_type = "")' => [ + $usergroup_id, + $user_id, + ], + ] + ); + } else { + // Other roles: delete only this specific role + Database::delete( + $this->usergroup_rel_user_table, + [ + 'usergroup_id = ? AND user_id = ? AND relation_type = ?' => [ + $usergroup_id, + $user_id, + $relationType, + ], + ] + ); + } } else { + // Legacy: default/empty relation_type Database::delete( $this->usergroup_rel_user_table, [ - 'usergroup_id = ? AND user_id = ? AND relation_type = ?' => [ + 'usergroup_id = ? AND (relation_type = "0" OR relation_type = "2" OR relation_type IS NULL OR relation_type = "")' => [ $usergroup_id, $user_id, - $relationType, ], ] ); @@ -1293,7 +1342,7 @@ public function subscribe_users_to_usergroup( // Adding new relationships if (!empty($new_items)) { - // Adding sessions + // Add sessions if (!empty($session_list)) { foreach ($session_list as $session_id) { SessionManager::subscribeUsersToSession($session_id, $new_items, null, false); @@ -1301,17 +1350,20 @@ public function subscribe_users_to_usergroup( } foreach ($new_items as $user_id) { - // Adding courses. + // Add courses if (!empty($course_list)) { foreach ($course_list as $course_id) { CourseManager::subscribeUser($user_id, $course_id); } } + + // Persist relation in usergroup_rel_user $params = [ - 'user_id' => $user_id, + 'user_id' => $user_id, 'usergroup_id' => $usergroup_id, - 'relation_type' => $relationType, + 'relation_type'=> $relationType, ]; + Database::insert($this->usergroup_rel_user_table, $params); } } @@ -2057,8 +2109,8 @@ public function is_group_member($group_id, $user_id = 0) * @param int $user_id * @param int $group_id * - * @return int 0 if there are not relationship otherwise returns the user group - * */ + * @return int 0 if there is no relationship; otherwise the normalized role + */ public function get_user_group_role($user_id, $group_id) { $table_group_rel_user = $this->usergroup_rel_user_table; @@ -2071,11 +2123,18 @@ public function get_user_group_role($user_id, $group_id) FROM $table_group_rel_user WHERE usergroup_id = $group_id AND - user_id = $user_id "; + user_id = $user_id"; $result = Database::query($sql); if (Database::num_rows($result) > 0) { $row = Database::fetch_assoc($result); - $return_value = $row['relation_type']; + $relationType = (int) $row['relation_type']; + + // Normalize legacy reader (2) to the canonical reader role + if (2 === $relationType) { + $relationType = GROUP_USER_PERMISSION_READER; + } + + $return_value = $relationType; } } @@ -2201,23 +2260,52 @@ public function delete_user_rel_group($userId, $groupId) */ public function add_user_to_group($user_id, $group_id, $relation_type = GROUP_USER_PERMISSION_READER) { - $table_url_rel_group = $this->usergroup_rel_user_table; - if (!empty($user_id) && !empty($group_id)) { - $role = $this->get_user_group_role($user_id, $group_id); - - if (0 == $role) { - $sql = "INSERT INTO $table_url_rel_group - SET - user_id = ".intval($user_id).", - usergroup_id = ".intval($group_id).", - relation_type = ".intval($relation_type); - Database::query($sql); - } elseif (GROUP_USER_PERMISSION_PENDING_INVITATION == $role) { - //if somebody already invited me I can be added - self::update_user_role($user_id, $group_id, GROUP_USER_PERMISSION_READER); + $table_rel = $this->usergroup_rel_user_table; + $user_id = (int) $user_id; + $group_id = (int) $group_id; + $relation_type = (int) $relation_type; + + if (empty($user_id) || empty($group_id)) { + return false; + } + + // Normalize relation type for classes vs social groups + $groupInfo = $this->get($group_id); // returns ['group_type' => ...] among others + if (!empty($groupInfo) && isset($groupInfo['group_type'])) { + if ((int) $groupInfo['group_type'] === self::NORMAL_CLASS) { + // For class memberships, we want relation_type = 0 as the canonical "reader" + if ($relation_type === GROUP_USER_PERMISSION_READER) { + $relation_type = 0; + } } } + // Current role (raw value from usergroup_rel_user) + $role = $this->get_user_group_role($user_id, $group_id); + + if (0 === $role) { + // No relationship yet → insert normalized relation_type + $sql = "INSERT INTO $table_rel + SET + user_id = $user_id, + usergroup_id = $group_id, + relation_type = $relation_type"; + Database::query($sql); + } elseif (GROUP_USER_PERMISSION_PENDING_INVITATION === $role) { + // If somebody already invited the user, upgrade to final role + self::update_user_role( + $user_id, + $group_id, + $relation_type ?: GROUP_USER_PERMISSION_READER + ); + } elseif ( + // Backward-compatibility: old "reader = 2" for classes → normalize to 0 + GROUP_USER_PERMISSION_READER === $role + && $relation_type === 0 + ) { + self::update_user_role($user_id, $group_id, 0); + } + return true; }