[TECH] Montée de version d'Ember pour Pix Certif de la 3.18 à 3.21 (PIX-1302)

[jbuget]

🦄 Contexte & enjeu

La version d'Ember pour Pix Certif est la 3.18. La version actuelle d'Ember est la 3.21. La 3.22 est sur le point de sortir.

L'objet de cette PR est de se mettre à jour pour bénéficier des dernières nouveautés d'Ember, Ember Data et Ember CLI et se simplifier la vie pour les prochaines montées de version.

🤖 Méthodologie

Nous sommes passés par l'outil Node ember-cli-update, que nous avons installé en global (d'habitude, c'et plutôt le mal, mais en l'occurrence, là c'était très pratique).

Pour rappel : la LTS actuelle est la 3.20. La version précédente était la 3.16. Nous n'étions donc pas sur une LTS depuis plusieurs mois. Est-ce grave ? pas forcément…

Nous avons tenté une première approche un peu "bourrine", en passant directement de la 3.18 à la 3.21. Nous avons rencontré pas mal de warnings, erreurs et tests en échec.

Nous avons alors décidé d'y aller de façon incrémentale : 3.18 → 3.19 → 3.20 → 3.21.

3.18 → 3.19

Problème de règle ESLint ember/no-mixins

La montée de version d'Ember (ember-source + ember-data + ember-cli) s'accompagne, via ember-cli-update, de la montée de version d'ESLint et du plugin eslint-plugin-ember. Ce dernier contient de nouvelles règles, dont ember/no-mixins.

Les seuls endroits dans le code de Pix Certif où nous utilisons encore des mixins sont dans la déclaration de routes, via ESA (Ember Simple Auth). Un patch existe à venir côté ESA (dans la v3.1.0-beta.0), mais il n'a pas encore été livré en production, en version finale.

Remarque : la montée de version d'ESA en v3.1.0 s'annonce comme un chantier conséquent à part entière.

Nous avons décidé, une fois n'est pas coutume, de désactiver la règle de linting, non pas de façon globale (on se respecte encore un petit peu), mais localement pour chaque route.

Nouveaux imports @ember-data

Dans la continuité des nouveautés ESLint, il a fallu ajuster les imports de propriétés ou classes d'Ember Data pour utiliser le système de @namespace NPM.

Avant :

// certif/app/models/user.js
import DS from 'ember-data';
const { Model, attr, hasMany } = DS;

Après :

// certif/app/models/user.js
import Model, { attr, hasMany } from '@ember-data/model';

Utilisation des nouveaux getters ES6

Avant :

// certif/app/routes/logout.js
beforeModel() {
    super.beforeModel(...arguments);
    if (this.get('session.isAuthenticated')) {
      this.session.invalidate();
    }
  }

Après :

// certif/app/routes/logout.js
beforeModel() {
    super.beforeModel(...arguments);
    if (this.session.isAuthenticated) {
      this.session.invalidate();
    }
  }

Ajout d'une fichier de config pour Ember CLI Update : config/ember-cli-update.json

Ember CLI propose de nouvelles fonctionnalités d'update, via son sous-module ember-cli-update dont un système de blueprints.

Pour fonctionner, il y a besoin d'un fichier config/ember-cli-update.json, sur le modèle de schéma suivant.

Le fichier ajouté par la PR est celui généré par la commande ember-cli-update --to 3.19, non modifié.

3.19 → 3.20

Mirage & tests en échec

Fort de ce premier succès, nous avons entreprise la seconde montée de version. Nous avons été confrontée à une erreur qui s'est révélée assez compliquée, sur un grand nombre de tests (une soixantaine).

Nous nous sommes concentrés sur la résolution du test d'acceptance authentication-test.js. Sa résolution a permis la résolution de tous les autres tests.

En effet, le problème venait de la configuration Mirage (mirage/config.js), et plus particulièrement de la ligne :

// certif/mirage/config.js
const userToken = request.requestHeaders.authorization.replace('Bearer ', '');

Suite à la montée de version, les propriétés contenues dans l'objet request.requestHeaders sont nommées en PascalCase plutôt qu'en greek-case, peut-être pour se rapprocher des "standards du Web".

Au passage, nous en avons profité pour monter de vesion :

• ember-cli-mirage : 1.1.6 → 1.1.8
• ember-fetch : 7.0.1 → 8.0.2

Remarque : nous avons réussi à détecter la source de l'erreur en activant l'option "Traces Mirage" dans l'onglet Chromium de test, et en checkant les logs (notamment d'erreur) dans la console du navigateur.

Avant :

// certif/mirage/config.js
const userToken = request.requestHeaders.authorization.replace('Bearer ', '');

Après :

// certif/mirage/config.js
const userToken = request.requestHeaders['Authorization'].replace('Bearer ', '');

Warning d'obsolescence à propos de l'évènement @mouseleave

Avant :

// certif/app/templates/authenticated/sessions/details/parameters.hbs
<CopyButton @clipboardText={{this.session.accessCode}} @success={{this.clipboardSuccess}} @mouseLeave={{this.clipboardOut}} @classNames="icon-button session-details-content__clipboard-button">

