From 9d18ed46716ff75c1897f7bf419cad87c6fdc29d Mon Sep 17 00:00:00 2001 From: Emmanuel Viennet Date: Sat, 3 Jun 2023 22:43:04 +0200 Subject: [PATCH] =?UTF-8?q?-=20Am=C3=A9lioration=20enregistrement=20note.?= =?UTF-8?q?=20-=20Nouveau=20point=20API:=20/evaluation//notes/set=20-=20Corrige=20API=20/evaluation//notes=20-=20Modernisation=20de=20code.=20-=20Am=C3=A9lio?= =?UTF-8?q?re=20tests=20unitaires=20APi=20evaluation.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/api/evaluations.py | 70 +++- app/scodoc/sco_evaluation_db.py | 2 +- app/scodoc/sco_saisie_notes.py | 301 +++++++++--------- app/static/js/saisie_notes.js | 213 +++++++------ app/views/notes.py | 6 - sco_version.py | 2 +- tests/api/test_api_evaluations.py | 32 +- tests/api/test_api_permissions.py | 1 + tests/api/tools_test_api.py | 3 +- tests/unit/test_sco_basic.py | 8 +- .../fakedatabase/create_test_api_database.py | 2 +- 11 files changed, 347 insertions(+), 293 deletions(-) diff --git a/app/api/evaluations.py b/app/api/evaluations.py index d5a7a3cfcd..8cd277679d 100644 --- a/app/api/evaluations.py +++ b/app/api/evaluations.py @@ -8,7 +8,7 @@ ScoDoc 9 API : accès aux évaluations """ -from flask import g +from flask import g, request from flask_json import as_json from flask_login import login_required @@ -17,7 +17,7 @@ import app from app.api import api_bp as bp, api_web_bp from app.decorators import scodoc, permission_required from app.models import Evaluation, ModuleImpl, FormSemestre -from app.scodoc import sco_evaluation_db +from app.scodoc import sco_evaluation_db, sco_saisie_notes from app.scodoc.sco_permissions import Permission import app.scodoc.sco_utils as scu @@ -28,7 +28,7 @@ import app.scodoc.sco_utils as scu @scodoc @permission_required(Permission.ScoView) @as_json -def the_eval(evaluation_id: int): +def evaluation(evaluation_id: int): """Description d'une évaluation. { @@ -93,24 +93,22 @@ def evaluations(moduleimpl_id: int): @as_json def evaluation_notes(evaluation_id: int): """ - Retourne la liste des notes à partir de l'id d'une évaluation donnée + Retourne la liste des notes de l'évaluation - evaluation_id : l'id d'une évaluation + evaluation_id : l'id de l'évaluation Exemple de résultat : { - "1": { - "id": 1, - "etudid": 10, + "11": { + "etudid": 11, "evaluation_id": 1, "value": 15.0, "comment": "", "date": "Wed, 20 Apr 2022 06:49:05 GMT", "uid": 2 }, - "2": { - "id": 2, - "etudid": 1, + "12": { + "etudid": 12, "evaluation_id": 1, "value": 12.0, "comment": "", @@ -128,8 +126,8 @@ def evaluation_notes(evaluation_id: int): .filter_by(dept_id=g.scodoc_dept_id) ) - the_eval = query.first_or_404() - dept = the_eval.moduleimpl.formsemestre.departement + evaluation = query.first_or_404() + dept = evaluation.moduleimpl.formsemestre.departement app.set_sco_dept(dept.acronym) notes = sco_evaluation_db.do_evaluation_get_all_notes(evaluation_id) @@ -137,7 +135,49 @@ def evaluation_notes(evaluation_id: int): # "ABS", "EXC", etc mais laisse les notes sur le barème de l'éval. note = notes[etudid] note["value"] = scu.fmt_note(note["value"], keep_numeric=True) - note["note_max"] = the_eval.note_max + note["note_max"] = evaluation.note_max del note["id"] - return notes + # in JS, keys must be string, not integers + return {str(etudid): note for etudid, note in notes.items()} + + +@bp.route("/evaluation//notes/set", methods=["POST"]) +@api_web_bp.route("/evaluation//notes/set", methods=["POST"]) +@login_required +@scodoc +@permission_required(Permission.ScoEnsView) +@as_json +def evaluation_set_notes(evaluation_id: int): + """Écriture de notes dans une évaluation. + The request content type should be "application/json", + and contains: + { + 'notes' : [ (etudid, value), ... ], + 'comment' : opetional string + } + Result: + - nb_changed: nombre de notes changées + - nb_suppress: nombre de notes effacées + - etudids_with_decision: liste des etudiants dont la note a changé + alors qu'ils ont une décision de jury enregistrée. + """ + query = Evaluation.query.filter_by(id=evaluation_id) + if g.scodoc_dept: + query = ( + query.join(ModuleImpl) + .join(FormSemestre) + .filter_by(dept_id=g.scodoc_dept_id) + ) + evaluation = query.first_or_404() + dept = evaluation.moduleimpl.formsemestre.departement + app.set_sco_dept(dept.acronym) + data = request.get_json(force=True) # may raise 400 Bad Request + notes = data.get("notes") + if notes is None: + return scu.json_error(404, "no notes") + if not isinstance(notes, list): + return scu.json_error(404, "invalid notes argument (must be a list)") + return sco_saisie_notes.save_notes( + evaluation, notes, comment=data.get("comment", "") + ) diff --git a/app/scodoc/sco_evaluation_db.py b/app/scodoc/sco_evaluation_db.py index 44ec6a7689..7cbc323310 100644 --- a/app/scodoc/sco_evaluation_db.py +++ b/app/scodoc/sco_evaluation_db.py @@ -252,7 +252,7 @@ def do_evaluation_delete(evaluation_id): def do_evaluation_get_all_notes( evaluation_id, table="notes_notes", filter_suppressed=True, by_uid=None ): - """Toutes les notes pour une evaluation: { etudid : { 'value' : value, 'date' : date ... }} + """Toutes les notes pour une évaluation: { etudid : { 'value' : value, 'date' : date ... }} Attention: inclut aussi les notes des étudiants qui ne sont plus inscrits au module. """ # pas de cache pour (rares) appels via undo_notes ou specifiant un enseignant diff --git a/app/scodoc/sco_saisie_notes.py b/app/scodoc/sco_saisie_notes.py index e5d7f3da55..6e9084d0c8 100644 --- a/app/scodoc/sco_saisie_notes.py +++ b/app/scodoc/sco_saisie_notes.py @@ -36,16 +36,20 @@ import flask from flask import g, url_for, request from flask_login import current_user +from app import log from app.auth.models import User from app.comp import res_sem from app.comp.res_compat import NotesTableCompat -from app.models import Evaluation, FormSemestre -from app.models import ModuleImpl, NotesNotes, ScolarNews +from app.models import ( + Evaluation, + FormSemestre, + Module, + ModuleImpl, + NotesNotes, + ScolarNews, +) from app.models.etudiants import Identite -import app.scodoc.sco_utils as scu -from app.scodoc.sco_utils import ModuleType -import app.scodoc.notesdb as ndb -from app import log + from app.scodoc.sco_exceptions import ( AccessDenied, InvalidNoteValue, @@ -55,14 +59,14 @@ from app.scodoc.sco_exceptions import ( ScoInvalidParamError, ScoValueError, ) -from app.scodoc.TrivialFormulator import TrivialFormulator, TF from app.scodoc import html_sco_header, sco_users from app.scodoc import htmlutils from app.scodoc import sco_abs from app.scodoc import sco_cache from app.scodoc import sco_edit_module -from app.scodoc import sco_evaluations +from app.scodoc import sco_etud from app.scodoc import sco_evaluation_db +from app.scodoc import sco_evaluations from app.scodoc import sco_excel from app.scodoc import sco_formsemestre_inscriptions from app.scodoc import sco_groups @@ -70,7 +74,11 @@ from app.scodoc import sco_groups_view from app.scodoc import sco_moduleimpl from app.scodoc import sco_permissions_check from app.scodoc import sco_undo_notes -from app.scodoc import sco_etud +import app.scodoc.notesdb as ndb +from app.scodoc.TrivialFormulator import TrivialFormulator, TF +import app.scodoc.sco_utils as scu +from app.scodoc.sco_utils import json_error +from app.scodoc.sco_utils import ModuleType def convert_note_from_string( @@ -128,29 +136,30 @@ def _displayNote(val): return val -def _check_notes(notes: list[(int, float)], evaluation: dict, mod: dict): +def _check_notes(notes: list[(int, float)], evaluation: Evaluation): # XXX typehint : float or str """notes is a list of tuples (etudid, value) mod is the module (used to ckeck type, for malus) returns list of valid notes (etudid, float value) - and 4 lists of etudid: invalids, withoutnotes, absents, tosuppress, existingjury + and 4 lists of etudid: etudids_invalids, etudids_without_notes, etudids_absents, etudid_to_suppress """ - note_max = evaluation["note_max"] - if mod["module_type"] in ( + note_max = evaluation.note_max or 0.0 + module: Module = evaluation.moduleimpl.module + if module.module_type in ( scu.ModuleType.STANDARD, scu.ModuleType.RESSOURCE, scu.ModuleType.SAE, ): note_min = scu.NOTES_MIN - elif mod["module_type"] == ModuleType.MALUS: + elif module.module_type == ModuleType.MALUS: note_min = -20.0 else: raise ValueError("Invalid module type") # bug - L = [] # liste (etudid, note) des notes ok (ou absent) - invalids = [] # etudid avec notes invalides - withoutnotes = [] # etudid sans notes (champs vides) - absents = [] # etudid absents - tosuppress = [] # etudids avec ancienne note à supprimer + valid_notes = [] # liste (etudid, note) des notes ok (ou absent) + etudids_invalids = [] # etudid avec notes invalides + etudids_without_notes = [] # etudid sans notes (champs vides) + etudids_absents = [] # etudid absents + etudid_to_suppress = [] # etudids avec ancienne note à supprimer for etudid, note in notes: note = str(note).strip().upper() @@ -166,31 +175,34 @@ def _check_notes(notes: list[(int, float)], evaluation: dict, mod: dict): note_max, note_min=note_min, etudid=etudid, - absents=absents, - tosuppress=tosuppress, - invalids=invalids, + absents=etudids_absents, + tosuppress=etudid_to_suppress, + invalids=etudids_invalids, ) if not invalid: - L.append((etudid, value)) + valid_notes.append((etudid, value)) else: - withoutnotes.append(etudid) - return L, invalids, withoutnotes, absents, tosuppress + etudids_without_notes.append(etudid) + return ( + valid_notes, + etudids_invalids, + etudids_without_notes, + etudids_absents, + etudid_to_suppress, + ) def do_evaluation_upload_xls(): """ Soumission d'un fichier XLS (evaluation_id, notefile) """ - authuser = current_user vals = scu.get_request_args() evaluation_id = int(vals["evaluation_id"]) comment = vals["comment"] - E = sco_evaluation_db.do_evaluation_list({"evaluation_id": evaluation_id})[0] - M = sco_moduleimpl.moduleimpl_withmodule_list(moduleimpl_id=E["moduleimpl_id"])[0] - # Check access - # (admin, respformation, and responsable_id) - if not sco_permissions_check.can_edit_notes(authuser, E["moduleimpl_id"]): - raise AccessDenied("Modification des notes impossible pour %s" % authuser) + evaluation: Evaluation = Evaluation.query.get_or_404(evaluation_id) + # Check access (admin, respformation, and responsable_id) + if not sco_permissions_check.can_edit_notes(current_user, evaluation.moduleimpl_id): + raise AccessDenied(f"Modification des notes impossible pour {current_user}") # diag, lines = sco_excel.excel_file_to_list(vals["notefile"]) try: @@ -239,14 +251,16 @@ def do_evaluation_upload_xls(): if etudid: notes.append((etudid, val)) ni += 1 - except: + except Exception as exc: diag.append( f"""Erreur: Ligne invalide ! (erreur ligne {ni})
{lines[ni]}""" ) - raise InvalidNoteValue() + raise InvalidNoteValue() from exc # -- check values - L, invalids, withoutnotes, absents, _ = _check_notes(notes, E, M["module"]) - if len(invalids): + valid_notes, invalids, withoutnotes, absents, _ = _check_notes( + notes, evaluation + ) + if invalids: diag.append( f"Erreur: la feuille contient {len(invalids)} notes invalides

" ) @@ -258,37 +272,33 @@ def do_evaluation_upload_xls(): diag.append("Notes invalides pour: " + ", ".join(etudsnames)) raise InvalidNoteValue() else: - nb_changed, nb_suppress, existing_decisions = notes_add( - authuser, evaluation_id, L, comment + etudids_changed, nb_suppress, etudids_with_decisions = notes_add( + current_user, evaluation_id, valid_notes, comment ) # news - E = sco_evaluation_db.do_evaluation_list({"evaluation_id": evaluation_id})[ - 0 - ] - M = sco_moduleimpl.moduleimpl_list(moduleimpl_id=E["moduleimpl_id"])[0] - mod = sco_edit_module.module_list(args={"module_id": M["module_id"]})[0] - mod["moduleimpl_id"] = M["moduleimpl_id"] - mod["url"] = url_for( + module: Module = evaluation.moduleimpl.module + status_url = url_for( "notes.moduleimpl_status", scodoc_dept=g.scodoc_dept, - moduleimpl_id=mod["moduleimpl_id"], + moduleimpl_id=evaluation.moduleimpl_id, _external=True, ) ScolarNews.add( typ=ScolarNews.NEWS_NOTE, - obj=M["moduleimpl_id"], - text='Chargement notes dans %(titre)s' % mod, - url=mod["url"], + obj=evaluation.moduleimpl_id, + text=f"""Chargement notes dans { + module.titre or module.code}""", + url=status_url, max_frequency=30 * 60, # 30 minutes ) - msg = ( - "

%d notes changées (%d sans notes, %d absents, %d note supprimées)

" - % (nb_changed, len(withoutnotes), len(absents), nb_suppress) - ) - if existing_decisions: - msg += """

