search

privacybydesign / irma-frontend-packages

Collection of frontend related packages, that together form a Javascript "client" to the IRMA server.
7 stars 4 forks source link

Introduced Aborted state #30

Closed ivard closed 4 years ago

ivard commented 4 years ago

Introduced a separate aborted state. When a popup is closed, the same instance cannot be started again. This means the promise that irmaCore.start() returns has to resolve or reject (depending on the case). I think the most neat solution for this is to make a separate state for this scenario such that irma-core can detect when the state gets in a unrecoverable state.

ivard commented 4 years ago

Lijkt me ook een goede suggestie. Heb je dan ook een suggestie hoe dat op te lossen met de handle pull request erbij? Dus restart is niet mogelijk, flow wordt aborted en dan wil je dat de oorspronkelijke error message nog zichtbaar blijft in die 3 seconde dat de pop-up nog niet automatisch geclosed is.

Of gaat dat dan vanzelf goed?

ivard commented 4 years ago

Ik heb er even over zitten denken en ik denk dat jouw proposal inderdaad het mooiste is. In de handle pull request zou ik dan kunnen toevoegen dat alle restart buttons automatisch hidden worden op het moment van een abort. Ik denk dat dat goed zou moeten gaan. Moet ik wel nog even die pull request aanpassen.

Ben je tevreden over de naam van je huidige commit? :)

Timendus commented 4 years ago

Ben je tevreden over de naam van je huidige commit? :)

Niet direct ;P

Begrijp ik dan goed dat je de 'aborted' state eigenlijk voor twee verschillende use-cases "misbruikt"?

  1. Flow is aborted by external code (popup wordt gesloten, bijvoorbeeld)
  2. Flow eindigt en komt op een "dood spoor" omdat de sessie niet herstart kan worden

Verdient dat tweede niet eigenlijk een eigen state dan..? Zit gewoon hardop te denken hier :P

ivard commented 4 years ago

Ja het is voor deze twee verschillende use cases inderdaad. Nu zijn deze twee use cases wel dermate hetzelfde dat ik het ook weer een beetje overkill vond om daar twee verschillende states voor te maken. Misschien is de naam Aborted dan niet helemaal goed, maar ik kon ook niet echt een betere term bedenken. Stopped vond ik ook niet echt de lading dekken.

Timendus commented 4 years ago

Misschien is in het geval van een gebruikersactie (gebruiker navigeert weg of sluit popup, of...) de state Cancelled qua naamgeving wel toepasselijk. Of je nou cancelled in de IRMA app of in de website, in feite probeer je hetzelfde te bereiken. Alleen wil je in het ene geval dat de popup sluit en in het andere geval dat de gebruiker de sessie kan herstarten. Hmm.... ik denk eigenlijk dat Aborted helemaal zo kwaad niet is voor een extern afgekapte flow.

Als het niet een gebruikersactie is, maar een doodlopend spoor dan vind ik Ended nog wel aardig..?

ivard commented 4 years ago

Tja de Cancelled state hergebruiken is wat lastig, want je wil de promise die wordt gereturned op irmaCore.start() rejecten op het moment dat je de pop-up closed en de Cancelled state hoeft niet per se final te zijn. Er is vanaf daar immers nog een restart mogelijk.

Misschien is het nog een optie dat we de transitie abort houden met de vervolgstate Ended. Dan kun je in het andere geval een transitie end maken die dat naar diezelfde Ended state leidt. Zo heb je wel het onderscheid zonder dat je twee states hebt die in de praktijk op hetzelfde neerkomen.

ivard commented 4 years ago

Processed the merge conflicts