Après :

// certif/app/templates/authenticated/sessions/details/parameters.hbs
<CopyButton @clipboardText={{this.session.accessCode}} @success={{this.clipboardSuccess}} {{on 'mouseLeave' this.clipboardOut}} @classNames="icon-button session-details-content__clipboard-button">

Warning d'obsolescence à propos des paramètres ordonnés

Auparavant, dans Ember, il était possible de renseigner des paramètres d'un composant en fonction de l'ordre dans lequel on les déclarait dans le template. Désormais, cette fonctionnalité est déprécée et génère un warning.

Avant :

// certif/app/templates/authenticated/sessions/list.hbs
{{fa-icon 'plus'}}

Après :

// certif/app/templates/authenticated/sessions/list.hbs
{{fa-icon icon='plus'}}

Le problème de la computed property @sort

Alors que nous pensions que c'était tout bon pour la montée de version, un test Cypress s'est mis à échouer (merci la CI).

Scénario de reproduction

• Je me connecte -> je vois la liste des sessions ✅
• J’ajoute une session -> je vois la page de détail de la session créée ✅
• Je reviens sur la liste -> je vois ma nouvelle session dans la liste des sessions ❌

Après 3 jours de recherche et d'analyse, nous avons :

• commencé par reproduire le comportement de Cypress et l'erreur en local
• identifié que le problème apparaît avec le commit qui réalise la montée de version de ember-source en 3.20.0 (nous avons été jusqu'à essayer les versions 3.20.0-beta.[0-5] pour isoler la version / le commit en erreur)
• identifié que le problème se trouvait dans le controller du endpoint /sessions/list, qui récupère le modèle depuis la route (model = collection de Sessions Ember) avant de lui appliquer une transformation de tri sur la date et l'heure via la computed property @sort
• trouvé plusieurs issues GitHub évoquant ou se rapprochant du problème, ainsi que des objets Git / GitHub (commits, PR, releases, etc.) la traitant
• trouvé une solution à "l'instinct" (d'aucun diront au pif) en écoutant non plus model mais model.[] (vu qu'il s'agit d'une collection d'objets Ember Data)

Au final, nous ne sommes pas super satisfait de la solution et du manque de doc associé (jusqu'à quel point, ce qu'on a fait est viable et pérenne dans le temps ?).

Au passage, avec @jonathanperret, nous avons des doutes sur la qualité de l'implémentation existante. En l'état, on passe par le service current-user et par une sous-sous-propriété pour obtenir la liste des sessions. Ca nous paraît une façon très détournée de procédée. Elle a au moins le mérite de fonctionner.

Il nous semble qu'on devrait se rapprocher d'un fonctionnement et d'une implémentation de Pix Orga, pour la gestion de la liste des Campagnes, avec du requêtage et de la pagination côté serveur.

Font Awesome et dépréciations en tout genre

Le passage de la v3.19 à la v3.20 a induit des avertissements liés à :

• la dépréciation de l'emploi de paramètres ordonnés pour les composants dans les templates
• le style {{curly-bracket}} pour les composants ember-fontawesome
• l'usage interne (pas dans Pix) de la méthode @ember/object/getWithDefault

Ces problèmes ont été résolus assez simplement en montant de version ember-fontawesome de la v0.2.1 à la v0.2.2 (laquelle embarque le correctif lié à getWithDefault) et en suivant les nouvelles règles d'Ember / Glimmer.

Avant :

{{fa-icon 'plus'}}

Après :

<FaIcon @icon='plus' />

3.20 → 3.21

Pas de problème pour celle-ci. Ouf ! 🤤

💯 Pour tester

Il faut faire une passe complète sur Pix Certif 😬

[pix-service]

I'm deploying this PR to these urls:

• App: https://app-pr1867.review.pix.fr
• Orga: https://orga-pr1867.review.pix.fr
• Certif: https://certif-pr1867.review.pix.fr
• Admin: https://admin-pr1867.review.pix.fr
• API: https://api-pr1867.review.pix.fr/api/

Please check it out!

[laura-bergoens]

Revue FONC OK. Pour rappel, le contenu de la revue FONC pour PixCertif :

• Connexion / déconnexion
• Affichage du nom du centre de certif à gauche
• Bouton Documentation en bas à droite qui redirige vers la doc
• Création de session
• Modification de session
• Téléchargement de PV de session
• Inscription de candidats via import de PV
• Inscription de candidat unitairement
• Modification / Suppression de candidat
• Finalisation de session (+ on fait joujou avec les coches de fin de test)
• Actualisation du bon statut de la session (créée, finalisée et résultats transmis par Pix)

• j'ai fait le tour des icônes, je pense qu'elles sont toutes présentes !
  Merci les jérémy, je vais jeter un oeil au code maintenant avant de placer ma coche !

[Anne-Gaelle-S]

Testé fonctionnellement ça a l'air good :))