From 8b37f661d6a2eb7afe54eb89b2c64b9320998fa0 Mon Sep 17 00:00:00 2001 From: Emmanuel Viennet Date: Sat, 8 Jul 2023 16:35:32 +0200 Subject: [PATCH] =?UTF-8?q?Am=C3=A9liore=20code=20gestion=20des=20groupes?= =?UTF-8?q?=20et=20corrige=20qq=20bugs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/models/formsemestre.py | 11 ++++ app/models/groups.py | 64 ++++++++++++++++------- app/scodoc/sco_groups.py | 81 +++++++++++++++-------------- app/scodoc/sco_permissions_check.py | 4 +- 4 files changed, 101 insertions(+), 59 deletions(-) diff --git a/app/models/formsemestre.py b/app/models/formsemestre.py index 6b76e2250..8ab323cee 100644 --- a/app/models/formsemestre.py +++ b/app/models/formsemestre.py @@ -573,6 +573,17 @@ class FormSemestre(db.Model): user ) + def can_change_groups(self, user: User = None) -> bool: + """Vrai si l'utilisateur (par def. current) peut changer les groupes dans + ce semestre: vérifie permission et verrouillage. + """ + if not self.etat: + return False # semestre verrouillé + user = user or current_user + if user.has_permission(Permission.ScoEtudChangeGroups): + return True # typiquement admin, chef dept + return self.est_responsable(user) + def can_edit_jury(self, user: User = None): """Vrai si utilisateur (par def. current) peut saisir decision de jury dans ce semestre: vérifie permission et verrouillage. diff --git a/app/models/groups.py b/app/models/groups.py index dce377da8..8e9d5b620 100644 --- a/app/models/groups.py +++ b/app/models/groups.py @@ -10,11 +10,11 @@ from operator import attrgetter from sqlalchemy.exc import IntegrityError -from app import db +from app import db, log from app.models import SHORT_STR_LEN from app.models import GROUPNAME_STR_LEN from app.scodoc import sco_utils as scu -from app.scodoc.sco_exceptions import ScoValueError +from app.scodoc.sco_exceptions import AccessDenied, ScoValueError class Partition(db.Model): @@ -119,7 +119,7 @@ class Partition(db.Model): .first() ) - def set_etud_group(self, etudid: int, group: "GroupDescr") -> bool: + def set_etud_group(self, etud: "Identite", group: "GroupDescr") -> bool: """Affect etudid to group_id in given partition. Raises IntegrityError si conflit, or ValueError si ce group_id n'est pas dans cette partition @@ -128,36 +128,37 @@ class Partition(db.Model): """ if not group.id in (g.id for g in self.groups): raise ScoValueError( - f"""Le groupe {group.id} n'est pas dans la partition {self.partition_name or "tous"}""" + f"""Le groupe {group.id} n'est pas dans la partition { + self.partition_name or "tous"}""" ) - if etudid not in (e.id for e in self.formsemestre.etuds): + if etud.id not in (e.id for e in self.formsemestre.etuds): raise ScoValueError( - f"etudiant {etudid} non inscrit au formsemestre du groupe {group.id}" + f"""étudiant {etud.nomprenom} non inscrit au formsemestre du groupe { + group.group_name}""" ) try: existing_row = ( db.session.query(group_membership) - .filter_by(etudid=etudid) + .filter_by(etudid=etud.id) .join(GroupDescr) .filter_by(partition_id=self.id) .first() ) - existing_group_id = existing_row[1] if existing_row: + existing_group_id = existing_row[1] if group.id == existing_group_id: return False - update_row = ( - group_membership.update() - .where( - group_membership.c.etudid == etudid, - group_membership.c.group_id == existing_group_id, - ) - .values(group_id=group.id) - ) - db.session.execute(update_row) + # Fait le changement avec l'ORM sinon risque élevé de blocage + existing_group = GroupDescr.query.get(existing_group_id) + db.session.commit() + group.etuds.append(etud) + existing_group.etuds.remove(etud) + db.session.add(etud) + db.session.add(existing_group) + db.session.add(group) else: new_row = group_membership.insert().values( - etudid=etudid, group_id=group.id + etudid=etud.id, group_id=group.id ) db.session.execute(new_row) db.session.commit() @@ -166,6 +167,33 @@ class Partition(db.Model): raise return True + def create_group(self, group_name="", default=False) -> "GroupDescr": + "Crée un groupe dans cette partition" + if not self.formsemestre.can_change_groups(): + raise AccessDenied( + """Vous n'avez pas le droit d'effectuer cette opération, + ou bien le semestre est verrouillé !""" + ) + if group_name: + group_name = group_name.strip() + if not group_name and not default: + raise ValueError("invalid group name: ()") + if not GroupDescr.check_name(self, group_name, default=default): + raise ScoValueError( + f"Le groupe {group_name} existe déjà dans cette partition" + ) + numeros = [g.numero if g.numero is not None else 0 for g in self.groups] + if len(numeros) > 0: + new_numero = max(numeros) + 1 + else: + new_numero = 0 + group = GroupDescr(partition=self, group_name=group_name, numero=new_numero) + db.session.add(group) + db.session.commit() + log(f"create_group: created group_id={group.id}") + # + return group + class GroupDescr(db.Model): """Description d'un groupe d'une partition""" diff --git a/app/scodoc/sco_groups.py b/app/scodoc/sco_groups.py index 8438ba09c..689dab347 100644 --- a/app/scodoc/sco_groups.py +++ b/app/scodoc/sco_groups.py @@ -34,7 +34,6 @@ Optimisation possible: """ import collections -import operator import time from xml.etree import ElementTree @@ -45,7 +44,7 @@ from flask import g, request from flask import url_for, make_response from sqlalchemy.sql import text -from app import db +from app import cache, db, log from app.comp import res_sem from app.comp.res_compat import NotesTableCompat from app.models import FormSemestre, Identite, Scolog @@ -53,7 +52,6 @@ from app.models import GROUPNAME_STR_LEN, SHORT_STR_LEN from app.models.groups import GroupDescr, Partition, group_membership import app.scodoc.sco_utils as scu import app.scodoc.notesdb as ndb -from app import log, cache from app.scodoc.scolog import logdb from app.scodoc import html_sco_header from app.scodoc import sco_cache @@ -669,7 +667,8 @@ def change_etud_group_in_partition(etudid: int, group: GroupDescr) -> bool: (et le désinscrit d'autres groupes de cette partition) Return True si changement, False s'il était déjà dans ce groupe. """ - if not group.partition.set_etud_group(etudid, group): + etud: Identite = Identite.query.get_or_404(etudid) + if not group.partition.set_etud_group(etud, group): return # pas de changement # - log @@ -706,7 +705,6 @@ def setGroups( Ne peux pas modifier les groupes des partitions non éditables. """ - from app.scodoc import sco_formsemestre def xml_error(msg, code=404): data = ( @@ -716,26 +714,27 @@ def setGroups( response.headers["Content-Type"] = scu.XML_MIMETYPE return response - partition = get_partition(partition_id) - if not partition["groups_editable"] and (groupsToCreate or groupsToDelete): + partition: Partition = Partition.query.get(partition_id) + if not partition.groups_editable and (groupsToCreate or groupsToDelete): msg = "setGroups: partition non editable" log(msg) return xml_error(msg, code=403) - formsemestre_id = partition["formsemestre_id"] - formsemestre = FormSemestre.get_formsemestre(formsemestre_id) - if not sco_permissions_check.can_change_groups(formsemestre_id): + + if not sco_permissions_check.can_change_groups(partition.formsemestre.id): raise AccessDenied("Vous n'avez pas le droit d'effectuer cette opération !") log("***setGroups: partition_id=%s" % partition_id) log("groupsLists=%s" % groupsLists) log("groupsToCreate=%s" % groupsToCreate) log("groupsToDelete=%s" % groupsToDelete) - sem = sco_formsemestre.get_formsemestre(formsemestre_id) - if not sem["etat"]: + + if not partition.formsemestre.etat: raise AccessDenied("Modification impossible: semestre verrouillé") groupsToDelete = [g for g in groupsToDelete.split(";") if g] - etud_groups = formsemestre_get_etud_groupnames(formsemestre_id, attr="group_id") + etud_groups = formsemestre_get_etud_groupnames( + partition.formsemestre.id, attr="group_id" + ) for line in groupsLists.split("\n"): # for each group_id (one per line) fs = line.split(";") group_id = fs[0].strip() @@ -748,15 +747,13 @@ def setGroups( continue group: GroupDescr = GroupDescr.query.get(group_id) # Anciens membres du groupe: - old_members = get_group_members(group_id) - old_members_set = set([x["etudid"] for x in old_members]) + old_members_set = {etud.id for etud in group.etuds} # Place dans ce groupe les etudiants indiqués: for etudid_str in fs[1:-1]: etudid = int(etudid_str) if etudid in old_members_set: - old_members_set.remove( - etudid - ) # a nouveau dans ce groupe, pas besoin de l'enlever + # était dans ce groupe, l'enlever + old_members_set.remove(etudid) if (etudid not in etud_groups) or ( group_id != etud_groups[etudid].get(partition_id, "") ): # pas le meme groupe qu'actuel @@ -765,7 +762,6 @@ def setGroups( cnx = ndb.GetDBConnexion() cursor = cnx.cursor(cursor_factory=ndb.ScoDocCursor) for etudid in old_members_set: - log("removing %s from group %s" % (etudid, group_id)) ndb.SimpleQuery( "DELETE FROM group_membership WHERE etudid=%(etudid)s and group_id=%(group_id)s", {"etudid": etudid, "group_id": group_id}, @@ -775,8 +771,8 @@ def setGroups( cnx, method="removeFromGroup", etudid=etudid, - msg="formsemestre_id=%s,partition_name=%s, group_name=%s" - % (formsemestre_id, partition["partition_name"], group["group_name"]), + msg=f"""formsemestre_id={partition.formsemestre.id},partition_name={ + partition.partition_name}, group_name={group.group_name}""", ) # Supprime les groupes indiqués comme supprimés: @@ -799,7 +795,7 @@ def setGroups( change_etud_group_in_partition(etudid, group.id) # Update parcours - formsemestre.update_inscriptions_parcours_from_groups() + partition.formsemestre.update_inscriptions_parcours_from_groups() data = ( 'Groupes enregistrés' @@ -812,6 +808,7 @@ def setGroups( def create_group(partition_id, group_name="", default=False) -> GroupDescr: """Create a new group in this partition. If default, create default partition (with no name) + Obsolete: utiliser Partition.create_group """ partition = Partition.query.get_or_404(partition_id) if not sco_permissions_check.can_change_groups(partition.formsemestre_id): @@ -833,7 +830,7 @@ def create_group(partition_id, group_name="", default=False) -> GroupDescr: 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}") + log(f"create_group: created group_id={group.id}") # return group @@ -1377,11 +1374,11 @@ def groups_auto_repartition(partition_id=None): """Reparti les etudiants dans des groupes dans une partition, en respectant le niveau et la mixité. """ - partition = get_partition(partition_id) - if not partition["groups_editable"]: + partition: Partition = Partition.query.get_or_404(partition_id) + if not partition.groups_editable: raise AccessDenied("Partition non éditable") - formsemestre_id = partition["formsemestre_id"] - formsemestre = FormSemestre.get_formsemestre(formsemestre_id) + formsemestre_id = partition.formsemestre_id + formsemestre = partition.formsemestre # renvoie sur page édition groupes dest_url = url_for( "scolar.affect_groups", scodoc_dept=g.scodoc_dept, partition_id=partition_id @@ -1404,12 +1401,14 @@ def groups_auto_repartition(partition_id=None): H = [ html_sco_header.sco_header(page_title="Répartition des groupes"), - "

