From b728e06f272a3fee95682864de0b7c036fe6dca3 Mon Sep 17 00:00:00 2001
From: Emmanuel Viennet <emmanuel.viennet@gmail.com>
Date: Thu, 23 Feb 2023 21:19:57 +0100
Subject: [PATCH] =?UTF-8?q?Cr=C3=A9ation=20de=20nouvelles=20versions=20de?=
 =?UTF-8?q?=20formations:=20am=C3=A9lioration=20dialogue,=20propose=20syst?=
 =?UTF-8?q?=C3=A9matriquement=20d'embarquer=20des=20formsemestres?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 app/scodoc/sco_edit_formation.py      |   6 +-
 app/scodoc/sco_edit_ue.py             |  33 +++--
 app/scodoc/sco_formations.py          |  13 +-
 app/scodoc/sco_formsemestre_edit.py   | 178 +++++++++++++++-----------
 app/scodoc/sco_formsemestre_status.py |   5 +-
 app/scodoc/sco_utils.py               |  30 ++---
 app/static/css/scodoc.css             |   4 -
 app/views/notes.py                    |  12 +-
 tests/unit/test_formsemestre.py       |   4 +-
 9 files changed, 167 insertions(+), 118 deletions(-)

diff --git a/app/scodoc/sco_edit_formation.py b/app/scodoc/sco_edit_formation.py
index 8e75e4a0d..384ec0650 100644
--- a/app/scodoc/sco_edit_formation.py
+++ b/app/scodoc/sco_edit_formation.py
@@ -68,10 +68,12 @@ def formation_delete(formation_id=None, dialog_confirmed=False):
         )
         for sem in sems:
             H.append(
-                '<li><a href="formsemestre_status?formsemestre_id=%(formsemestre_id)s">%(titremois)s</a></li>'
+                '<li><a class="stdlink" href="formsemestre_status?formsemestre_id=%(formsemestre_id)s">%(titremois)s</a></li>'
                 % sem
             )
-        H.append('</ul><p><a href="%s">Revenir</a></p>' % scu.NotesURL())
+        H.append(
+            '</ul><p><a class="stdlink" href="%s">Revenir</a></p>' % scu.NotesURL()
+        )
     else:
         if not dialog_confirmed:
             return scu.confirm_dialog(
diff --git a/app/scodoc/sco_edit_ue.py b/app/scodoc/sco_edit_ue.py
index 9f4900bc4..8fe5fd021 100644
--- a/app/scodoc/sco_edit_ue.py
+++ b/app/scodoc/sco_edit_ue.py
@@ -118,7 +118,7 @@ def do_ue_create(args):
             f"""Acronyme d'UE "{args['acronyme']}" déjà utilisé ! 
                 (chaque UE doit avoir un acronyme unique dans la formation)"""
         )
-    if not "ue_code" in args:
+    if (not "ue_code" in args) or (not args["ue_code"].strip()):
         # évite les conflits de code
         while True:
             cursor = db.session.execute("select notes_newid_ucod();")
