Skip to content

[Grenoble] Améliorer le lien de création de groupe à partir d'une classe, refs #5774 #1709 #1710 #6295

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

[Grenoble] Améliorer le lien de création de groupe à partir d'une classe, refs #5774 #1709 #1710 #6295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3bbeba3
Group: Allow to create linked group to usergroup
eristich Mar 11, 2025
bc0b76f
Language: Add new terms for groups
eristich Apr 29, 2025
d573589
Minor: add phpdoc block and type hinting
eristich Apr 29, 2025
27fa710
Group: revert new group category options for future PR
eristich May 15, 2025
54babdc
Merge branch 'master' into eristich/linked-group-to-usergroup-ref-5774
eristich May 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion public/main/group/group.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,20 @@
if ('true' === api_get_setting('allow_group_categories')) {
if (empty($categories)) {
$defaultCategoryId = GroupManager::create_category(
get_lang('Default groups')
get_lang('Default groups'),
'',
0,
0,
0,
0,
0,
0,
0,
0,
0,
1,
0,
0
);
$defaultCategory = GroupManager::get_category($defaultCategoryId);
$categories = [$defaultCategory];
Expand Down Expand Up @@ -267,6 +280,11 @@
href="group_category.php?'.api_get_cidreq().'&id='.$categoryId.'" title="'.get_lang('Edit').'">'.
Display::getMdiIcon('pencil', 'ch-tool-icon', null, ICON_SIZE_SMALL, get_lang('Edit this category')).'</a>';

// Add group
$actions .= ' <a
href="group_creation.php?'.api_get_cidreq().'&category_id='.$categoryId.'">'.
Display::getMdiIcon(ActionIcon::SUBSCRIBE_GROUP_USERS_TO_RESOURCE, 'ch-tool-icon', null, ICON_SIZE_SMALL, get_lang('Create new group(s)')).'</a>';

// Delete
$actions .= Display::url(
Display::getMdiIcon(ActionIcon::DELETE, count($categories) == 1 ? 'ch-tool-icon-disabled' : 'ch-tool-icon', null, ICON_SIZE_SMALL, get_lang('Delete')),
Expand Down
8 changes: 7 additions & 1 deletion public/main/group/group_category.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ function check_groups_per_user($value)
'announcements_state' => GroupManager::TOOL_PRIVATE,
'forum_state' => GroupManager::TOOL_PRIVATE,
'max_student' => 0,
'document_access' => 0,
'document_access' => 0
];
}

Expand Down Expand Up @@ -452,6 +452,12 @@ function check_groups_per_user($value)
$defaults['max_member_no_limit'] = 1;
$defaults['max_member'] = $defaults['max_student'];
}
/*
if (api_get_setting('allow_group_categories')) {
$defaults['id'] = $_GET['id'];
}
*/

$form->setDefaults($defaults);
$form->display();

Expand Down
106 changes: 78 additions & 28 deletions public/main/group/group_creation.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,18 @@