Répartition des groupes de %s

" % partition["partition_name"], - f"

Semestre {formsemestre.titre_annee()}

", - """

Les groupes existants seront effacés et remplacés par + f"""

Répartition des groupes de {partition.partition_name}

+

Semestre {formsemestre.titre_annee()}

", +

Les groupes existants seront effacés et remplacés par ceux créés ici. La répartition aléatoire tente d'uniformiser le niveau des groupes (en utilisant la dernière moyenne générale disponible pour - chaque étudiant) et de maximiser la mixité de chaque groupe.

""", + chaque étudiant) et de maximiser la mixité de chaque groupe. +

+ """, ] tf = TrivialFormulator( @@ -1429,23 +1428,24 @@ def groups_auto_repartition(partition_id=None): # form submission log( "groups_auto_repartition( partition_id=%s partition_name=%s" - % (partition_id, partition["partition_name"]) + % (partition_id, partition.partition_name) ) groupNames = tf[2]["groupNames"] - group_names = sorted(set([x.strip() for x in groupNames.split(",")])) + group_names = sorted({x.strip() for x in groupNames.split(",")}) # Détruit les groupes existant de cette partition - for old_group in get_partition_groups(partition): - group_delete(old_group["group_id"]) + for group in partition.groups: + db.session.delete(group) + db.session.commit() # Crée les nouveaux groupes groups = [] for group_name in group_names: if group_name.strip(): - groups.append(create_group(partition_id, group_name)) + groups.append(partition.create_group(group_name)) # nt: NotesTableCompat = res_sem.load_formsemestre_results(formsemestre) identdict = nt.identdict # build: { civilite : liste etudids trie par niveau croissant } - civilites = set([x["civilite"] for x in identdict.values()]) + civilites = {x["civilite"] for x in identdict.values()} listes = {} for civilite in civilites: listes[civilite] = [ @@ -1460,14 +1460,17 @@ def groups_auto_repartition(partition_id=None): igroup = 0 nbgroups = len(groups) while n > 0: + log(f"n={n}") for civilite in civilites: + log(f"civilite={civilite}") if len(listes[civilite]): n -= 1 etudid = listes[civilite].pop()[1] group = groups[igroup] igroup = (igroup + 1) % nbgroups + log(f"in {etudid} in group {group.id}") change_etud_group_in_partition(etudid, group) - log("%s in group %s" % (etudid, group.id)) + log(f"{etudid} in group {group.id}") return flask.redirect(dest_url) @@ -1475,8 +1478,6 @@ def _get_prev_moy(etudid, formsemestre_id): """Donne la derniere moyenne generale calculee pour cette étudiant, ou 0 si on n'en trouve pas (nouvel inscrit,...). """ - from app.scodoc import sco_cursus_dut - info = sco_etud.get_etud_info(etudid=etudid, filled=True) if not info: raise ScoValueError("etudiant invalide: etudid=%s" % etudid) diff --git a/app/scodoc/sco_permissions_check.py b/app/scodoc/sco_permissions_check.py index 224881bf8..dcc19f455 100644 --- a/app/scodoc/sco_permissions_check.py +++ b/app/scodoc/sco_permissions_check.py @@ -142,7 +142,9 @@ def check_access_diretud(formsemestre_id, required_permission=Permission.ScoImpl def can_change_groups(formsemestre_id: int) -> bool: - "Vrai si l'utilisateur peut changer les groupes dans ce semestre" + """Vrai si l'utilisateur peut changer les groupes dans ce semestre + Obsolete: utiliser FormSemestre.can_change_groups + """ formsemestre: FormSemestre = FormSemestre.query.get_or_404(formsemestre_id) if not formsemestre.etat: return False # semestre verrouillé