Important: il y avait déjà des décisions de jury enregistrées, qui sont potentiellement à revoir suite à cette modification !

""" - # msg += '

' + str(notes) # debug + msg = f"""

{len(etudids_changed)} notes changées ({len(withoutnotes)} sans notes, { + len(absents)} absents, {nb_suppress} note supprimées) +

""" + if etudids_with_decisions: + msg += """

Important: il y avait déjà des décisions de jury + enregistrées, qui sont peut-être à revoir suite à cette modification !

+ """ return 1, msg except InvalidNoteValue: @@ -310,14 +320,12 @@ def do_evaluation_set_etud_note(evaluation: Evaluation, etud: Identite, value) - if not sco_permissions_check.can_edit_notes(current_user, evaluation.moduleimpl.id): raise AccessDenied(f"Modification des notes impossible pour {current_user}") # Convert and check value - L, invalids, _, _, _ = _check_notes( - [(etud.id, value)], evaluation.to_dict(), evaluation.moduleimpl.module.to_dict() - ) + L, invalids, _, _, _ = _check_notes([(etud.id, value)], evaluation) if len(invalids) == 0: - nb_changed, _, _ = notes_add( + etudids_changed, _, _ = notes_add( current_user, evaluation.id, L, "Initialisation notes" ) - if nb_changed == 1: + if len(etudids_changed) == 1: return True return False # error @@ -352,9 +360,7 @@ def do_evaluation_set_missing( if etudid not in notes_db: # pas de note notes.append((etudid, value)) # Convert and check values - L, invalids, _, _, _ = _check_notes( - notes, evaluation.to_dict(), modimpl.module.to_dict() - ) + valid_notes, invalids, _, _, _ = _check_notes(notes, evaluation) dest_url = url_for( "notes.saisie_notes", scodoc_dept=g.scodoc_dept, evaluation_id=evaluation_id ) @@ -372,13 +378,13 @@ def do_evaluation_set_missing( """ # Confirm action if not dialog_confirmed: - plural = len(L) > 1 + plural = len(valid_notes) > 1 return scu.confirm_dialog( f"""

