-
Notifications
You must be signed in to change notification settings - Fork 20
[#563] [CLEANUP] Nettoyage de code côté front (US-867). #563
Conversation
jbuget
commented
Oct 18, 2017
•
edited
Loading
edited
- suppression de routes inutiles
- utilisation des nested routes
- utilisation "correcte" des models relatifs au routes params (amélioration des perfs)
- suppression d'un initializer devenu inutile
- correction d'une erreur levée lors de la validation d'une épreuve (pour 2 challenges du même type qui se suivent)
3c0e116
to
621340a
Compare
|
||
afterModel(model) { | ||
|
||
redirect(course) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pourquoi créer une méthode redirect dans cette route ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
C'est dans le cas où on doit rediriger vers une route qui requiert les mêmes modèles que la route initiale, ça permet d'éviter de recharger "again and again" les modèles (qu'on a déjà). cf. https://guides.emberjs.com/v2.16.0/routing/redirection/#toc_child-routes
.then(() => challengeAdapter.queryNext(store, assessment.get('id'))) | ||
.then(challenge => this.transitionTo('assessments.get-challenge', { assessment, challenge })); | ||
.then(() => challengeAdapter.queryNext(store, assessment.id)) | ||
.then(challenge => this.transitionTo('assessments.challenge', { assessment, challenge })); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On avait dit qu'on faisait les transitions dans les after-model n'est-ce pas ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oui, mais je n'avais pas envie de prendre ça en compte dans cette PR, qui est déjà assez conséquente.
|
||
return store.findRecord('course', model.courseId) | ||
.then((course) => store.createRecord('assessment', { course }).save()) | ||
return store.createRecord('assessment', { course }).save() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pour le coup, est-ce qu'on ne pourrait pas faire le createRecord dans un model ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non, car le store Ember fait des trucs en plus du genre sauvegarder l'instance dans un état transient dans son registre et mettre à jour l'état de l'instance une fois persistée par l'API.
const store = this.get('store'); | ||
|
||
const assessmentId = params.assessment_id; | ||
const { assessment_id: assessmentId } = this.paramsFor('assessments'); | ||
const challengeId = params.challenge_id; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Du coup, on ne peux pas avoir une destructuration pour les deux ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
J'ai testé vite fait et apparemment ce n'est pas aussi simple que ça. Je préfère ne pas gaspiller de temps dessus.
return model.assessment.get('course').then((course) => { | ||
model.progress = course.getProgress(model.challenge); | ||
const store = this.get('store'); | ||
const answers = store.queryRecord('answer', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
La variable store est pas vraiment nécessaire ici.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Impossible de mettre le queryRecord
dans le model ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non, car le store fait des trucs en plus, du genre : gérer la désérialization et pusher le bon objet dans le bon état dans son registre des (instances de) modèles.
@@ -44,21 +44,6 @@ describe('Integration | Component | challenge actions', function() { | |||
expect(this.$('.challenge-actions__loader-spinner')).to.have.lengthOf(1); | |||
}); | |||
|
|||
it('should be enable again when the treatment succeeded', function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Il y a une raison pour laquelle ce test n'est plus nécessaire ?
…erf optimizations as a result ^^)
17d8f70
to
fba319c
Compare
* Remove useless route courses.get-challenge-preview * Micro-refactor router.js to use new Ember 2.16 import * Remove useless route file 'create-assessment-old' * Rename route 'challenges.get-preview' into 'challenges.preview' * Rename route 'challenges.preview' into challenge-preview * Add a '/' before 'rejoindre' route path * Rename route (and files)' assessments.get-challenge' into 'assessments.challenge' * Rename route (and files)' assessments.results' into 'assessments.results' * Refactor assessments routes to declare and use nesting routes (with perf optimizations as a result ^^) * Fix little regression on 'validate' button management * Rename route 'assessments.get-comparison' into 'assessments.comparison' * Remove useless initializer
* Remove useless route courses.get-challenge-preview * Micro-refactor router.js to use new Ember 2.16 import * Remove useless route file 'create-assessment-old' * Rename route 'challenges.get-preview' into 'challenges.preview' * Rename route 'challenges.preview' into challenge-preview * Add a '/' before 'rejoindre' route path * Rename route (and files)' assessments.get-challenge' into 'assessments.challenge' * Rename route (and files)' assessments.results' into 'assessments.results' * Refactor assessments routes to declare and use nesting routes (with perf optimizations as a result ^^) * Fix little regression on 'validate' button management * Rename route 'assessments.get-comparison' into 'assessments.comparison' * Remove useless initializer
* Remove useless route courses.get-challenge-preview * Micro-refactor router.js to use new Ember 2.16 import * Remove useless route file 'create-assessment-old' * Rename route 'challenges.get-preview' into 'challenges.preview' * Rename route 'challenges.preview' into challenge-preview * Add a '/' before 'rejoindre' route path * Rename route (and files)' assessments.get-challenge' into 'assessments.challenge' * Rename route (and files)' assessments.results' into 'assessments.results' * Refactor assessments routes to declare and use nesting routes (with perf optimizations as a result ^^) * Fix little regression on 'validate' button management * Rename route 'assessments.get-comparison' into 'assessments.comparison' * Remove useless initializer
… (US-924). (#585) * Add Sorted skills * litlle refacto of user-service_test * Massive function rename * refacto * refacto * [US-880] resolve Bug from func review (only skills from 1st assessment) * [US-880] delete unused function * [#563] [CLEANUP] Nettoyage de code côté front (US-867). (#563) * Remove useless route courses.get-challenge-preview * Micro-refactor router.js to use new Ember 2.16 import * Remove useless route file 'create-assessment-old' * Rename route 'challenges.get-preview' into 'challenges.preview' * Rename route 'challenges.preview' into challenge-preview * Add a '/' before 'rejoindre' route path * Rename route (and files)' assessments.get-challenge' into 'assessments.challenge' * Rename route (and files)' assessments.results' into 'assessments.results' * Refactor assessments routes to declare and use nesting routes (with perf optimizations as a result ^^) * Fix little regression on 'validate' button management * Rename route 'assessments.get-comparison' into 'assessments.comparison' * Remove useless initializer * [#576] [TECH] Mise à jour de la config eslint pour Mocha. [#576] [TECH] Mise à jour de la config eslint pour Mocha * [US-880] integrating review comments * Fixing linter * [#574] [FEATURE] Possibilité de reprendre un test déjà commencé depuis la page du profil (US-565). (#574) * [US-565] Add link reprendre le test when assessment is related to competence * [US-565] create data to test functionnally on mirage * [US-565] add assessmentId field in competence model * [US-565] implement link reprendre when assessment is present for a competence * [US-565] create resume assessment route * [US-565] get assessment infos in resume assessment route * [US-565] clean tests * [US-565] implement route assessment resume and make a little change on adapter * delete only * wip clean mirage for functional testing * Fix Mirage and make resume works (but break tests) * Take into account dynamic assessment in Mirage * finalize mirage routes refacto * Rename AssessmentRepository#findCompletedAssessmentsByUserId by AssessmentRepository#findLastAssessmentsForEachCoursesByUser * [US-565] implement assessment repo, get all assessments from one user * [US-565] add assessment id to competence in profile object * [US-565] implement profile-serializer with assessment-id * [US-565] change api and adapter front * Undo assessment-service fix * skip acceptance tests which fail (temporary action) * Re implement findCompletedAssessmentByUser * Unskip and fix broken acceptance tests on challenge validation * remove acceptance test on deprecated behaviour: two timed challenges cannot follow each other * delete skip * [US-880] changes from tech review * try to resolve acceptance test after rebase * fix errors from rebase * fix bug * [#578] [FEATURE] Mise à jour des logos des ministères de l'éducation (Sup & SCO) (US-822) (#578) * Update Mensr logo * Add alternative text to ministries logo * [#577] [CLEANUP] Modification des dates de fin d'inscriptions pour les etablissements (US-865) (#577) * modify date of inscription end * delete little note * delete unused style * Update lycee section content * Update sup section content * Add new component tutorial-panel (#582) * [#581] [FEATURE] Bouton seconde chance pour que l'utilisateur repasse la compétence (US-863). * [US-565] Add link reprendre le test when assessment is related to competence * [US-565] create data to test functionnally on mirage * [US-565] add assessmentId field in competence model * [US-565] implement link reprendre when assessment is present for a competence * [US-565] create resume assessment route * [US-565] get assessment infos in resume assessment route * [US-565] clean tests * [US-565] implement route assessment resume and make a little change on adapter * delete only * wip clean mirage for functional testing * Fix Mirage and make resume works (but break tests) * Take into account dynamic assessment in Mirage * finalize mirage routes refacto * Rename AssessmentRepository#findCompletedAssessmentsByUserId by AssessmentRepository#findLastAssessmentsForEachCoursesByUser * [US-565] implement assessment repo, get all assessments from one user * [US-565] add assessment id to competence in profile object * [US-565] implement profile-serializer with assessment-id * [US-565] change api and adapter front * Undo assessment-service fix * skip acceptance tests which fail (temporary action) * Re implement findCompletedAssessmentByUser * Unskip and fix broken acceptance tests on challenge validation * remove acceptance test on deprecated behaviour: two timed challenges cannot follow each other * delete skip * [US-880] changes from tech review * try to resolve acceptance test after rebase * fix errors from rebase * Add status in competence in progress bar * Add function to set status to competences in profile * Add Status in competence of profile and add status in tests * Rebase work for relaunch test + Add set status correctly for competences * Add simple link to replay assessment + modify check to continue assessment * [US-565] Add link reprendre le test when assessment is related to competence * [US-565] implement link reprendre when assessment is present for a competence * [US-565] create resume assessment route * [US-565] implement route assessment resume and make a little change on adapter * wip clean mirage for functional testing * Rename AssessmentRepository#findCompletedAssessmentsByUserId by AssessmentRepository#findLastAssessmentsForEachCoursesByUser * skip acceptance tests which fail (temporary action) * Re implement findCompletedAssessmentByUser * delete skip * fix errors from rebase * Add status in competence in progress bar * Add function to set status to competences in profile * Add Status in competence of profile and add status in tests * Rebase work for relaunch test + Add set status correctly for competences * Add simple link to replay assessment + modify check to continue assessment * Correct error for assessment with 0 pix score which was consider as notEvaluated * Add simple visible button to replay assessment + clean code * Correct test and clean code * Clean code after rebase * Clean code after rebase * Add findLastAssessmentsForEachCoursesByUser * Fix tests * Extracting logic from Profile constructor to service * Defining warnings * [#580] [BUGFIX] Correction du loader qui disparaît de façon impromptue au démarrage d'un test de positionnement (US-917). (#580) * Use replaceWith instead of transitionTo in order to have a better browser history, cf. https://guides.emberjs.com/v2.16.0/routing/redirection/#toc-toggle * Add template on route create-assessment (the fix is here) because afterModel() hook sends a notification to the Ember engine that it can stop the loader + change the loading text for a better user experience. * Remove routes nesting because, according to Ember, it is a good mechanism if we use a common templates for all the child routes. Here, it is not the case and it adds a little complexity that is too much. * Force Ember to reload assessment model from results page in order to fix bug
… évaluation de type certification (US-893) (#589) * Add Sorted skills * litlle refacto of user-service_test * Massive function rename * refacto * refacto * [US-880] resolve Bug from func review (only skills from 1st assessment) * [US-880] delete unused function * [#563] [CLEANUP] Nettoyage de code côté front (US-867). (#563) * Remove useless route courses.get-challenge-preview * Micro-refactor router.js to use new Ember 2.16 import * Remove useless route file 'create-assessment-old' * Rename route 'challenges.get-preview' into 'challenges.preview' * Rename route 'challenges.preview' into challenge-preview * Add a '/' before 'rejoindre' route path * Rename route (and files)' assessments.get-challenge' into 'assessments.challenge' * Rename route (and files)' assessments.results' into 'assessments.results' * Refactor assessments routes to declare and use nesting routes (with perf optimizations as a result ^^) * Fix little regression on 'validate' button management * Rename route 'assessments.get-comparison' into 'assessments.comparison' * Remove useless initializer * [#576] [TECH] Mise à jour de la config eslint pour Mocha. [#576] [TECH] Mise à jour de la config eslint pour Mocha * [US-880] integrating review comments * Fixing linter * [#574] [FEATURE] Possibilité de reprendre un test déjà commencé depuis la page du profil (US-565). (#574) * [US-565] Add link reprendre le test when assessment is related to competence * [US-565] create data to test functionnally on mirage * [US-565] add assessmentId field in competence model * [US-565] implement link reprendre when assessment is present for a competence * [US-565] create resume assessment route * [US-565] get assessment infos in resume assessment route * [US-565] clean tests * [US-565] implement route assessment resume and make a little change on adapter * delete only * wip clean mirage for functional testing * Fix Mirage and make resume works (but break tests) * Take into account dynamic assessment in Mirage * finalize mirage routes refacto * Rename AssessmentRepository#findCompletedAssessmentsByUserId by AssessmentRepository#findLastAssessmentsForEachCoursesByUser * [US-565] implement assessment repo, get all assessments from one user * [US-565] add assessment id to competence in profile object * [US-565] implement profile-serializer with assessment-id * [US-565] change api and adapter front * Undo assessment-service fix * skip acceptance tests which fail (temporary action) * Re implement findCompletedAssessmentByUser * Unskip and fix broken acceptance tests on challenge validation * remove acceptance test on deprecated behaviour: two timed challenges cannot follow each other * delete skip * [US-880] changes from tech review * try to resolve acceptance test after rebase * fix errors from rebase * fix bug * [#578] [FEATURE] Mise à jour des logos des ministères de l'éducation (Sup & SCO) (US-822) (#578) * Update Mensr logo * Add alternative text to ministries logo * [#577] [CLEANUP] Modification des dates de fin d'inscriptions pour les etablissements (US-865) (#577) * modify date of inscription end * delete little note * delete unused style * Update lycee section content * Update sup section content * Add new component tutorial-panel (#582) * [#581] [FEATURE] Bouton seconde chance pour que l'utilisateur repasse la compétence (US-863). * [US-565] Add link reprendre le test when assessment is related to competence * [US-565] create data to test functionnally on mirage * [US-565] add assessmentId field in competence model * [US-565] implement link reprendre when assessment is present for a competence * [US-565] create resume assessment route * [US-565] get assessment infos in resume assessment route * [US-565] clean tests * [US-565] implement route assessment resume and make a little change on adapter * delete only * wip clean mirage for functional testing * Fix Mirage and make resume works (but break tests) * Take into account dynamic assessment in Mirage * finalize mirage routes refacto * Rename AssessmentRepository#findCompletedAssessmentsByUserId by AssessmentRepository#findLastAssessmentsForEachCoursesByUser * [US-565] implement assessment repo, get all assessments from one user * [US-565] add assessment id to competence in profile object * [US-565] implement profile-serializer with assessment-id * [US-565] change api and adapter front * Undo assessment-service fix * skip acceptance tests which fail (temporary action) * Re implement findCompletedAssessmentByUser * Unskip and fix broken acceptance tests on challenge validation * remove acceptance test on deprecated behaviour: two timed challenges cannot follow each other * delete skip * [US-880] changes from tech review * try to resolve acceptance test after rebase * fix errors from rebase * Add status in competence in progress bar * Add function to set status to competences in profile * Add Status in competence of profile and add status in tests * Rebase work for relaunch test + Add set status correctly for competences * Add simple link to replay assessment + modify check to continue assessment * [US-565] Add link reprendre le test when assessment is related to competence * [US-565] implement link reprendre when assessment is present for a competence * [US-565] create resume assessment route * [US-565] implement route assessment resume and make a little change on adapter * wip clean mirage for functional testing * Rename AssessmentRepository#findCompletedAssessmentsByUserId by AssessmentRepository#findLastAssessmentsForEachCoursesByUser * skip acceptance tests which fail (temporary action) * Re implement findCompletedAssessmentByUser * delete skip * fix errors from rebase * Add status in competence in progress bar * Add function to set status to competences in profile * Add Status in competence of profile and add status in tests * Rebase work for relaunch test + Add set status correctly for competences * Add simple link to replay assessment + modify check to continue assessment * Correct error for assessment with 0 pix score which was consider as notEvaluated * Add simple visible button to replay assessment + clean code * Correct test and clean code * Clean code after rebase * Clean code after rebase * Add findLastAssessmentsForEachCoursesByUser * Fix tests * Extracting logic from Profile constructor to service * Defining warnings * [#580] [BUGFIX] Correction du loader qui disparaît de façon impromptue au démarrage d'un test de positionnement (US-917). (#580) * Use replaceWith instead of transitionTo in order to have a better browser history, cf. https://guides.emberjs.com/v2.16.0/routing/redirection/#toc-toggle * Add template on route create-assessment (the fix is here) because afterModel() hook sends a notification to the Ember engine that it can stop the loader + change the loading text for a better user experience. * Remove routes nesting because, according to Ember, it is a good mechanism if we use a common templates for all the child routes. Here, it is not the case and it adds a little complexity that is too much. * Remove toJSON() from AirTableRecord in model objects * adding missing tests to challenge-tests * add certifications/results route * add tests to certifications/results route * add certification-banner component * WIP styling certification + custom route /certifications for dev * Add top level template-component and related tests * lint compte-test * Update according to code review * updating certification result message * renaming certification-results-template into certification-results-page