break;
case 'create_class_groups':
GroupManager::create_class_groups($_POST['group_category']);
$classIds = [];
foreach (array_keys($_POST) as $key) {
if (strpos($key, 'checkbox_class_id_') !== false) {
$classId = str_replace('checkbox_class_id_', '', $key);
$classIds[] = (int)$classId;
}
}
if (isset($_POST['is_consistent_link'])) {
GroupManager::create_usergroup_consistent_groups($_POST['group_category'], $classIds);
} else {
GroupManager::create_class_groups($_POST['group_category'], $classIds);
}
Display::addFlash(Display::return_message(get_lang('group(s) has (have) been added')));
header('Location: '.$currentUrl);
exit;
Expand Down Expand Up @@ -181,13 +192,17 @@ function copy_value(key) {
$group_el = [];
$group_el[] = $form->createElement('static', null, null, ' ');
if ($allowGroupCategories) {
$group_el[] = $form->createElement(
'checkbox',
'same_category',
null,
get_lang('same for all'),
['onclick' => "javascript: switch_state('category');"]
);
if (!isset($_GET['category_id'])) {
$group_el[] = $form->createElement(
'checkbox',
'same_category',
null,
get_lang('same for all'),
['onclick' => "javascript: switch_state('category');"]
);
} else {
$group_el[] = $form->createElement('static', null, null, ' ');
}
}
$group_el[] = $form->createElement(
'checkbox',
Expand All @@ -203,13 +218,29 @@ function copy_value(key) {
$group_el = [];
$group_el[] = $form->createElement('text', 'group_'.$group_number.'_name');
if ($allowGroupCategories) {
$group_el[] = $form->createElement(
'select',
'group_'.$group_number.'_category',
null,
$categories,
['id' => 'category_'.$group_number]
);
if(isset($_GET['category_id'])) {
$group_el[] = $form->createElement(
'select',
'group_' . $group_number . '_category',
null,
$categories,
[
'id' => 'category_' . $group_number,
'disabled' => 'true',
'style' => 'background-color: #f0f0f0;'
]
);
$group_el[] = $form->createElement('hidden', 'group_'.$group_number.'_category', $_GET['category_id']);
$defaults['group_'.$group_number.'_category'] = $_GET['category_id'];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$_GET['category_id'] should be filtered (because we know it's an integer) with (int) $_GET['category_id']

When a $categoryId comes from an integer field in the database, you know it's an integer so you don't have to filter it, but anything from $_GET, $_POST, $_REQUEST, even $_SESSION and other superglobals has to be filtered before inserting into the database or an URL or anything on a web page.

} else {
$group_el[] = $form->createElement(
'select',
'group_' . $group_number . '_category',
null,
$categories,
['id' => 'category_' . $group_number]
);
}
} else {
$group_el[] = $form->createElement('hidden', 'group_'.$group_number.'_category', 0);

Expand Down Expand Up @@ -247,7 +278,7 @@ function copy_value(key) {
/*
* Show form to generate new groups
*/
$create_groups_form = new FormValidator('create_groups', 'post', api_get_self().'?'.api_get_cidreq());
$create_groups_form = new FormValidator('create_groups', 'post', api_get_self().'?'.api_get_cidreq().(isset($_GET['category_id']) ? '&category_id='.$_GET['category_id'] : ''));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When using $_GET[something], always filter it before using it as part of a URL. Here, because we know it's an integer (because it ends with _id), you can immediately and very cheaply filter it with (int) $_GET['category_id'] directly inside the concatenation.

$create_groups_form->addElement('header', $nameTools);
$create_groups_form->addText('number_of_groups', get_lang('Number of groups to create'), null, ['value' => '1']);
$create_groups_form->addButton('submit', get_lang('Proceed to create group(s)'), 'plus', 'primary');
Expand Down Expand Up @@ -306,16 +337,7 @@ function copy_value(key) {
$obj = new UserGroupModel();
$classes = $obj->getUserGroupInCourse($options);
if (count($classes) > 0) {
$description = '<p>'.get_lang('Using this option, you can create groups based on the classes subscribed to your course.').'</p>';
$description .= '<ul>';
foreach ($classes as $index => $class) {
$number_of_users = count($obj->get_users_by_usergroup($class['id']));
$description .= '<li>';
$description .= $class['name'];
$description .= ' ('.$number_of_users.' '.get_lang('Users').')';
$description .= '</li>';
}
$description .= '</ul>';
$description = '<p>'.get_lang('Using this option, you can create groups based on the classes subscribed to your course.').'</p><br>';

$classForm = new FormValidator(
'create_class_groups_form',
Expand All @@ -325,14 +347,42 @@ function copy_value(key) {
$classForm->addHeader(get_lang('Groups from classes'));

$classForm->addHtml($description);

$classGroup = [];

foreach ($classes as $index => $class) {
$number_of_users = count($obj->get_users_by_usergroup($class['id']));
// $classForm->addCheckBox('checkbox_class_id_'.$class['id'], $class['title'] . ' ('.$number_of_users.' '.get_lang('Users').')');
$classGroup[] = $classForm->createElement('checkbox', 'checkbox_class_id_'.$class['id'], null, $class['title'] . ' ('.$number_of_users.' '.get_lang('Users').')');
}

$classForm->addGroup(
$classGroup,
'',
null,
null,
false
);

$classForm->addElement('hidden', 'action');
if ($allowGroupCategories) {
$classForm->addSelect('group_category', null, $categories);
if (isset($_GET['category_id'])) {
$classForm->addElement('hidden', 'group_category', $_GET['category_id']);
} else {
$classForm->addSelect('group_category', null, $categories);
}
} else {
$classForm->addElement('hidden', 'group_category');
}

$classForm->addHtml('<div style="height: 1px; width: 100%; background-color: #e6e6e6; border-radius: 8px;" /><br>');

$classForm->addCheckBox('is_consistent_link', null, get_lang('Link classes to created groups ?'),
['title' => get_lang('Info message about deactivation of consistent link')]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An info message should not start with "Info message about". It should be directly the message to the user 😄

);
$classForm->addHtml('<p class="alert alert-info">'.get_lang('Info message about deactivation of consistent link').'</p>');

$classForm->addButtonSave(get_lang('Validate'));
$defaults['group_category'] = GroupManager::DEFAULT_GROUP_CATEGORY;
$defaults['action'] = 'create_class_groups';
$classForm->setDefaults($defaults);
$classForm->display();
Expand Down
8 changes: 8 additions & 0 deletions public/main/group/group_space.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,14 @@ class="btn btn--plain" href="'.api_get_self().'?selfUnReg=1"
}
}

// add edit tool
if (true) {
$actions_array[] = [
'url' => 'settings.php?'.api_get_cidreq(true, false).'&gid='.$group_id,
'content' => Display::getMdiIcon(ActionIcon::EDIT, 'ch-tool-icon', null, ICON_SIZE_MEDIUM, get_lang('Edit')),
];
}

if (GroupManager::TOOL_NOT_AVAILABLE != $groupEntity->getDocState()) {
$params = [
'toolName' => 'document',
Expand Down
26 changes: 24 additions & 2 deletions public/main/group/member_settings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
/* For licensing terms, see /license.txt */

require_once __DIR__.'/../inc/global.inc.php';

use Chamilo\CoreBundle\Framework\Container;

$this_section = SECTION_COURSES;
$current_course_tool = TOOL_GROUP;

Expand All @@ -14,7 +17,7 @@
$groupEntity = api_get_group_entity($group_id);

$nameTools = get_lang('Edit this group');
$interbreadcrumb[] = ['url' => 'group.php', 'name' => get_lang('Groups')];
$interbreadcrumb[] = ['url' => 'group.php?'.api_get_cidreq(), 'name' => get_lang('Groups')];
$interbreadcrumb[] = ['url' => 'group_space.php?'.api_get_cidreq(), 'name' => $groupEntity->getTitle()];

$is_group_member = GroupManager::isTutorOfGroup(api_get_user_id(), $groupEntity);
Expand Down Expand Up @@ -227,6 +230,25 @@ function check_group_members($value)

$form->setDefaults($defaults);
echo GroupManager::getSettingBar('member');
$form->display();

// check if group has a CGroupRelUsergroup
$courseInfo = api_get_course_info_by_id(api_get_course_int_id());

if (GroupManager::is_group_linked_to_usergroup($groupEntity)) {

echo '<div class="alert alert-info">Ce groupe est lié à la classe ' .$courseInfo['title'].'. La liste de ses membres dépend des membres de la classe et ne peut-être modifiée.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is in French. Has to be translated to English, and because it includes a variable ($courseInfo['title']), it should be a string with a %s` in it, and the get_lang() call should be called inside a sprintf() call. Look for sprintf in the code, I'm sure you'll find many examples.

Allez dans les paramètres du groupe pour rompre ce lien si vous souhaitez ajouter ou ôter des membres de la classe.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

French

</div>';

echo '<h3>Membres du groupe</h3>';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

French

$memberInfos = GroupManager::get_subscribed_users($groupEntity);
echo '<ul>';
foreach ($memberInfos as $memberInfo) {
echo '<li>'.$memberInfo['firstname'].' '.$memberInfo['lastname'].' ('.$memberInfo['username'].')'.'</li>';
}
echo '</ul>';
} else {
$form->display();
}

Display::display_footer();
22 changes: 19 additions & 3 deletions public/main/group/settings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@
$groupRepo = Container::getGroupRepository();
/** @var CGroup $groupEntity */
$groupEntity = $groupRepo->find($group_id);
$linkedCategory = GroupManager::get_category_from_group($group_id);

if (isset($_GET['remove_consistent_link'])) {
GroupManager::remove_group_consistent_link($groupEntity);
Display::addFlash(Display::return_message(get_lang('Group is no longer linked to the course'), 'normal'));
header('Location: group.php?'.api_get_cidreq(true, false));
}

if (null === $groupEntity) {
api_not_allowed(true);
Expand All @@ -35,9 +42,7 @@
$interbreadcrumb[] = ['url' => 'group_space.php?'.api_get_cidreq(), 'name' => $groupEntity->getTitle()];
$groupMember = GroupManager::isTutorOfGroup(api_get_user_id(), $groupEntity);

if (!$groupMember && !api_is_allowed_to_edit(false, true)) {
api_not_allowed(true);
}
$courseInfo = api_get_course_info_by_id(api_get_course_int_id());

// Build form
$form = new FormValidator('group_edit', 'post', api_get_self().'?'.api_get_cidreq());
Expand All @@ -55,6 +60,17 @@
// Group name
$form->addElement('text', 'name', get_lang('Group name'));

if (!$groupMember && !api_is_allowed_to_edit(false, true)) {
api_not_allowed(true);
}

// Message for group rel usergroup
if (GroupManager::is_group_linked_to_usergroup($groupEntity)) {
$form->addHtml('<div class="alert alert-info">'.get_lang('Warning message to warn that the user cannot modify members of linked group').'<br>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A warning message should be the message to the final user, not start with "Warning message to warn...".

<a href="settings.php?'.api_get_cidreq().'&remove_consistent_link=true" class="btn p-button-sm p-button p-mr-2 mt-2 pointer">'.get_lang('Remove the group link with the class').
'</a></div>');
}

if ('true' === api_get_setting('allow_group_categories')) {
$groupCategories = GroupManager::get_categories();
$categoryList = [];
Expand Down
Loading
Loading