[intégrée] Prop0034 - Supprimer le répertoire scr/

Statut

Intégrée dans la version 4.7.0

Idée générale

Toujours dans mon idée plus large de supprimer les EXTERNALS, voici une POC pour la suppression du répertoire scr/.

J’ai donc créé une branche à partir de la branche 4.7.0 : https://adullact.net/scm/viewvc.php/openmairie/openmairie_exemple/branches/poc-remove-code-from-scr/

  • Faire une modification la moins impactant possible pour l’existant.
  • Les routes (urls) sont définies dans des propriétés de la classe application et sont donc surchargeables.
  • Ajout d’une constante par ROUTE, les constantes sont définies à l’instanciation de la classe application.
  • Utilisation de la constante partout.

Points déjà réalisés

Points problématiques

  • Dans les fichiers javascript js/*.js on trouve des appels à des URLs en dur vers scr/form.php, il en reste trois qui ne sont pas vus par les tests :

    • js/layout_jqueryui_after.js:
      1473: $.getJSON( “…/scr/form.php?obj=om_sousetat&idx=0&action=5”, function( data ) {
      => Dans la barre d’outils de tinymce on récupère la liste des sous-états que l’on peut insérer
      => Vérifier si il est possible que l’URL provienne du DOM comme ça a été fait pour la composition du tableau de bord
      => Pas de test ? aucune des fonctions de la barre d’outil de tinymce n’est testée via robotframework pour des contraintes techniques

    • js/layout_jqueryui_after.js:
      1930: url: “…/scr/form.php?obj=om_requete&idx=”+field_select_type_value+"&action=4&contentonly=true",
      => Dans le formulaire de lettre type/état lorsqu’on sélectionne une requête (champ om_sql) une aide à la saisie permet d’afficher l’aide à la saisie des champs de fusion
      => Vérifier si il est possible que l’URL provienne du DOM comme ça a été fait pour la composition du tableau de bord
      => Pas de test ? non il faut en ajouter un

    • /home/flo/liger_html/framework-openmairie/BR-4.7.0-poc-remove-code-from-scr/js/sig.js:
      772: link = ‘…/scr/form_sig.php?obj=’+obj+’&idx=’+idx_sel+’&etendue=’+etendue+’&reqmo=’+reqmo+
      => Dans le module SIG mais je n’ai pas vérifié où
      => Vérifier si il est possible que l’URL provienne du DOM comme ça a été fait pour la composition du tableau de bord
      => Pas de test ? non aucune des fonctions de géolocalisation n’est testée, il va falloir attendre la prop0026 pour avoir des tests sur ce point : [intégrée] Prop0026 - Fiabiliser / pérenniser le module SIG

Points à améliorer

  • Le terme module (index.php?module=login) a été choisi de manière assez arbitraire, si il y a d’autres propositions on est toujours à temps de changer.
  • La méthode application::view_main() devrait s’appuyer sur le tableau $routes_map défini dans application::init_routes()

Rétrocompatibilité envisagée

L’objectif est de supprimer complètement le répertoire scr/ du framework, il ne sera donc plus disponible du tout pour être récupérer en EXTERNALS via subversion.

Les applicatifs pourront soit faire la mise à jour soit conserver leur état actuel en :

  • ajoutant le répertoire scr/ directement dans leur SCM

  • surchargeant les propriétés suivantes dans obj/utils.class.php :

     protected $route__dashboard = "../scr/dashboard.php?";
     protected $route__login = "../scr/login.php?";
     protected $route__logout = "../scr/logout.php?";
     protected $route__password = "../scr/password.php?";
     protected $route__password_reset = "../scr/login.php?mode=password_reset";
     protected $route__tab = "../scr/tab.php?";
     protected $route__soustab = "../scr/soustab.php?";
     protected $route__form = "../scr/form.php?";
     protected $route__sousform = "../scr/sousform.php?";
     protected $route__tab_sig = "../scr/tab_sig.php?";
     protected $route__form_sig = "../scr/form_sig.php?";
     protected $route__module_edition = "../scr/edition.php?";
     protected $route__module_import = "../scr/import.php?";
     protected $route__module_reqmo = "../scr/reqmo.php?";
     protected $route__module_gen = "../scr/gen.php?";
    
  • surchargeant les variables suivantes dans tests/resources/resources.robot :

    ${OM_ROUTE_DASHBOARD} scr/dashboard.php?
    ${OM_ROUTE_LOGIN} scr/login.php?
    ${OM_ROUTE_LOGOUT} scr/logout.php?
    ${OM_ROUTE_PASSWORD} scr/password.php?
    ${OM_ROUTE_PASSWORD_RESET} scr/login.php?mode=password_reset
    ${OM_ROUTE_TAB} scr/tab.php?
    ${OM_ROUTE_SOUSTAB} scr/soustab.php?
    ${OM_ROUTE_FORM} scr/form.php?
    ${OM_ROUTE_SOUSFORM} scr/sousform.php?
    ${OM_ROUTE_FORM_SIG} scr/form_sig.php?
    ${OM_ROUTE_TAB_SIG} scr/tab_sig.php?
    ${OM_ROUTE_MODULE_EDITION} scr/edition.php?
    ${OM_ROUTE_MODULE_GEN} scr/gen.php?
    ${OM_ROUTE_MODULE_REQMO} scr/reqmo.php?
    ${OM_ROUTE_MODULE_IMPORT} scr/import.php?
    ${OM_PDF_TITLE} form

  • en remplaçant les anciennes propriétés éventuellement surchargées :

    application::$url_dashboard devient application::$route__dashboard
    application::$url_password_reset devient application::$route__password_reset

Bonjour,

Concernant le point problématique :

une proposition de correction a été effectuée dont le commit est visible ici : https://adullact.net/scm/viewvc.php/openmairie?view=revision&revision=3934
Il manque tout de même le test automatique.

Cordialement,
Sofien Timezouaght.

edit: Proposition sur les tests à 80% hors sujet car elle dépasse le cadre de cette POC.

Archivée ici:
https://framagit.org/snippets/620

Je ne suis pas sûr de ce que tu proposes mais il me semble que c’est un peu hors sujet ici. Ce sujet concerne la construction d’une POC pour supprimer le répertoire scr/ du framework. Lors du développement de cette POC, on se rend compte que trois points ne sont plus fonctionnels et que les tests ne couvrent pas ces trois points. Il faut donc ajouter des tests dans le framework pour couvrir ces trois points.

Ceci dit, si tu as des propositions sur les stratégies de tests des versions du framework ou des fonctionnalités du framework, ou encore sur la séparation de couverture de tests entre applicatif et framework, je t’invite à créer un sujet dédié pour en discuter.

Ça a commencé par adresser cette question pour faire avancer la POC (Stratégie 1) mais ça a dérivé sur tout un brainstorming sur le sujet.

Le corps du message a été archivé afin de garder ce fil de discussion lisible.

Bonjour,

Concernant le point problématique:

Il a été traité sur le modèle de la proposition de @stimezouaght

Avec pour divergence d’avoir laissé &obj=om_sousetat&idx=0&action=5 dans le JS.
@stimezouaght tu peux confirmer si ça ne pose pas de problèmes de ne passer que OM_ROUTE_FORM au JS?
Et si du coup on peut fusionner les parties PHP de nos commits?

https://adullact.net/scm/viewvc.php/openmairie?view=revision&revision=3946
Il manque aussi le test automatique.

Tests manuels:
Après le passage des tests automatisés.
Aller modifier un etat et une lettre type quelconques. Et pour les deux:
Aller dans l’éditeur riche du corps.
Voir que l’on peut insérer un sous état.

@stimezouaght
C’est volontaire d’avoir mis la variable labels_merge_fields_href_base en globale? https://adullact.net/scm/viewvc.php/openmairie/openmairie_exemple/branches/poc-remove-code-from-scr/js/layout_jqueryui_after.js?r1=3934&r2=3933&pathrev=3934#l1928

@tuxayo
Victor, il serait préférable de gérer les URL en dehors du Javascript donc de lui passer le reste en même temps que OM_ROUTE_FORM.

Concernant la variable déclarée de façon implicite et donc globale, c’est une erreur de ma part.

1 « J'aime »

@stimezouaght Corrections faites.
Le commentaire et l’id choisis sont peut-être douteux.

https://adullact.net/scm/viewvc.php/openmairie?view=revision&revision=3950
https://adullact.net/scm/viewvc.php/openmairie?view=revision&revision=3951

Ajout d’un test sur le chargement des champs de fusion dans l’aide à la saisie des lettres types et des états.
https://adullact.net/scm/viewvc.php/openmairie?view=revision&revision=3953

1 « J'aime »

Point numéro 3:

Après avec commencé à regarder, je suis bloqué par le fait de ne pas savoir comment tester manuellement ça. Quelqu’un sais comment mettre en action ce code ?

Après un point avec @flohcim, voilà comment reproduire le bug n°3 (sig.js:772) affectant cette POC:
Lancer les tests 010_module_sig_interne.robot
Ajouter le paramètre option_localisation=sig_interne
Aller dans administration → om_sig_map
Cliquer sur testcarte01
Paramètres cartographiques → Centrer sur le point (si différent du centre de l’étendue) (bouton à droit de la page qui ressemble à ^)
Ce qui ouvre une pop up
Outils → Editer → Enregistrer
Ce qui déclenche une requête sur /scr/form_sig.php?obj=om_sig_map[...] qui revient en 404 quand on regarde le moniteur réseau du navigateur

edit: ajout étape lancement des tests

Cool! J’ai mergé ton commit dans la branche 4.7.0 car le test n’est pas lié à la POC https://adullact.net/scm/viewvc.php/openmairie?view=revision&revision=3954

bug n°3 (sig.js:772):
POC de fix: https://adullact.net/scm/viewvc.php/openmairie?view=revision&revision=3974

bug n°2 (js/layout_jqueryui_after.js:1473)
Correction commentaire obsolète dans layout_jqueryui_after.js
https://adullact.net/scm/viewvc.php/openmairie?view=revision&revision=3975

Tout les 3 points ont été corrigés.
Le.s.quel.s faut-il/peut-on tester ? Le 2e ? (js/layout_jqueryui_after.js:1930)

La POC est satisfaisante, je la transforme en prop.

Documentation de la mise à niveau : http://openmairie.readthedocs.io/projects/omframework/fr/4.7/upgrades/v4.7.html#gestion-de-la-suppression-des-repertoires-scr-et-spg

https://adullact.net/tracker/index.php?func=detail&aid=8966&group_id=265&atid=1999