@@ -405,6 +405,7 @@ def ue_edit(ue_id=None, create=False, formation_id=None, default_semestre_idx=No
                 "explanation": """code interne (non vide). Toutes les UE partageant le même code 
                 (et le même code de formation) sont compatibles (compensation de semestres, capitalisation d'UE). 
                 Voir liste ci-dessous.""",
+                "allow_null": False,
             },
         ),
         (
@@ -663,6 +664,13 @@ def ue_table(formation_id=None, semestre_idx=1, msg=""):  # was ue_list
     ues_externes_obj = UniteEns.query.filter_by(
         formation_id=formation_id, is_external=True
     )
+    # liste ordonnée des formsemestres de cette formation:
+    formsemestres = sorted(
+        FormSemestre.query.filter_by(formation_id=formation_id).all(),
+        key=lambda s: s.sort_key(),
+        reverse=True,
+    )
+
     if is_apc:
         # Pour faciliter la transition des anciens programmes non APC
         for ue in ues_obj:
@@ -901,18 +909,29 @@ du programme" (menu "Semestre") si vous avez un semestre en cours);
         """
         )
     H.append("<p><ul>")
-    if editable:
+    if has_perm_change:
         H.append(
             f"""
             <li><a class="stdlink" href="{
-            url_for('notes.formation_create_new_version', 
+            url_for('notes.formsemestre_associate_new_version', 
             scodoc_dept=g.scodoc_dept, formation_id=formation_id
             )
-            }">Créer une nouvelle version (non verrouillée)</a>
+            }">Créer une nouvelle version de la formation</a> (copie non verrouillée)
             </li>
             
         """
         )
+        if not len(formsemestres):
+            H.append(
+                f"""
+            <li><a class="stdlink" href="{
+            url_for('notes.formation_delete', 
+            scodoc_dept=g.scodoc_dept, formation_id=formation_id
+            )
+            }">Supprimer cette formation</a> (pas encore utilisée par des semestres)
+            </li>
+            """
+            )
     H.append(
         f"""
         <li><a class="stdlink" href="{
@@ -951,11 +970,7 @@ du programme" (menu "Semestre") si vous avez un semestre en cours);
         <h3> <a name="sems">Semestres ou sessions de cette formation</a></h3>
         <p><ul>"""
         )
-        for formsemestre in sorted(
-            FormSemestre.query.filter_by(formation_id=formation_id).all(),
-            key=lambda s: s.sort_key(),
-            reverse=True,
-        ):
+        for formsemestre in formsemestres:
             H.append(
                 f"""<li><a class="stdlink" href="{
                     url_for("notes.formsemestre_status", scodoc_dept=g.scodoc_dept,
diff --git a/app/scodoc/sco_formations.py b/app/scodoc/sco_formations.py
index 29103d94c..6d167afa2 100644
--- a/app/scodoc/sco_formations.py
+++ b/app/scodoc/sco_formations.py
@@ -474,22 +474,23 @@ def formation_list_table() -> GenTable:
             FormSemestre.date_debut
         ).all()
         row["sems_list_txt"] = ", ".join(s.session_id() for s in row["formsemestres"])
