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

eristich · 2025-05-15T08:15:59Z

No description provided.

codeclimate · 2025-05-15T08:21:47Z

public/main/inc/lib/groupmanager.lib.php

+     * @return array
+     * @throws \Doctrine\DBAL\DBALException
+     */
+    public static function create_usergroup_consistent_groups(int $category_id = 0, array $classIds = []): array


Method create_usergroup_consistent_groups has 63 lines of code (exceeds 25 allowed). Consider refactoring.

codeclimate · 2025-05-15T08:21:48Z

public/main/inc/lib/groupmanager.lib.php

+     * @return array
+     * @throws \Doctrine\DBAL\DBALException
+     */
+    public static function create_usergroup_consistent_groups(int $category_id = 0, array $classIds = []): array


Function create_usergroup_consistent_groups has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

codeclimate · 2025-05-15T08:21:48Z

public/main/inc/lib/groupmanager.lib.php

     *
     * @return array
     */
-    public static function create_class_groups($categoryId)
+    public static function create_class_groups(int $categoryId, array $classIds = []): array


Function create_class_groups has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring.

codeclimate · 2025-05-15T08:21:49Z

Code Climate has analyzed commit 54babdc and detected 3 issues on this pull request.

Here's the issue category breakdown:

Category	Count
Complexity	3

View more on Code Climate.

ywarnier

Several (small) things to fix before this can be merged.

ywarnier · 2025-06-20T16:44:34Z

public/main/group/group_creation.php

@@ -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'] : ''));


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.

ywarnier · 2025-06-20T16:45:23Z

public/main/group/group_creation.php

+                        ]
+                    );
+                    $group_el[] = $form->createElement('hidden', 'group_'.$group_number.'_category', $_GET['category_id']);
+                    $defaults['group_'.$group_number.'_category'] = $_GET['category_id'];


$_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.

ywarnier · 2025-06-20T16:50:41Z

public/main/group/member_settings.php

+
+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.


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.

ywarnier · 2025-06-20T16:50:49Z

public/main/group/member_settings.php

+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.
+    Allez dans les paramètres du groupe pour rompre ce lien si vous souhaitez ajouter ou ôter des membres de la classe.


ywarnier · 2025-06-20T16:50:55Z

public/main/group/member_settings.php

+    Allez dans les paramètres du groupe pour rompre ce lien si vous souhaitez ajouter ou ôter des membres de la classe.
+    </div>';
+
+    echo '<h3>Membres du groupe</h3>';


ywarnier · 2025-06-20T16:53:14Z

public/main/group/settings.php

+
+// 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>


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

ywarnier · 2025-06-20T16:54:48Z

public/main/inc/lib/groupmanager.lib.php

@@ -2317,6 +2502,9 @@ class = "btn btn--plain"
        $table->set_header($column++, get_lang('Groups'));
        $table->set_header($column++, get_lang('Group tutor'));
        $table->set_header($column++, get_lang('Registered'), false);
+        if (api_get_setting('allow_group_categories') === 'true') {
+            $table->set_header($column++, get_lang('Classe liée'), false);


Text in French. Should be "Linked class".

ywarnier · 2025-06-20T16:56:47Z

translations/messages.en.po

+
+msgid "Group is no longer linked to the course"
+msgstr "Group is no longer linked to the course"
+


This has the wrong text msgid for those messages like "Info message about ..." and "Warning message to warn..." (should be updated together with the code).
It is also missing the various strings that are currently in French in the code.

Also, try to always add your new strings at the end of the file. No matter the group of strings or any alphabetical order.

ywarnier · 2025-06-20T16:57:09Z

translations/messages.fr.po

+
+msgid "Group is no longer linked to the course"
+msgstr "Le groupe n'esp plus lié au cours"
+


Same notes as for the English file

ywarnier · 2025-06-20T16:57:20Z

translations/messages.pot

+
+msgid "Group is no longer linked to the course"
+msgstr ""
+


Same notes as for the English file

eristich added 5 commits March 11, 2025 16:49

Group: Allow to create linked group to usergroup

3bbeba3

Language: Add new terms for groups

bc0b76f

Minor: add phpdoc block and type hinting

d573589

Group: revert new group category options for future PR

27fa710

Merge branch 'master' into eristich/linked-group-to-usergroup-ref-5774

54babdc

codeclimate bot reviewed May 15, 2025

View reviewed changes

ywarnier requested changes Jun 20, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

Uh oh!

eristich commented May 15, 2025

Uh oh!

codeclimate bot May 15, 2025

Uh oh!

codeclimate bot May 15, 2025

Uh oh!

codeclimate bot May 15, 2025

Uh oh!

codeclimate bot commented May 15, 2025

Uh oh!

ywarnier left a comment

Uh oh!

ywarnier Jun 20, 2025

Uh oh!

ywarnier Jun 20, 2025

Uh oh!

ywarnier Jun 20, 2025

Uh oh!

ywarnier Jun 20, 2025

Uh oh!

ywarnier Jun 20, 2025

Uh oh!

ywarnier Jun 20, 2025

Uh oh!

ywarnier Jun 20, 2025

Uh oh!

ywarnier Jun 20, 2025

Uh oh!

ywarnier Jun 20, 2025

Uh oh!

ywarnier Jun 20, 2025

Uh oh!

Uh oh!


		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.


		msgid "Group is no longer linked to the course"
		msgstr "Group is no longer linked to the course"


		msgid "Group is no longer linked to the course"
		msgstr "Le groupe n'esp plus lié au cours"

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

Are you sure you want to change the base?

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

Uh oh!

Conversation

eristich commented May 15, 2025

Uh oh!

codeclimate bot May 15, 2025

Choose a reason for hiding this comment

Uh oh!

codeclimate bot May 15, 2025

Choose a reason for hiding this comment

Uh oh!

codeclimate bot May 15, 2025

Choose a reason for hiding this comment

Uh oh!

codeclimate bot commented May 15, 2025

Uh oh!

ywarnier left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!