Mettre toutes les notes manquantes de l'évaluation à la valeur {value} ?

Seuls les étudiants pour lesquels aucune note (ni valeur, ni ABS, ni EXC) n'a été rentrée seront affectés.

-

{len(L)} étudiant{"s" if plural else ""} concerné{"s" if plural else ""} +

{len(valid_notes)} étudiant{"s" if plural else ""} concerné{"s" if plural else ""} par ce changement de note.

""", @@ -392,7 +398,7 @@ def do_evaluation_set_missing( ) # ok comment = "Initialisation notes manquantes" - nb_changed, _, _ = notes_add(current_user, evaluation_id, L, comment) + etudids_changed, _, _ = notes_add(current_user, evaluation_id, valid_notes, comment) # news url = url_for( "notes.moduleimpl_status", @@ -408,7 +414,7 @@ def do_evaluation_set_missing( ) return f""" { html_sco_header.sco_header() } -

{nb_changed} notes changées

+

{len(etudids_changed)} notes changées

  • Revenir au formulaire de saisie des notes @@ -454,7 +460,7 @@ def evaluation_suppress_alln(evaluation_id, dialog_confirmed=False): ) if not dialog_confirmed: - nb_changed, nb_suppress, existing_decisions = notes_add( + etudids_changed, nb_suppress, existing_decisions = notes_add( current_user, evaluation_id, notes, do_it=False, check_inscription=False ) msg = f"""

    Confirmer la suppression des {nb_suppress} notes ? @@ -475,14 +481,14 @@ def evaluation_suppress_alln(evaluation_id, dialog_confirmed=False): ) # modif - nb_changed, nb_suppress, existing_decisions = notes_add( + etudids_changed, nb_suppress, existing_decisions = notes_add( current_user, evaluation_id, notes, comment="effacer tout", check_inscription=False, ) - assert nb_changed == nb_suppress + assert len(etudids_changed) == nb_suppress H = [f"""

    {nb_suppress} notes supprimées

    """] if existing_decisions: H.append( @@ -516,7 +522,7 @@ def notes_add( comment=None, do_it=True, check_inscription=True, -) -> tuple: +) -> tuple[list[int], int, list[int]]: """ Insert or update notes notes is a list of tuples (etudid,value) @@ -524,11 +530,12 @@ def notes_add( WOULD be changed or suppressed. Nota: - si la note existe deja avec valeur distincte, ajoute une entree au log (notes_notes_log) - Return tuple (nb_changed, nb_suppress, existing_decisions) + + Return: tuple (etudids_changed, nb_suppress, etudids_with_decision) """ now = psycopg2.Timestamp(*time.localtime()[:6]) - # Verifie inscription et valeur note + # Vérifie inscription et valeur note inscrits = { x[0] for x in sco_groups.do_evaluation_listeetuds_groups( @@ -547,13 +554,13 @@ def notes_add( # Met a jour la base cnx = ndb.GetDBConnexion() cursor = cnx.cursor(cursor_factory=ndb.ScoDocCursor) - nb_changed = 0 + etudids_changed = [] nb_suppress = 0 evaluation: Evaluation = Evaluation.query.get_or_404(evaluation_id) formsemestre: FormSemestre = evaluation.moduleimpl.formsemestre res: NotesTableCompat = res_sem.load_formsemestre_results(formsemestre) # etudids pour lesquels il y a une decision de jury et que la note change: - etudids_with_existing_decision = [] + etudids_with_decision = [] try: for etudid, value in notes: changed = False @@ -561,7 +568,7 @@ def notes_add( # nouvelle note if value != scu.NOTES_SUPPRESS: if do_it: - aa = { + args = { "etudid": etudid, "evaluation_id": evaluation_id, "value": value, @@ -569,27 +576,21 @@ def notes_add( "uid": user.id, "date": now, } - ndb.quote_dict(aa) - try: - cursor.execute( - """INSERT INTO notes_notes - (etudid, evaluation_id, value, comment, date, uid) - VALUES (%(etudid)s,%(evaluation_id)s,%(value)s,%(comment)s,%(date)s,%(uid)s) - """, - aa, - ) - except psycopg2.errors.UniqueViolation as exc: - # XXX ne devrait pas arriver mais bug possible ici (non reproductible) - existing_note = NotesNotes.query.filter_by( - evaluation_id=evaluation_id, etudid=etudid - ).first() - sco_cache.EvaluationCache.delete(evaluation_id) - notes_db = sco_evaluation_db.do_evaluation_get_all_notes( - evaluation_id - ) - raise ScoBugCatcher( - f"dup: existing={existing_note} etudid={repr(etudid)} value={value} in_db={etudid in notes_db}" - ) from exc + ndb.quote_dict(args) + # Note: le conflit ci-dessous peut arriver si un autre thread + # a modifié la base après qu'on ait lu notes_db + cursor.execute( + """INSERT INTO notes_notes + (etudid, evaluation_id, value, comment, date, uid) + VALUES + (%(etudid)s,%(evaluation_id)s,%(value)s, + %(comment)s,%(date)s,%(uid)s) + ON CONFLICT ON CONSTRAINT notes_notes_etudid_evaluation_id_key + DO UPDATE SET etudid=%(etudid)s, evaluation_id=%(evaluation_id)s, + value=%(value)s, comment=%(comment)s, date=%(date)s, uid=%(uid)s + """, + args, + ) changed = True else: # il y a deja une note @@ -615,7 +616,7 @@ def notes_add( """, {"etudid": etudid, "evaluation_id": evaluation_id}, ) - aa = { + args = { "etudid": etudid, "evaluation_id": evaluation_id, "value": value, @@ -623,7 +624,7 @@ def notes_add( "comment": comment, "uid": user.id, } - ndb.quote_dict(aa) + ndb.quote_dict(args) if value != scu.NOTES_SUPPRESS: if do_it: cursor.execute( @@ -632,34 +633,36 @@ def notes_add( WHERE etudid = %(etudid)s and evaluation_id = %(evaluation_id)s """, - aa, + args, ) else: # suppression ancienne note if do_it: log( - "notes_add, suppress, evaluation_id=%s, etudid=%s, oldval=%s" - % (evaluation_id, etudid, oldval) + f"""notes_add, suppress, evaluation_id={evaluation_id}, etudid={ + etudid}, oldval={oldval}""" ) cursor.execute( """DELETE FROM notes_notes WHERE etudid = %(etudid)s AND evaluation_id = %(evaluation_id)s """, - aa, + args, ) # garde trace de la suppression dans l'historique: - aa["value"] = scu.NOTES_SUPPRESS + args["value"] = scu.NOTES_SUPPRESS cursor.execute( - """INSERT INTO notes_notes_log (etudid,evaluation_id,value,comment,date,uid) - VALUES (%(etudid)s, %(evaluation_id)s, %(value)s, %(comment)s, %(date)s, %(uid)s) + """INSERT INTO notes_notes_log + (etudid,evaluation_id,value,comment,date,uid) + VALUES + (%(etudid)s, %(evaluation_id)s, %(value)s, %(comment)s, %(date)s, %(uid)s) """, - aa, + args, ) nb_suppress += 1 if changed: - nb_changed += 1 + etudids_changed.append(etudid) if res.etud_has_decision(etudid): - etudids_with_existing_decision.append(etudid) + etudids_with_decision.append(etudid) except Exception as exc: log("*** exception in notes_add") if do_it: @@ -672,7 +675,7 @@ def notes_add( cnx.commit() sco_cache.invalidate_formsemestre(formsemestre_id=formsemestre.id) sco_cache.EvaluationCache.delete(evaluation_id) - return nb_changed, nb_suppress, etudids_with_existing_decision + return etudids_changed, nb_suppress, etudids_with_decision def saisie_notes_tableur(evaluation_id, group_ids=()): @@ -1345,48 +1348,56 @@ def _form_saisie_notes( return None -def save_note(etudid=None, evaluation_id=None, value=None, comment=""): - """Enregistre une note (ajax)""" - log( - f"save_note: evaluation_id={evaluation_id} etudid={etudid} uid={current_user} value={value}" - ) - E = sco_evaluation_db.do_evaluation_list({"evaluation_id": evaluation_id})[0] - M = sco_moduleimpl.moduleimpl_list(moduleimpl_id=E["moduleimpl_id"])[0] - Mod = sco_edit_module.module_list(args={"module_id": M["module_id"]})[0] - Mod["url"] = url_for( +def save_notes( + evaluation: Evaluation, notes: list[tuple[(int, float)]], comment: str = "" +) -> dict: + """Enregistre une liste de notes. + Vérifie que les étudiants sont bien inscrits à ce module, et que l'on a le droit. + Result: dict avec + """ + log(f"save_note: evaluation_id={evaluation.id} uid={current_user} notes={notes}") + status_url = url_for( "notes.moduleimpl_status", scodoc_dept=g.scodoc_dept, - moduleimpl_id=M["moduleimpl_id"], + moduleimpl_id=evaluation.moduleimpl_id, _external=True, ) - result = {"nbchanged": 0} # JSON # Check access: admin, respformation, or responsable_id - if not sco_permissions_check.can_edit_notes(current_user, E["moduleimpl_id"]): - result["status"] = "unauthorized" + if not sco_permissions_check.can_edit_notes(current_user, evaluation.moduleimpl_id): + return json_error(403, "modification notes non autorisee pour cet utilisateur") + # + valid_notes, _, _, _, _ = _check_notes(notes, evaluation) + if valid_notes: + etudids_changed, _, etudids_with_decision = notes_add( + current_user, evaluation.id, valid_notes, comment=comment, do_it=True + ) + ScolarNews.add( + typ=ScolarNews.NEWS_NOTE, + obj=evaluation.moduleimpl_id, + text=f"""Chargement notes dans { + evaluation.moduleimpl.module.titre or evaluation.moduleimpl.module.code}""", + url=status_url, + max_frequency=30 * 60, # 30 minutes + ) + result = { + "etudids_with_decision": etudids_with_decision, + "etudids_changed": etudids_changed, + "history_menu": { + etudid: get_note_history_menu(evaluation.id, etudid) + for etudid in etudids_changed + }, + } else: - L, _, _, _, _ = _check_notes([(etudid, value)], E, Mod) - if L: - nbchanged, _, existing_decisions = notes_add( - current_user, evaluation_id, L, comment=comment, do_it=True - ) - ScolarNews.add( - typ=ScolarNews.NEWS_NOTE, - obj=M["moduleimpl_id"], - text='Chargement notes dans %(titre)s' % Mod, - url=Mod["url"], - max_frequency=30 * 60, # 30 minutes - ) - result["nbchanged"] = nbchanged - result["existing_decisions"] = existing_decisions - if nbchanged > 0: - result["history_menu"] = get_note_history_menu(evaluation_id, etudid) - else: - result["history_menu"] = "" # no update needed - result["status"] = "ok" - return scu.sendJSON(result) + result = { + "etudids_changed": [], + "etudids_with_decision": [], + "history_menu": [], + } + + return result -def get_note_history_menu(evaluation_id, etudid): +def get_note_history_menu(evaluation_id: int, etudid: int) -> str: """Menu HTML historique de la note""" history = sco_undo_notes.get_note_history(evaluation_id, etudid) if not history: diff --git a/app/static/js/saisie_notes.js b/app/static/js/saisie_notes.js index 208368ca8a..7fb223bc53 100644 --- a/app/static/js/saisie_notes.js +++ b/app/static/js/saisie_notes.js @@ -1,133 +1,142 @@ // Formulaire saisie des notes $().ready(function () { + $("#formnotes .note").bind("blur", valid_note); - $("#formnotes .note").bind("blur", valid_note); - - $("#formnotes input").bind("paste", paste_text); - $(".btn_masquer_DEM").bind("click", masquer_DEM); - + $("#formnotes input").bind("paste", paste_text); + $(".btn_masquer_DEM").bind("click", masquer_DEM); }); function is_valid_note(v) { - if (!v) - return true; + if (!v) return true; - var note_min = parseFloat($("#eval_note_min").text()); - var note_max = parseFloat($("#eval_note_max").text()); + var note_min = parseFloat($("#eval_note_min").text()); + var note_max = parseFloat($("#eval_note_max").text()); - if (!v.match("^-?[0-9]*.?[0-9]*$")) { - return (v == "ABS") || (v == "EXC") || (v == "SUPR") || (v == "ATT") || (v == "DEM"); - } else { - var x = parseFloat(v); - return (x >= note_min) && (x <= note_max); - } + if (!v.match("^-?[0-9]*.?[0-9]*$")) { + return v == "ABS" || v == "EXC" || v == "SUPR" || v == "ATT" || v == "DEM"; + } else { + var x = parseFloat(v); + return x >= note_min && x <= note_max; + } } function valid_note(e) { - var v = this.value.trim().toUpperCase().replace(",", "."); - if (is_valid_note(v)) { - if (v && (v != $(this).attr('data-last-saved-value'))) { - this.className = "note_valid_new"; - var etudid = $(this).attr('data-etudid'); - save_note(this, v, etudid); - } - } else { - /* Saisie invalide */ - this.className = "note_invalid"; - sco_message("valeur invalide ou hors barème"); + var v = this.value.trim().toUpperCase().replace(",", "."); + if (is_valid_note(v)) { + if (v && v != $(this).attr("data-last-saved-value")) { + this.className = "note_valid_new"; + const etudid = parseInt($(this).attr("data-etudid")); + save_note(this, v, etudid); } + } else { + /* Saisie invalide */ + this.className = "note_invalid"; + sco_message("valeur invalide ou hors barème"); + } } -function save_note(elem, v, etudid) { - var evaluation_id = $("#formnotes_evaluation_id").attr("value"); - var formsemestre_id = $("#formnotes_formsemestre_id").attr("value"); - $('#sco_msg').html("en cours...").show(); - $.post(SCO_URL + '/Notes/save_note', - { - 'etudid': etudid, - 'evaluation_id': evaluation_id, - 'value': v, - 'comment': document.getElementById('formnotes_comment').value +async function save_note(elem, v, etudid) { + let evaluation_id = $("#formnotes_evaluation_id").attr("value"); + let formsemestre_id = $("#formnotes_formsemestre_id").attr("value"); + $("#sco_msg").html("en cours...").show(); + try { + const response = await fetch( + SCO_URL + "/../api/evaluation/" + evaluation_id + "/notes/set", + { + method: "POST", + headers: { + "Content-Type": "application/json", }, - function (result) { - $('#sco_msg').hide(); - if (result['nbchanged'] > 0) { - sco_message("enregistré"); - elem.className = "note_saved"; - // il y avait une decision de jury ? - if (result.existing_decisions[0] == etudid) { - if (v != $(elem).attr('data-orig-value')) { - $("#jurylink_" + etudid).html('mettre à jour décision de jury'); - } else { - $("#jurylink_" + etudid).html(''); - } - } - // mise a jour menu historique - if (result['history_menu']) { - $("#hist_" + etudid).html(result['history_menu']); - } - $(elem).attr('data-last-saved-value', v); - } else { - $('#sco_msg').html("").show(); - sco_message("valeur non enregistrée"); - } - - } + body: JSON.stringify({ + notes: [[etudid, v]], + comment: document.getElementById("formnotes_comment").value, + }), + } ); + if (!response.ok) { + sco_message("Erreur: valeur non enregistrée"); + } else { + const data = await response.json(); + $("#sco_msg").hide(); + if (data.etudids_changed.length > 0) { + sco_message("enregistré"); + elem.className = "note_saved"; + // Il y avait une decision de jury ? + if (data.etudids_with_decision.includes(etudid)) { + if (v != $(elem).attr("data-orig-value")) { + $("#jurylink_" + etudid).html( + 'mettre à jour décision de jury' + ); + } else { + $("#jurylink_" + etudid).html(""); + } + } + // Mise à jour menu historique + if (data.history_menu[etudid]) { + $("#hist_" + etudid).html(data.history_menu[etudid]); + } + $(elem).attr("data-last-saved-value", v); + } + } + } catch (error) { + console.error("Fetch error:", error); + sco_message("Erreur réseau: valeur non enregistrée"); + } } function change_history(e) { - var opt = e.selectedOptions[0]; - var val = $(opt).attr("data-note"); - var etudid = $(e).attr('data-etudid'); - // le input associé a ce menu: - var input_elem = e.parentElement.parentElement.parentElement.childNodes[0]; - input_elem.value = val; - save_note(input_elem, val, etudid); + let opt = e.selectedOptions[0]; + let val = $(opt).attr("data-note"); + const etudid = parseInt($(e).attr("data-etudid")); + // le input associé a ce menu: + let input_elem = e.parentElement.parentElement.parentElement.childNodes[0]; + input_elem.value = val; + save_note(input_elem, val, etudid); } // Contribution S.L.: copier/coller des notes - function paste_text(e) { - var event = e.originalEvent; - event.stopPropagation(); - event.preventDefault(); - var clipb = e.originalEvent.clipboardData; - var data = clipb.getData('Text'); - var list = data.split(/\r\n|\r|\n|\t| /g); - var currentInput = event.currentTarget; - var masquerDEM = document.querySelector("body").classList.contains("masquer_DEM"); + var event = e.originalEvent; + event.stopPropagation(); + event.preventDefault(); + var clipb = e.originalEvent.clipboardData; + var data = clipb.getData("Text"); + var list = data.split(/\r\n|\r|\n|\t| /g); + var currentInput = event.currentTarget; + var masquerDEM = document + .querySelector("body") + .classList.contains("masquer_DEM"); - for (var i = 0; i < list.length; i++) { - currentInput.value = list[i]; - var evt = document.createEvent("HTMLEvents"); - evt.initEvent("blur", false, true); - currentInput.dispatchEvent(evt); - var sibbling = currentInput.parentElement.parentElement.nextElementSibling; - while ( - sibbling && - ( - sibbling.style.display == "none" || - ( - masquerDEM && sibbling.classList.contains("etud_dem") - ) - ) - ) { - sibbling = sibbling.nextElementSibling; - } - if (sibbling) { - currentInput = sibbling.querySelector("input"); - if (!currentInput) { - return; - } - } else { - return; - } + for (var i = 0; i < list.length; i++) { + currentInput.value = list[i]; + var evt = document.createEvent("HTMLEvents"); + evt.initEvent("blur", false, true); + currentInput.dispatchEvent(evt); + var sibbling = currentInput.parentElement.parentElement.nextElementSibling; + while ( + sibbling && + (sibbling.style.display == "none" || + (masquerDEM && sibbling.classList.contains("etud_dem"))) + ) { + sibbling = sibbling.nextElementSibling; } + if (sibbling) { + currentInput = sibbling.querySelector("input"); + if (!currentInput) { + return; + } + } else { + return; + } + } } function masquer_DEM() { - document.querySelector("body").classList.toggle("masquer_DEM"); + document.querySelector("body").classList.toggle("masquer_DEM"); } diff --git a/app/views/notes.py b/app/views/notes.py index aba053555a..70990ac60a 100644 --- a/app/views/notes.py +++ b/app/views/notes.py @@ -1875,12 +1875,6 @@ sco_publish( Permission.ScoEnsView, ) sco_publish("/saisie_notes", sco_saisie_notes.saisie_notes, Permission.ScoEnsView) -sco_publish( - "/save_note", - sco_saisie_notes.save_note, - Permission.ScoEnsView, - methods=["GET", "POST"], -) sco_publish( "/do_evaluation_set_missing", sco_saisie_notes.do_evaluation_set_missing, diff --git a/sco_version.py b/sco_version.py index f0a3ae1d44..8a435b8c5d 100644 --- a/sco_version.py +++ b/sco_version.py @@ -1,7 +1,7 @@ # -*- mode: python -*- # -*- coding: utf-8 -*- -SCOVERSION = "9.4.80" +SCOVERSION = "9.4.81" SCONAME = "ScoDoc" diff --git a/tests/api/test_api_evaluations.py b/tests/api/test_api_evaluations.py index 4a3064746c..9e1de1e235 100644 --- a/tests/api/test_api_evaluations.py +++ b/tests/api/test_api_evaluations.py @@ -24,7 +24,7 @@ from tests.api.setup_test_api import API_URL, CHECK_CERTIFICATE, api_headers from tests.api.tools_test_api import ( verify_fields, EVALUATIONS_FIELDS, - EVALUATION_FIELDS, + NOTES_FIELDS, ) @@ -35,7 +35,7 @@ def test_evaluations(api_headers): Route : - /moduleimpl//evaluations """ - moduleimpl_id = 1 + moduleimpl_id = 20 r = requests.get( f"{API_URL}/moduleimpl/{moduleimpl_id}/evaluations", headers=api_headers, @@ -44,6 +44,7 @@ def test_evaluations(api_headers): ) assert r.status_code == 200 list_eval = r.json() + assert list_eval assert isinstance(list_eval, list) for eval in list_eval: assert verify_fields(eval, EVALUATIONS_FIELDS) is True @@ -63,16 +64,14 @@ def test_evaluations(api_headers): assert eval["moduleimpl_id"] == moduleimpl_id -def test_evaluation_notes( - api_headers, -): # XXX TODO changer la boucle pour parcourir le dict sans les indices +def test_evaluation_notes(api_headers): """ Test 'evaluation_notes' Route : - /evaluation//notes """ - eval_id = 1 + eval_id = 20 r = requests.get( f"{API_URL}/evaluation/{eval_id}/notes", headers=api_headers, @@ -81,14 +80,15 @@ def test_evaluation_notes( ) assert r.status_code == 200 eval_notes = r.json() - for i in range(1, len(eval_notes)): - assert verify_fields(eval_notes[f"{i}"], EVALUATION_FIELDS) - assert isinstance(eval_notes[f"{i}"]["id"], int) - assert isinstance(eval_notes[f"{i}"]["etudid"], int) - assert isinstance(eval_notes[f"{i}"]["evaluation_id"], int) - assert isinstance(eval_notes[f"{i}"]["value"], float) - assert isinstance(eval_notes[f"{i}"]["comment"], str) - assert isinstance(eval_notes[f"{i}"]["date"], str) - assert isinstance(eval_notes[f"{i}"]["uid"], int) + assert eval_notes + for etudid, note in eval_notes.items(): + assert int(etudid) == note["etudid"] + assert verify_fields(note, NOTES_FIELDS) + assert isinstance(note["etudid"], int) + assert isinstance(note["evaluation_id"], int) + assert isinstance(note["value"], float) + assert isinstance(note["comment"], str) + assert isinstance(note["date"], str) + assert isinstance(note["uid"], int) - assert eval_id == eval_notes[f"{i}"]["evaluation_id"] + assert eval_id == note["evaluation_id"] diff --git a/tests/api/test_api_permissions.py b/tests/api/test_api_permissions.py index d2195e30f8..bb6ca6095d 100755 --- a/tests/api/test_api_permissions.py +++ b/tests/api/test_api_permissions.py @@ -58,6 +58,7 @@ def test_permissions(api_headers): "nip": 1, "partition_id": 1, "role_name": "Ens", + "start": "abc", "uid": 1, "version": "long", "assiduite_id": 1, diff --git a/tests/api/tools_test_api.py b/tests/api/tools_test_api.py index b277dd281b..b5aeae34f1 100644 --- a/tests/api/tools_test_api.py +++ b/tests/api/tools_test_api.py @@ -568,8 +568,7 @@ EVALUATIONS_FIELDS = { "visi_bulletin", } -EVALUATION_FIELDS = { - "id", +NOTES_FIELDS = { "etudid", "evaluation_id", "value", diff --git a/tests/unit/test_sco_basic.py b/tests/unit/test_sco_basic.py index 770fbf4e5d..5472c17b61 100644 --- a/tests/unit/test_sco_basic.py +++ b/tests/unit/test_sco_basic.py @@ -103,14 +103,14 @@ def run_sco_basic(verbose=False) -> FormSemestre: # --- Saisie toutes les notes de l'évaluation for idx, etud in enumerate(etuds): - nb_changed, nb_suppress, existing_decisions = G.create_note( + etudids_changed, nb_suppress, existing_decisions = G.create_note( evaluation_id=e["id"], etudid=etud["etudid"], note=NOTES_T[idx % len(NOTES_T)], ) assert not existing_decisions assert nb_suppress == 0 - assert nb_changed == 1 + assert len(etudids_changed) == 1 # --- Vérifie que les notes sont prises en compte: b = sco_bulletins.formsemestre_bulletinetud_dict(formsemestre_id, etud["etudid"]) @@ -136,7 +136,7 @@ def run_sco_basic(verbose=False) -> FormSemestre: ) # Saisie les notes des 5 premiers étudiants: for idx, etud in enumerate(etuds[:5]): - nb_changed, nb_suppress, existing_decisions = G.create_note( + etudids_changed, nb_suppress, existing_decisions = G.create_note( evaluation_id=e2["id"], etudid=etud["etudid"], note=NOTES_T[idx % len(NOTES_T)], @@ -158,7 +158,7 @@ def run_sco_basic(verbose=False) -> FormSemestre: # Saisie des notes qui manquent: for idx, etud in enumerate(etuds[5:]): - nb_changed, nb_suppress, existing_decisions = G.create_note( + etudids_changed, nb_suppress, existing_decisions = G.create_note( evaluation_id=e2["id"], etudid=etud["etudid"], note=NOTES_T[idx % len(NOTES_T)], diff --git a/tools/fakedatabase/create_test_api_database.py b/tools/fakedatabase/create_test_api_database.py index 1825a884fa..673df6ca42 100644 --- a/tools/fakedatabase/create_test_api_database.py +++ b/tools/fakedatabase/create_test_api_database.py @@ -242,7 +242,7 @@ def create_evaluations(formsemestre: FormSemestre): "jour": datetime.date(2022, 3, 1) + datetime.timedelta(days=modimpl.id), "heure_debut": "8h00", "heure_fin": "9h00", - "description": None, + "description": f"Evaluation-{modimpl.module.code}", "note_max": 20, "coefficient": 1.0, "visibulletin": True,