-        row["_sems_list_txt_html"] = (
-            ", ".join(
+        row["_sems_list_txt_html"] = ", ".join(
+            [
                 f"""<a class="discretelink" href="{
                 url_for("notes.formsemestre_status",
                     scodoc_dept=g.scodoc_dept, formsemestre_id=s.id
-                )}">{s.session_id()}</a>
-            """
+                )}">{s.session_id()}</a>"""
                 for s in row["formsemestres"]
-            )
-            + f""", <a class="stdlink" id="add-semestre-{
+            ]
+            + [
+                f"""<a class="stdlink" id="add-semestre-{
                 formation.acronyme.lower().replace(" ", "-")}"
                 href="{ url_for("notes.formsemestre_createwithmodules",
                 scodoc_dept=g.scodoc_dept, formation_id=formation.id, semestre_id=1
                 )
                 }">ajouter</a>
             """
+            ]
         )
         if row["formsemestres"]:
             row["date_fin_dernier_sem"] = (
diff --git a/app/scodoc/sco_formsemestre_edit.py b/app/scodoc/sco_formsemestre_edit.py
index 3280b1be5..cd231467b 100644
--- a/app/scodoc/sco_formsemestre_edit.py
+++ b/app/scodoc/sco_formsemestre_edit.py
@@ -1285,107 +1285,136 @@ def do_formsemestre_clone(
 
 
 def formsemestre_associate_new_version(
-    formsemestre_id,
-    other_formsemestre_ids=[],
-    dialog_confirmed=False,
+    formation_id: int,
+    formsemestre_id: int = None,
+    other_formsemestre_ids: list[int] = None,
 ):
-    """Formulaire changement formation d'un semestre"""
-    formsemestre_id = int(formsemestre_id)
-    other_formsemestre_ids = [int(x) for x in other_formsemestre_ids]
-    if not dialog_confirmed:
-        # dresse le liste des semestres de la meme formation et version
-        formsemestre: FormSemestre = FormSemestre.query.get_or_404(formsemestre_id)
-        othersems = sco_formsemestre.do_formsemestre_list(
-            args={
-                "formation_id": formsemestre.formation.id,
-                "version": formsemestre.formation.version,
-                "etat": "1",
-            },
+    """Formulaire nouvelle version formation et association d'un ou plusieurs formsemestre.
+    formation_id: la formation à dupliquer
+    formsemestre_id: optionnel, formsemestre de départ, qui sera associé à la noiuvelle version
+    """
+    if formsemestre_id is not None:
+        formsemestre_id = int(formsemestre_id)
+    formation: Formation = Formation.query.get_or_404(formation_id)
+    other_formsemestre_ids = {int(x) for x in other_formsemestre_ids or []}
+    if request.method == "GET":
+        # dresse la liste des semestres non verrouillés de la même formation
+        other_formsemestres: list[FormSemestre] = formation.formsemestres.filter_by(
+            etat=True
         )
+
         H = []
-        for s in othersems:
-            if (
-                s["formsemestre_id"] == formsemestre_id
-                or s["formsemestre_id"] in other_formsemestre_ids
-            ):
-                checked = 'checked="checked"'
-            else:
-                checked = ""
-            if s["formsemestre_id"] == formsemestre_id:
-                disabled = 'disabled="1"'
-            else:
-                disabled = ""
+        for other_formsemestre in other_formsemestres:
+            checked = (
+                'checked="checked"'
+                if (
+                    other_formsemestre.id == formsemestre_id
+                    or other_formsemestre.id in other_formsemestre_ids
+                )
+                else ""
+            )
+            disabled = (
+                'disabled="1"' if other_formsemestre.id == formsemestre_id else ""
+            )
+
             H.append(
                 f"""<div><input type="checkbox" name="other_formsemestre_ids:list"
-                value="{s['formsemestre_id']}" {checked} {disabled}
-                >{s['titremois']}</input></div>"""
+                value="{other_formsemestre.id}" {checked} {disabled}
+                ><a class="stdlink" href="{
+                    url_for("notes.formsemestre_status", 
+                        scodoc_dept=g.scodoc_dept, formsemestre_id=other_formsemestre.id)
+                }">{other_formsemestre.titre_mois()}</a></input></div>"""
+            )
+        if formsemestre_id is None:
+            cancel_url = url_for(
+                "notes.ue_table", scodoc_dept=g.scodoc_dept, formation_id=formation.id
+            )
+        else:
+            cancel_url = url_for(
+                "notes.formsemestre_status",
+                scodoc_dept=g.scodoc_dept,
+                formsemestre_id=formsemestre_id,
             )
 
         return scu.confirm_dialog(
-            f"""<h2>Associer à une nouvelle version de formation non verrouillée ?</h2>
+            (
+                """<h2>Associer à une nouvelle version de formation non verrouillée ?</h2>"""
+                if formsemestre_id
+                else """<h2>Créer une nouvelle version de la formation ?</h2>"""
+            )
+            + f"""<p><b>Formation: </b><a class="stdlink" href="{
+                url_for("notes.ue_table", scodoc_dept=g.scodoc_dept, formation_id=formation.id)
+                }">{formation.titre} version {formation.version}</a></p>
+            
                 <p class="help">Le programme pédagogique ("formation") va être dupliqué
-                pour que vous puissiez le modifier sans affecter les autres
-                semestres. Les autres paramètres (étudiants, notes...) du
-                semestre seront inchangés.
+                pour que vous puissiez le modifier sans affecter les semestres déjà terminés. 
                 </p>
                 <p class="help">Veillez à ne pas abuser de cette possibilité, car créer
                 trop de versions de formations va vous compliquer la gestion
                 (à vous de garder trace des différences et à ne pas vous
                 tromper par la suite...).
                 </p>
