[DO NOT MERGE] Try to adapt POC and API

misstick commented 7 years ago

Why this PR?

StateMachine implementation was not fully implemented,
POC merged wasnt stick to API/Documentation,
misunderstanding with concept of stateMachine.
The aim of this PR

Answering the questions :

What is a StateMachine?
Why using a StateMachine?
Is it necessary to implement a stepByStepComponent?
If yes, is POC should be improved?
if not, what should we kept?
What about handling data into steps? (ie. password)
What's next (...step :) )?
What is MachineStatePattern about?
having undo/redo actions (ie. go back to previous step),
do not change data into views or subViews but into an agnostic model (here into lib/onboarding aka OnboardingState),
having more readable views,
routes are directly related to OnboardingState

To my mind a MachineState pattern isnt onboarding component. MachineState is the pattern to handle data, and we will use it first on an onboarding workflow.

Improvement into `./lib/onboarding`

[x] move files from config/* into lib/onboarding/model/*,
[ ] separate OnboardingLib into 3 objects: Getter, ActionCreator and StateController,
[ ] define these 3 concept (with comments into OnboardingLib),
StateController is about make it all works,
ActionCreator are called by views (here BackboneViews) to update OnboardingState,
Getter is here to get data from OnboardingState to BackboneViews.

!!! To be consistent data can't/musnt be updated outside of OnboardingLib; otherwhise it's not a stateMachine anymore.

Tasks

[ ] adding a schema related to this architecture (with the objectNames),
[ ] make POC works,
[ ] FIX test.

nono commented 7 years ago

Désolé, je ne vais pas pouvoir trancher. Je n'ai pas suivi vos discussions. Vu de loin, j'ai l'impression que la PR est très complexe pour le peu qu'elle apporte. Mais il se peut que l'existant soit devenu trop compliqué à maintenir et qu'il faille revoir les choses en profondeur. Si c'est le cas, j'aurais bien aimé voir les raisons dans la description de la PR.

misstick commented 7 years ago

@nono : mon point de vue est ton 2nd point; le contexte est dans la description mais peut-être pas évident pour quelqu'un qui na rien suivi à cette histoire;

En gros il y a eu 3 visions différentes sur l'implémentation d'une "machine à état"; @gregorylegarec et @CPatchane ont fourni 2 POC, dont seul le 1er a abouti; quant à moi j'ai "pensé" une documentation sur les vues et leur branchement avec cet objet sur un 3eme concept (celui d'un état unique qui ne peut être changé que par l'intermédiaire controller, mais pas directement par les vues - cf flux concept).

Suite aux retours sur ma PR de documentation (cf https://github.com/cozy/cozy-proxy/pull/300) j'ai compris qu'il y avait eu mésentente sur les concept, et j'ai pensé par cette PR pouvoir mieux expliquer mon point de vue, tout en étant persuadée que ça améliorerait/simplifierait (nommage, organisation des fichiers, appels des méthodes) pour les implémentation futures dans les vues.

J'ai l'impression que ce point de vue n'a/avait toujours pas était compris; sachant que jusqu'à présent, mes retours sur l'API ont étaient vagues et m'ont empéché/bloqué pour savoir ce que je devait changer/modifier pour avancer suivant les attentes communes.

A ce jour cette incompréhension a duré beaucoup trop de temps; suite au commentaire de @m4dz on abandonne cette idée; si ça se trouve il n'y aura que moi qui ne comprendra pas comment implémenter cet objet; l'onboarding n'étant pas destiné à être changé tous les jours, une amélioration quelle quelle soit ne sera sûrement pas nécessaire.

Cependant j'aimerais bien que lorsque ce qui est codé ne correspond pas à ce qui est demandé, ce soit explicitement précisé avec des exemples, pour permettre la modification possible et faire comprendre où est la mé-compréhension.

@nono : est-ce que c'est plus claire du coup?

cozy / cozy-proxy