Edition des groupes: amélioration traitement erreurs, empeche edition des parcours

app/api/partitions.py

@@ -323,9 +323,10 @@ def group_edit(group_id: int):
     data = request.get_json(force=True)  # may raise 400 Bad Request
     group_name = data.get("group_name")
     if group_name is not None:
+        group_name = group_name.strip()
         if not GroupDescr.check_name(group.partition, group_name, existing=True):
             return json_error(404, "invalid group_name")
-        group.group_name = group_name.strip()
+        group.group_name = group_name
         db.session.add(group)
         db.session.commit()
         log(f"modified {group}")

app/scodoc/sco_formsemestre.py

@@ -256,7 +256,7 @@ def do_formsemestre_create(args, silent=False):
         redirect=0,
         numero=1000000,  # à la fin
     )
-    _group_id = sco_groups.create_group(partition_id, default=True)
+    _ = sco_groups.create_group(partition_id, default=True)
 
     # news
     if "titre" not in args:

app/scodoc/sco_groups.py

@@ -93,7 +93,7 @@ groupEditor = ndb.EditableTable(
 group_list = groupEditor.list
 
 
-def get_group(group_id: int):
+def get_group(group_id: int) -> dict:
     """Returns group object, with partition"""
     r = ndb.SimpleDictFetch(
         """SELECT gd.id AS group_id, gd.*, p.id AS partition_id, p.*
@@ -238,8 +238,8 @@ def get_default_group(formsemestre_id, fix_if_missing=False):
             partition_id = partition_create(
                 formsemestre_id, default=True, redirect=False
             )
-        group_id = create_group(partition_id, default=True)
+        group = create_group(partition_id, default=True)
-        return group_id
+        return group.id
     # debug check
     if len(r) != 1:
         raise ScoException("invalid group structure for %s" % formsemestre_id)
@@ -292,34 +292,6 @@ def get_group_members(group_id, etat=None):
     return r
 
 
-def check_group_name(group_name, partition, raiser=False):
-    """If groupe name exists in partition : if raiser -> Raise ScoValueError else-> return true"""
-    exists = group_name in [g["group_name"] for g in get_partition_groups(partition)]
-    if exists:
-        if raiser:
-            raise ScoValueError("Le nom de groupe existe déjà dans la partition")
-        else:
-            return True
-    return False
-
-
-# obsolete:  sco_groups_view.DisplayedGroupsInfos
-# def get_groups_members(group_ids, etat=None):
-#     """Liste les étudiants d'une liste de groupes
-#     chaque étudiant n'apparait qu'une seule fois dans le résultat.
-#     La liste est triée par nom / prenom
-#     """
-#     D = {} # { etudid : etud  }
-#     for group_id in group_ids:
-#         members = get_group_members(group_id, etat=etat)
-#         for m in members:
-#             D[m['etudid']] = m
-#     r = D.values()
-#     r.sort(key=operator.itemgetter('nom_disp', 'prenom')) # tri selon nom_usuel ou nom
-
-#     return r
-
-
 def get_group_infos(group_id, etat=None):  # was _getlisteetud
     """legacy code: used by group_list and trombino"""
     from app.scodoc import sco_formsemestre
@@ -565,7 +537,7 @@ def XMLgetGroupsInPartition(partition_id):  # was XMLgetGroupesTD
     """
     Deprecated: use group_list
     Liste des étudiants dans chaque groupe de cette partition.
-    <group partition_id="" partition_name="" group_id="" group_name="">
+    <group partition_id="" partition_name="" group_id="" group_name="" groups_editable="">
     <etud etuid="" sexe="" nom="" prenom="" civilite="" origin=""/>
     </group>
     <group ...>
@@ -588,6 +560,7 @@ def XMLgetGroupsInPartition(partition_id):  # was XMLgetGroupesTD
             "group",
             partition_id=str(partition_id),
             partition_name=partition["partition_name"],
+            groups_editable=str(int(partition["groups_editable"])),
             group_id=str(group["group_id"]),
             group_name=group["group_name"],
         )
@@ -614,6 +587,7 @@ def XMLgetGroupsInPartition(partition_id):  # was XMLgetGroupesTD
             "group",
             partition_id=str(partition_id),
             partition_name=partition["partition_name"],
+            groups_editable=str(int(partition["groups_editable"])),
             group_id="_none_",
             group_name="",
         )
@@ -744,7 +718,7 @@ def setGroups(
     groupsToCreate="",  # name and members of new groups
     groupsToDelete="",  # groups to delete
 ):
-    """Affect groups (Ajax request): renvoie du XML
+    """Affect groups (Ajax POST request): renvoie du XML
     groupsLists: lignes de la forme "group_id;etudid;...\n"
     groupsToCreate: lignes "group_name;etudid;...\n"
     groupsToDelete: group_id;group_id;...
@@ -833,10 +807,14 @@ def setGroups(
         group_name = fs[0].strip()
         if not group_name:
             continue
-        group_id = create_group(partition_id, group_name)
+        try:
+            group = create_group(partition_id, group_name)
+        except ScoValueError as exc:
+            msg = exc.args[0] if len(exc.args) > 0 else "erreur inconnue"
+            return xml_error(msg, code=404)
         # Place dans ce groupe les etudiants indiqués:
         for etudid in fs[1:-1]:
-            change_etud_group_in_partition(etudid, group_id, partition)
+            change_etud_group_in_partition(etudid, group.id, partition)
 
     # Update parcours
     formsemestre = FormSemestre.query.get(formsemestre_id)
@@ -850,29 +828,29 @@ def setGroups(
     return response
 
 
-def create_group(partition_id, group_name="", default=False) -> int:
+def create_group(partition_id, group_name="", default=False) -> GroupDescr:
     """Create a new group in this partition"""
-    partition = get_partition(partition_id)
+    partition = Partition.query.get_or_404(partition_id)
-    formsemestre_id = partition["formsemestre_id"]
+    if not sco_permissions_check.can_change_groups(partition.formsemestre_id):
-    if not sco_permissions_check.can_change_groups(formsemestre_id):
         raise AccessDenied("Vous n'avez pas le droit d'effectuer cette opération !")
     #
     if group_name:
         group_name = group_name.strip()
     if not group_name and not default:
         raise ValueError("invalid group name: ()")
-    # checkGroupName(group_name)
+
-    if check_group_name(group_name, partition):
+    if not GroupDescr.check_name(partition, group_name):
-        raise ScoValueError(
+        raise ScoValueError(f"Le groupe {group_name} existe déjà dans cette partition")
-            f"group_name {group_name} already exists in partition"
+
-        )  # XXX FIX: incorrect error handling (in AJAX)
+    new_numero = (
-    cnx = ndb.GetDBConnexion()
+        max([g.numero if g.numero is not None else 0 for g in partition.groups]) + 1
-    group_id = groupEditor.create(
-        cnx, {"partition_id": partition_id, "group_name": group_name}
     )
-    log("create_group: created group_id={group_id}")
+    group = GroupDescr(partition=partition, group_name=group_name, numero=new_numero)
+    db.session.add(group)
+    db.session.commit()
+    log("create_group: created group_id={group.id}")
     #
-    return group_id
+    return group
 
 
 def delete_group(group_id, partition_id=None):
@@ -1330,40 +1308,43 @@ def partition_set_name(partition_id, partition_name, redirect=1):
         )
 
 
-def group_set_name(group_id, group_name, redirect=True):
+def group_set_name(group: GroupDescr, group_name: str, redirect=True):
     """Set group name"""
+    if not sco_permissions_check.can_change_groups(group.partition.formsemestre.id):
+        raise AccessDenied("Vous n'avez pas le droit d'effectuer cette opération !")
+    if group.group_name is None:
+        raise ValueError("can't set a name to default group")
+    destination = url_for(
+        "scolar.affect_groups",
+        scodoc_dept=g.scodoc_dept,
+        partition_id=group.partition_id,
+    )
     if group_name:
         group_name = group_name.strip()
     if not group_name:
-        raise ScoValueError("nom de groupe vide !")
+        raise ScoValueError("nom de groupe vide !", dest_url=destination)
-    group = get_group(group_id)
+    if not GroupDescr.check_name(group.partition, group_name):
-    if group["group_name"] is None:
+        raise ScoValueError(
-        raise ValueError("can't set a name to default group")
+            "Le nom de groupe existe déjà dans la partition", dest_url=destination
-    formsemestre_id = group["formsemestre_id"]
+        )
-    if not sco_permissions_check.can_change_groups(formsemestre_id):
+
-        raise AccessDenied("Vous n'avez pas le droit d'effectuer cette opération !")
     redirect = int(redirect)
-    cnx = ndb.GetDBConnexion()
+    group.group_name = group_name
-    groupEditor.edit(cnx, {"group_id": group_id, "group_name": group_name})
+    db.session.add(group)
-    check_group_name(group_name, get_partition(group["partition_id"]), True)
+    db.session.commit()
+
     # redirect to partition edit page:
     if redirect:
-        return flask.redirect(
+        return flask.redirect(destination)
-            url_for(
-                "scolar.affect_groups",
-                scodoc_dept=g.scodoc_dept,
-                partition_id=group["partition_id"],
-            )
-        )
 
 
 def group_rename(group_id):
     """Form to rename a group"""
-    group = get_group(group_id)
+    group = GroupDescr.query.get_or_404(group_id)
-    formsemestre_id = group["formsemestre_id"]
+    formsemestre_id = group.partition.formsemestre_id
     if not sco_permissions_check.can_change_groups(formsemestre_id):
         raise AccessDenied("Vous n'avez pas le droit d'effectuer cette opération !")
-    H = ["<h2>Renommer un groupe de %s</h2>" % group["partition_name"]]
+    H = [f"<h2>Renommer un groupe de {group.partition.partition_name or '-'}</h2>"]
     tf = TrivialFormulator(
         request.base_url,
         scu.get_request_args(),
@@ -1373,7 +1354,7 @@ def group_rename(group_id):
                 "group_name",
                 {
                     "title": "Nouveau nom",
-                    "default": group["group_name"],
+                    "default": group.group_name,
                     "size": 12,
                     "allow_null": False,
                     "validator": lambda val, _: len(val) < GROUPNAME_STR_LEN,
@@ -1396,12 +1377,12 @@ def group_rename(group_id):
             url_for(
                 "scolar.affect_groups",
                 scodoc_dept=g.scodoc_dept,
-                partition_id=group["partition_id"],
+                partition_id=group.partition_id,
             )
         )
     else:
         # form submission
-        return group_set_name(group_id, tf[2]["group_name"])
+        return group_set_name(group, tf[2]["group_name"])
 
 
 def groups_auto_repartition(partition_id=None):
@@ -1473,12 +1454,7 @@ def groups_auto_repartition(partition_id=None):
         # Crée les nouveaux groupes
         group_ids = []
         for group_name in group_names:
-            # try:
+            group_ids.append(create_group(partition_id, group_name).id)
-            #     checkGroupName(group_name)
-            # except:
-            #     H.append('<p class="warning">Nom de groupe invalide: %s</p>'%group_name)
-            #     return '\n'.join(H) + tf[1] + html_sco_header.sco_footer()
-            group_ids.append(create_group(partition_id, group_name))
         #
         nt: NotesTableCompat = res_sem.load_formsemestre_results(formsemestre)
         identdict = nt.identdict
@@ -1562,8 +1538,8 @@ def create_etapes_partition(formsemestre_id, partition_name="apo_etapes"):
     groups_by_names = {g["group_name"]: g for g in groups}
     for etape in etapes:
         if not (etape in groups_by_names):
-            gid = create_group(pid, etape)
+            new_group = create_group(pid, etape)
-            g = get_group(gid)
+            g = get_group(new_group.id)  # XXX transition: recupere old style dict
             groups_by_names[etape] = g
     # Place les etudiants dans les groupes
     for i in ins:

app/scodoc/sco_groups_copy.py

@@ -35,15 +35,15 @@ def clone_partitions_and_groups(
         for (new_partition_id, list_groups) in list_groups_per_part:
             if newpart["partition_id"] == new_partition_id:
                 for group in list_groups:
-                    new_group_id = sco_groups.create_group(
+                    new_group = sco_groups.create_group(
                         new_partition_id, group_name=group["group_name"]
                     )
-                    groups_old2new[group["group_id"]] = new_group_id
+                    groups_old2new[group["group_id"]] = new_group.id
     #
     if inscrit_etuds:
         cnx = ndb.GetDBConnexion()
         cursor = cnx.cursor()
-        for old_group_id, new_group_id in groups_old2new.items():
+        for old_group_id, new_group.id in groups_old2new.items():
             cursor.execute(
                 """
                 WITH etuds AS (
@@ -60,7 +60,7 @@ def clone_partitions_and_groups(
                 {
                     "orig_formsemestre_id": orig_formsemestre_id,
                     "old_group_id": old_group_id,
-                    "new_group_id": new_group_id,
+                    "new_group_id": new_group.id,
                 },
             )
         cnx.commit()

app/static/js/groupmgr.js

@@ -44,10 +44,10 @@ function loadGroupes() {
 function populateGroup(node) {
     var group_id = node.attributes.getNamedItem("group_id").value;
     var group_name = node.attributes.getNamedItem("group_name").value;
-
+    var groups_editable = Boolean(parseInt(node.attributes.getNamedItem("groups_editable").value));
     // CREE LA BOITE POUR CE GROUPE
     if (group_id) {
-        var gbox = new CGroupBox(group_id, group_name);
+        var gbox = new CGroupBox(group_id, group_name, groups_editable);
         var etuds = node.getElementsByTagName('etud');
         var x = '';
         gbox.sorting = false; // disable to speedup
@@ -72,7 +72,7 @@ function populateGroup(node) {
 var groupBoxes = new Object(); // assoc group_id : groupBox
 var groupsToDelete = new Object(); // list of group_id to be supressed
 
-var CGroupBox = function (group_id, group_name) {
+var CGroupBox = function (group_id, group_name, groups_editable) {
     group_id = $.trim(group_id);
     var regex = /^\w+$/;
     if (!regex.test(group_id)) {
@@ -86,6 +86,7 @@ var CGroupBox = function (group_id, group_name) {
     groups[group_id] = 1;
     this.group_id = group_id;
     this.group_name = group_name;
+    this.groups_editable = groups_editable;
     this.etuds = new Object();
     this.nbetuds = 0;
     this.isNew = false; // true for newly user-created groups
@@ -131,10 +132,10 @@ var CGroupBox = function (group_id, group_name) {
 $.extend(CGroupBox.prototype, {
     // menu for group title
     groupTitle: function () {
-        var menuSpan = document.createElement("span");
+        let menuSpan = document.createElement("span");
         menuSpan.className = "barrenav";
-        var h = "<table><tr><td><ul class=\"nav\"><li onmouseover=\"MenuDisplay(this)\" onmouseout=\"MenuHide(this)\"><a href=\"#\" class=\"menu custommenu\"><span id=\"titleSpan" + this.group_id + "\" class=\"groupTitle\">menu</span></a><ul>";
+        let h = "<table><tr><td><ul class=\"nav\"><li onmouseover=\"MenuDisplay(this)\" onmouseout=\"MenuHide(this)\"><a href=\"#\" class=\"menu custommenu\"><span id=\"titleSpan" + this.group_id + "\" class=\"groupTitle\">menu</span></a><ul>";
-        if (this.group_id != '_none_') {
+        if (this.groups_editable && (this.group_id != '_none_')) {
             h += "<li><a href=\"#\" onClick=\"suppressGroup('" + this.group_id + "');\">Supprimer</a></li>";
             h += "<li><a href=\"#\" onClick=\"renameGroup('" + this.group_id + "');\">Renommer</a></li>";
         }
@@ -207,7 +208,7 @@ function suppressGroup(group_id) {
     // 1- associate all members to group _none_
     if (!groupBoxes['_none_']) {
         // create group _none_
-        var gbox = new CGroupBox('_none_', 'Etudiants sans groupe');
+        var gbox = new CGroupBox('_none_', 'Etudiants sans groupe', true);
     }
     var dst_group_id = groupBoxes['_none_'].group_id;
     var src_box_etuds = groupBoxes[group_id].etuds;
@@ -271,7 +272,7 @@ function createGroup() {
     }
     var group_id = newGroupId();
     groups_unsaved = true;
-    var gbox = new CGroupBox(group_id, group_name);
+    var gbox = new CGroupBox(group_id, group_name, true);
     gbox.isNew = true;
     gbox.updateTitle();
     return true;
@@ -387,8 +388,8 @@ function submitGroups() {
     groupsToDelete = new Object(); // empty
     var partition_id = document.formGroup.partition_id.value;
     // Send to server
-    $.get(url, {
+    $.post(url, {
-        groupsLists: groupsLists, // encodeURIComponent
+        groupsLists: groupsLists,
         partition_id: partition_id,
         groupsToDelete: todel,
         groupsToCreate: groupsToCreate
@@ -396,8 +397,12 @@ function submitGroups() {
         .done(function (data) {
             processResponse(data);
         })
-        .fail(function () {
+        .fail(function (xhr, status, error) {
-            handleError("Erreur lors de l'enregistrement de groupes");
+            let msg = "inconnue";
+            if (xhr.responseXML.childNodes.length > 0) {
+                msg = xhr.responseXML.childNodes[0].innerHTML;
+            }
+            handleError("Erreur lors de l'enregistrement de groupes: " + msg);
         });
 }
 

app/templates/scolar/affect_groups.html

@@ -4,8 +4,8 @@
 
 <p>Faites glisser les étudiants d'un groupe à l'autre. Les modifications ne 
 sont enregistrées que lorsque vous cliquez sur le bouton "<em>Enregistrer ces groupes</em>".
-Vous pouvez créer de nouveaux groupes. Pour <em>supprimer</em> un groupe, utiliser le lien 
+Vous pouvez créer de nouveaux groupes. Pour <em>supprimer</em> ou <em>renommer</em>
-"suppr." en haut à droite de sa boite. 
+un groupe, utiliser le menu en haut à droite de sa boite. 
 Vous pouvez aussi <a class="stdlink" 
 href="{{ url_for('scolar.groups_auto_repartition', scodoc_dept=g.scodoc_dept, partition_id=partition.id) }}"
 >répartir automatiquement les groupes</a>. 

app/views/scolar.py

@@ -827,27 +827,8 @@ sco_publish(
     Permission.ScoView,
 )
 
-sco_publish("/setGroups", sco_groups.setGroups, Permission.ScoView)
+sco_publish("/setGroups", sco_groups.setGroups, Permission.ScoView, methods=["POST"])
 
-sco_publish("/create_group", sco_groups.create_group, Permission.ScoView)
-
-
-@bp.route("/suppressGroup")  # backward compat (ScoDoc7 API)
-@bp.route("/delete_group")
-@scodoc
-@permission_required(Permission.ScoView)
-@scodoc7func
-def delete_group(group_id, partition_id):
-    sco_groups.delete_group(group_id=group_id, partition_id=partition_id)
-    return "", 204
-
-
-sco_publish(
-    "/group_set_name",
-    sco_groups.group_set_name,
-    Permission.ScoView,
-    methods=["GET", "POST"],
-)
 
 sco_publish(
     "/group_rename",

tools/fakedatabase/create_test_api_database.py

@@ -209,7 +209,7 @@ def create_formsemestre(
     partition_id = sco_groups.partition_create(
         formsemestre.id, default=True, redirect=False
     )
-    _group_id = sco_groups.create_group(partition_id, default=True)
+    group = sco_groups.create_group(partition_id, default=True)
     return formsemestre