-                <p class="help">Si vous souhaitez créer un programme pour de futurs semestres,
-                utilisez plutôt <a class="stdlink" href="{
-                    url_for('notes.formation_create_new_version',
-                        scodoc_dept=g.scodoc_dept, formation_id=formsemestre.formation.id
-                )}">Créer une nouvelle version</a>.
-                </p>
                 <div class="othersemlist">
-                <p>Si vous voulez associer aussi d'autres semestres à la nouvelle
-                version, cochez-les:
+                <p>Si vous voulez associer des semestres à la nouvelle
+                version, cochez-les maintenant <br>
+                (<b>attention&nbsp;: vous ne pourrez pas le faire plus tard car on ne peut pas 
+                changer la formation d'un semestre !</b>):
                 </p>"""
             + "".join(H)
+            + """<p>Les données (étudiants, notes...) de ces semestres seront inchangées.</p>"""
             + "</div>",
-            OK="Associer ces semestres à une nouvelle version",
+            OK="Créer une nouvelle version et y associer ces semestres",
             dest_url="",
-            cancel_url=url_for(
-                "notes.formsemestre_status",
-                scodoc_dept=g.scodoc_dept,
-                formsemestre_id=formsemestre_id,
-            ),
-            parameters={"formsemestre_id": formsemestre_id},
+            cancel_url=cancel_url,
+            parameters={"formation_id": formation_id},
         )
-    else:
-        do_formsemestres_associate_new_version(
-            [formsemestre_id] + other_formsemestre_ids
+    elif request.method == "POST":
+        if formsemestre_id is not None:  # pas dans le form car checkbox disabled
+            other_formsemestre_ids |= {formsemestre_id}
+        new_formation_id = do_formsemestres_associate_new_version(
+            formation_id, other_formsemestre_ids
         )
-        flash("Semestre associé à une nouvelle version de la formation")
-        return flask.redirect(
-            url_for(
-                "notes.formsemestre_status",
-                scodoc_dept=g.scodoc_dept,
-                formsemestre_id=formsemestre_id,
+        flash(
+            "Nouvelle version de la formation créée"
+            + (" et semestres associés." if other_formsemestre_ids else ".")
+        )
+        if formsemestre_id is None:
+            return flask.redirect(
+                url_for(
+                    "notes.ue_table",
+                    scodoc_dept=g.scodoc_dept,
+                    formation_id=new_formation_id,
+                )
             )
-        )
+        else:
+            return flask.redirect(
+                url_for(
+                    "notes.formsemestre_status",
+                    scodoc_dept=g.scodoc_dept,
+                    formsemestre_id=formsemestre_id,
+                )
+            )
+    else:
+        raise ScoValueError("Méthode invalide")
 
 
-def do_formsemestres_associate_new_version(formsemestre_ids):
-    """Cree une nouvelle version de la formation du semestre, et y rattache les semestres.
+def do_formsemestres_associate_new_version(
+    formation_id: int, formsemestre_ids: list[int]
+) -> int:
+    """Crée une nouvelle version de la formation du semestre, et y rattache les semestres.
     Tous les moduleimpl sont ré-associés à la nouvelle formation, ainsi que les decisions de jury
     si elles existent (codes d'UE validées).
-    Les semestre doivent tous appartenir à la meme version de la formation
+    Les semestre doivent tous appartenir à la meme version de la formation.
+    renvoie l'id de la nouvelle formation.
     """
-    log(f"do_formsemestres_associate_new_version {formsemestre_ids}")
-    if not formsemestre_ids:
-        return
-    # Check: tous de la même formation
-    assert isinstance(formsemestre_ids[0], int)
-    sem = sco_formsemestre.get_formsemestre(formsemestre_ids[0])
-    formation_id = sem["formation_id"]
-    for formsemestre_id in formsemestre_ids[1:]:
-        assert isinstance(formsemestre_id, int)
-        sem = sco_formsemestre.get_formsemestre(formsemestre_id)
-        if formation_id != sem["formation_id"]:
-            raise ScoValueError("les semestres ne sont pas tous de la même formation !")
+    log(f"do_formsemestres_associate_new_version {formation_id} {formsemestre_ids}")
+
+    # Check: tous les semestre de la formation
+    formsemestres = [FormSemestre.query.get_or_404(i) for i in formsemestre_ids]
+    if not all(
+        [formsemestre.formation_id == formation_id for formsemestre in formsemestres]
+    ):
+        raise ScoValueError("les semestres ne sont pas tous de la même formation !")
 
     cnx = ndb.GetDBConnexion()
     # New formation:
@@ -1414,6 +1443,7 @@ def do_formsemestres_associate_new_version(formsemestre_ids):
         _reassociate_moduleimpls(cnx, formsemestre_id, ues_old2new, modules_old2new)
 
     cnx.commit()
+    return formation_id
 
 
 def _reassociate_moduleimpls(cnx, formsemestre_id, ues_old2new, modules_old2new):
@@ -1676,7 +1706,7 @@ def formsemestre_change_publication_bul(
             msg = ""
         return scu.confirm_dialog(
             "<h2>Confirmer la %s publication des bulletins ?</h2>" % msg,
-            helpmsg="""Il est parfois utile de désactiver la diffusion des bulletins,
+            help_msg="""Il est parfois utile de désactiver la diffusion des bulletins,
             par exemple pendant la tenue d'un jury ou avant harmonisation des notes.
             <br>
             Ce réglage n'a d'effet que si votre établissement a interfacé ScoDoc et un portail étudiant.
diff --git a/app/scodoc/sco_formsemestre_status.py b/app/scodoc/sco_formsemestre_status.py
index 32478d0b5..279ab5542 100644
--- a/app/scodoc/sco_formsemestre_status.py
+++ b/app/scodoc/sco_formsemestre_status.py
@@ -236,7 +236,10 @@ def formsemestre_status_menubar(formsemestre: FormSemestre) -> str:
         {
             "title": "Associer à une nouvelle version du programme",
             "endpoint": "notes.formsemestre_associate_new_version",
-            "args": {"formsemestre_id": formsemestre_id},
+            "args": {
+                "formsemestre_id": formsemestre_id,
+                "formation_id": formsemestre.formation_id,
+            },
             "enabled": current_user.has_permission(Permission.ScoChangeFormation)
             and formsemestre.etat,
             "helpmsg": "",
diff --git a/app/scodoc/sco_utils.py b/app/scodoc/sco_utils.py
index 6ff484ca8..dc4fb9772 100644
--- a/app/scodoc/sco_utils.py
+++ b/app/scodoc/sco_utils.py
@@ -1075,16 +1075,18 @@ def query_portal(req, msg="Portail Apogee", timeout=3):
 def confirm_dialog(
     message="<p>Confirmer ?</p>",
     OK="OK",
-    Cancel="Annuler",
-    dest_url="",
-    cancel_url="",
-    target_variable="dialog_confirmed",
-    parameters={},
     add_headers=True,  # complete page
-    helpmsg=None,
+    cancel_label="Annuler",
+    cancel_url="",
+    dest_url="",
+    help_msg=None,
+    parameters: dict = None,
+    target_variable="dialog_confirmed",
 ):
+    """HTML confirmation dialog: submit (POST) to same page or dest_url if given."""
     from app.scodoc import html_sco_header
 
+    parameters = parameters or {}
     # dialog de confirmation simple
     parameters[target_variable] = 1
     # Attention: la page a pu etre servie en GET avec des parametres
@@ -1105,24 +1107,22 @@ def confirm_dialog(
         H.append(f'<input type="submit" value="{OK}"/>')
     if cancel_url:
         H.append(
-            """<input type ="button" value="%s"
-            onClick="document.location='%s';"/>"""
-            % (Cancel, cancel_url)
+            f"""<input type ="button" value="{cancel_label}"
+            onClick="document.location='{cancel_url}';"/>"""
         )
     for param in parameters.keys():
         if parameters[param] is None:
             parameters[param] = ""
-        if type(parameters[param]) == type([]):
+        if isinstance(parameters[param], list):
             for e in parameters[param]:
-                H.append('<input type="hidden" name="%s" value="%s"/>' % (param, e))
+                H.append(f"""<input type="hidden" name="{param}" value="{e}"/>""")
         else:
             H.append(
-                '<input type="hidden" name="%s" value="%s"/>'
-                % (param, parameters[param])
+                f"""<input type="hidden" name="{param}" value="{parameters[param]}"/>"""
             )
     H.append("</form>")
-    if helpmsg:
-        H.append('<p class="help">' + helpmsg + "</p>")
+    if help_msg:
+        H.append('<p class="help">' + help_msg + "</p>")
     if add_headers:
         return (
             html_sco_header.sco_header() + "\n".join(H) + html_sco_header.sco_footer()
diff --git a/app/static/css/scodoc.css b/app/static/css/scodoc.css
index e1e3d0b83..c0830343c 100644
--- a/app/static/css/scodoc.css
+++ b/app/static/css/scodoc.css
@@ -3622,10 +3622,6 @@ div.othersemlist {
   border: 1px solid gray;
 }
 
-div.othersemlist p {
-  font-weight: bold;
-  margin-top: 0px;
-}
 
 div.othersemlist input {
   margin-left: 20px;
diff --git a/app/views/notes.py b/app/views/notes.py
index 2bf9b1723..13f971639 100644
--- a/app/views/notes.py
+++ b/app/views/notes.py
@@ -738,11 +738,11 @@ def formation_import_xml_form():
         """
 
 
-sco_publish(
-    "/formation_create_new_version",
-    sco_formations.formation_create_new_version,
-    Permission.ScoChangeFormation,
-)
+# sco_publish(
+#     "/formation_create_new_version",
+#     sco_formations.formation_create_new_version,
+#     Permission.ScoChangeFormation,
+# )
 
 # --- UE
 sco_publish(
@@ -838,7 +838,7 @@ def formsemestre_flip_lock(formsemestre_id, dialog_confirmed=False):
         msg = "verrouillage" if formsemestre.etat else "déverrouillage"
         return scu.confirm_dialog(
             f"<h2>Confirmer le {msg} du semestre ?</h2>",
-            helpmsg="""Les notes d'un semestre verrouillé ne peuvent plus être modifiées.
+            help_msg="""Les notes d'un semestre verrouillé ne peuvent plus être modifiées.
             Un semestre verrouillé peut cependant être déverrouillé facilement à tout moment
             (par son responsable ou un administrateur).
             <br>
diff --git a/tests/unit/test_formsemestre.py b/tests/unit/test_formsemestre.py
index ee2566953..4ca9a551a 100644
--- a/tests/unit/test_formsemestre.py
+++ b/tests/unit/test_formsemestre.py
@@ -105,7 +105,9 @@ def test_formsemestre_misc_views(test_client):
     assert isinstance(ans, (str, Response))  # ici str
     # Juste la page dialogue avant opération::
     ans = sco_formsemestre_edit.formsemestre_clone(formsemestre.id)
-    ans = sco_formsemestre_edit.formsemestre_associate_new_version(formsemestre.id)
+    ans = sco_formsemestre_edit.formsemestre_associate_new_version(
+        formsemestre.formation_id, formsemestre.id
+    )
     ans = sco_formsemestre_edit.formsemestre_delete(formsemestre.id)
 
     # ----- MENU INSCRIPTIONS