Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TECH] Mise à jour de PixAPP vers Ember v3.15 (PF-1034). #1025

Merged
merged 10 commits into from
Feb 18, 2020

Conversation

sbedeau
Copy link
Contributor

@sbedeau sbedeau commented Jan 31, 2020

🦄 Problème

Pas à jour

🤖 Solution

À jour en exécutant les code-mods.

📚 Littérature utilisée

☝️ Remarques

  • Compte tenu de la complexité de la montée de version, on assume exceptionnellement le fait que tous les commits ne soient pas tous verts. Cette décision est prise pour faciliter la revue.

  • Compte tenu de la complexité et du volume à traiter, l'objectif de cette PR est de rendre Mon-Pix compatible avec Ember v3.15. Par conséquent, seules les nouvelles pratiques nécessaires à la viabilité du produit sont abordées ici.

  • Les autres recommandations (passage intégral à glimmer, classes en native JS, etc.) seront implémentées dans d'autres PRs dédiées ou diluées dans les stories (fonction d'une décision à prendre)

⚠️ Pour valider

Compte tenu de la criticité associée à cette PR, on aimerait sécuriser avec

  • au moins 6 coches tech

  • au moins 2 coches func

@sbedeau sbedeau added Development in progress team-evaluation PR relatives à l'expérience d'évaluation labels Jan 31, 2020
@sbedeau sbedeau self-assigned this Jan 31, 2020
@pix-service
Copy link
Contributor

@sbedeau sbedeau force-pushed the tech-upgrade-pixapp-to-ember-3.15 branch 3 times, most recently from 5b70eb1 to 0120198 Compare February 6, 2020 13:33
@MelanieMEB MelanieMEB force-pushed the tech-upgrade-pixapp-to-ember-3.15 branch from 69de7c8 to f6e0085 Compare February 6, 2020 15:42
@celineung celineung force-pushed the tech-upgrade-pixapp-to-ember-3.15 branch 4 times, most recently from cdc0a49 to a3ac52f Compare February 11, 2020 10:03
@sbedeau sbedeau marked this pull request as ready for review February 12, 2020 10:12
@sbedeau sbedeau force-pushed the tech-upgrade-pixapp-to-ember-3.15 branch from 405327c to 5d9556b Compare February 13, 2020 09:56
mon-pix/app/adapters/user.js Outdated Show resolved Hide resolved
@sbedeau sbedeau force-pushed the tech-upgrade-pixapp-to-ember-3.15 branch from 5d9556b to 2c75534 Compare February 13, 2020 17:59
@jbuget
Copy link
Contributor

jbuget commented Feb 14, 2020

Je conseille aussi cette lecture ~ https://guides.emberjs.com/release/upgrading/current-edition/

Copy link
Contributor

@jbuget jbuget left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Attention aux icones FaIcon ! https://github.com/FortAwesome/ember-fontawesome#template

J'ai surtout des questions.

Je crois avoir détecté des erreurs d'inattention (ou d'incompréhension de ma part).

A part ça, bravo pour tout le boulot abattu !

mon-pix/app/adapters/user.js Show resolved Hide resolved
mon-pix/app/templates/components/challenge-item-qcm.hbs Outdated Show resolved Hide resolved
mon-pix/app/templates/components/routes/register-form.hbs Outdated Show resolved Hide resolved
mon-pix/config/optional-features.json Show resolved Hide resolved
@@ -81,7 +81,7 @@
"ember-mocha": "^0.16.2",
"ember-modal-dialog": "^2.4.4",
"ember-moment": "^8.0.0",
"ember-page-title": "^5.1.0",
"ember-page-title": "^5.2.0",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

du coup on enlève les titres ? Ya une stratégie pour les remettre après ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah non, je viens de comprendre, la nouvelle version 5.2.0 permet de plus hacker le truc, c'est ça ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je crois 🤔
@celineung ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non on enlève pas les titres, les titres ne marchaient plus avec la montée de version.
head.hbs est un fichier utilisé par ember-page-title lorsqu'il est en mode fastboot, sauf que nous ne l'utilisons plus en fastboot. En supprimant ce fichier, on se rend compte que les titres marchent à nouveau.
La montée de version ici est juste pour mettre le package .json à jour. Cela n'influe pas sur la correction du bug rencontré.

Copy link
Contributor

@lisequesnel lisequesnel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

J'ai une 500 sur la route POST student-dependent-users lorsqu'on essaie de se créer un compte après la pré-réconciliation
image

@sbedeau sbedeau force-pushed the tech-upgrade-pixapp-to-ember-3.15 branch from fd9fffe to 96d0977 Compare February 14, 2020 14:32
@laura-bergoens laura-bergoens force-pushed the tech-upgrade-pixapp-to-ember-3.15 branch from 96d0977 to 41cd2b2 Compare February 17, 2020 15:00
@PhilippineLoison PhilippineLoison added the Func Review OK PO validated functionally the PR label Feb 17, 2020
@asrodride asrodride self-requested a review February 17, 2020 16:54
@sbedeau sbedeau force-pushed the tech-upgrade-pixapp-to-ember-3.15 branch from 41cd2b2 to 8f612c9 Compare February 17, 2020 17:44
sbedeau and others added 10 commits February 18, 2020 10:24
As an example, @ is used when data comes from a parent and
this.stuff is used when stuff is retrieve or computed
inside the component. See the following links for details:
https://ember-learn.github.io/ember-octane-vs-classic-cheat-sheet/
Ember now refuse to find a model object of type <a> by id and retrieve
an object from a different type <b> with another id.

To fix this, we transformed the findRecord to a queryRecord with an
adapter in order to retrieve an object of type <user> and not a fake
<password-reset-demand> that is, in fact, of type <user>.

Note that a findRecord is supposed to find a record thanks to an id
while a queryRecord is more free in its research
@sbedeau sbedeau force-pushed the tech-upgrade-pixapp-to-ember-3.15 branch from 8f612c9 to 0a9ff30 Compare February 18, 2020 09:25
@MelanieMEB MelanieMEB dismissed lisequesnel’s stale review February 18, 2020 09:40

Vu avec elle directement

@sbedeau sbedeau merged commit 339476d into dev Feb 18, 2020
@sbedeau sbedeau deleted the tech-upgrade-pixapp-to-ember-3.15 branch February 18, 2020 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Func Review OK PO validated functionally the PR team-acces team-certif team-evaluation PR relatives à l'expérience d'évaluation team-prescription Tech Review OK
Projects
None yet
Development

Successfully merging this pull request may close these issues.