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

Ajoute le code de suivi pour l'étude d'impact Pôle Emploi #288

Merged
merged 10 commits into from Jun 27, 2016

Conversation

Projects
None yet
2 participants
@MattiSG
Member

MattiSG commented Jun 2, 2016

Ce code envoie des éléments de suivi de parcours pour les participants à l'étude d'impact de Mes Aides sur une cohorte d'inscrits à Pôle Emploi, dès lors qu'un identifiant de suivi eid est passé par la querystring.

Avant de déployer :

  • Vérifier que des éléments décrivent la recherche et ses parties prenantes de manière claire sur les serveurs de suivi.
Show outdated Hide outdated app/js/services/impactStudyService.js
STUDIED_PRESTATIONS = [ 'aah', 'aah_non_calculable', 'acs', 'adpa', 'af', 'aide_logement', 'aide_logement_non_calculable', 'alf', 'als', 'apl', 'asf', 'asi', 'aspa', 'ass', 'bourse_college', 'bourse_lycee', 'cf', 'cmu_c', 'paje_base', 'paris_logement_familles', 'ppa', 'rsa', 'rsa_majore', 'rsa_non_calculable', 'rsa_non_majore' ];
function getSessionId() {

This comment has been minimized.

@fpagnoux

fpagnoux Jun 7, 2016

Contributor

I still don't really get the point of capturing this knowing that we already have the eid. But anyway, if the api needs it, fair enough.

@fpagnoux

fpagnoux Jun 7, 2016

Contributor

I still don't really get the point of capturing this knowing that we already have the eid. But anyway, if the api needs it, fair enough.

This comment has been minimized.

@MattiSG

MattiSG Jun 7, 2016

Member
@MattiSG

MattiSG via email Jun 7, 2016

Member
Show outdated Hide outdated app/js/services/impactStudyService.js
}
function getResearchId() {
$sessionStorage.researchId = $sessionStorage.researchId || $location.search().eid;

This comment has been minimized.

@fpagnoux

fpagnoux Jun 7, 2016

Contributor

So if the studied user makes two simulations, the second one without the eid, we don't want to track him.her ?

@fpagnoux

fpagnoux Jun 7, 2016

Contributor

So if the studied user makes two simulations, the second one without the eid, we don't want to track him.her ?

This comment has been minimized.

@MattiSG

MattiSG Jun 7, 2016

Member
@MattiSG

MattiSG via email Jun 7, 2016

Member
@fpagnoux

This comment has been minimized.

Show comment
Hide comment
@fpagnoux

fpagnoux Jun 7, 2016

Contributor

Le code et les commentaires sont clairs sur l'intention 👍
Dans l'idéal, il faudrait que la notice soit publiée directement en page d'accueil sur mes-droits.fr, sinon je rajouterais peut-être un lien vers la page précise.
Et le stockage de l'eid en sessionStorage plutôt que localStorage me paraît pas forcément le plus pertinent, mais cela répond à la demande.
GTM

Contributor

fpagnoux commented Jun 7, 2016

Le code et les commentaires sont clairs sur l'intention 👍
Dans l'idéal, il faudrait que la notice soit publiée directement en page d'accueil sur mes-droits.fr, sinon je rajouterais peut-être un lien vers la page précise.
Et le stockage de l'eid en sessionStorage plutôt que localStorage me paraît pas forcément le plus pertinent, mais cela répond à la demande.
GTM

@MattiSG

This comment has been minimized.

Show comment
Hide comment
@MattiSG

MattiSG Jun 7, 2016

Member
Member

MattiSG commented Jun 7, 2016

@MattiSG

This comment has been minimized.

Show comment
Hide comment
Member

MattiSG commented Jun 7, 2016

ping @fpagnoux

Show outdated Hide outdated app/js/services/resultatService.js
@@ -1,6 +1,6 @@
'use strict';
angular.module('ddsApp').service('ResultatService', function($http, $modal, droitsDescription) {
angular.module('ddsApp').service('ResultatService', function($http, $modal, $location, droitsDescription) {

This comment has been minimized.

@MattiSG

MattiSG Jun 24, 2016

Member

Why is this still here while there's no add of this dependency?

@MattiSG

MattiSG Jun 24, 2016

Member

Why is this still here while there's no add of this dependency?

Show outdated Hide outdated package.json
@@ -3,6 +3,7 @@
"description": "Simulateur de prestations sociales mes-aides.gouv.fr",
"version": "1.0.0",
"dependencies": {
"angular-uuid": "0.0.2",

This comment has been minimized.

@MattiSG

MattiSG Jun 24, 2016

Member

Isn't this redundant with angular-uuids in Bower?

@MattiSG

MattiSG Jun 24, 2016

Member

Isn't this redundant with angular-uuids in Bower?

Show outdated Hide outdated test/spec/services/impactStudyService.js
return JSON.parse(data).session_id == sessionId;
});
httpMock.flush();

This comment has been minimized.

@MattiSG

MattiSG Jun 24, 2016

Member

This should probably be in an afterEach.

@MattiSG

MattiSG Jun 24, 2016

Member

This should probably be in an afterEach.

@MattiSG

This comment has been minimized.

Show comment
Hide comment
@MattiSG

MattiSG Jun 24, 2016

Member

@fpagnoux I just self-reviewed this and intend to merge & deploy today. I would really appreciate your review though, as this is important code.

Member

MattiSG commented Jun 24, 2016

@fpagnoux I just self-reviewed this and intend to merge & deploy today. I would really appreciate your review though, as this is important code.

@MattiSG MattiSG merged commit feedb37 into master Jun 27, 2016

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@MattiSG MattiSG deleted the etude-pse branch Jun 27, 2016

@fpagnoux

This comment has been minimized.

Show comment
Hide comment
@fpagnoux

fpagnoux Jun 27, 2016

Contributor

Post-merge reviewed, seems good to me.

Contributor

fpagnoux commented Jun 27, 2016

Post-merge reviewed, seems good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment