Development #17685
"préblocage" d'une réservation
0%
Description
On imagine un formulaire où on demande à choisir un événement mais derrière il y a encore des pages et pourtant on ne voudrait pas qu'au bout la personne soit déçue et n'ait pas sa place. Bref, imaginer une période de préblocage.
Files
Related issues
Associated revisions
History
Updated by Thomas Noël almost 6 years ago
Note dans #17681 : « À côté de ce côté interface on doit aussi réfléchir aux mécanismes pour éviter que ça ne soit abusé et sans doute d'autres points. »
Updated by Frédéric Péters over 5 years ago
- Has duplicate Development #20437: possibilité de vérouiller un créneau horaire added
Updated by Frédéric Péters about 5 years ago
- Related to Development #23008: api: ajouter une action "lockslot" pour bloquer des slots pendant x minutes added
Updated by Thomas Noël about 5 years ago
J'importe ici mes notes sur #23008 :
On peut avoir des conflits de réservation si plusieurs personnes arrivent sur le front, voient les mêmes datetimes et sélectionnent le même. Lors de l'appel fillslot, seul le premier réussi, les suivants passent en échec.
On pourrait éviter que cela se produise dans le workflow de traitement de la demande, en appelant le webservice en condition de sortie pour vérifier qu'une place est encore disponible et pour la "locker" pendant un certain temps.
Quelque chose comme « /api/agenda/<id>/lockslot/<id>/ » qui renvoie la même chose que fillslot, mais ne lock en réalité que pendant 10 minutes. Ou alors en payload d'un appel à fillslot, avec un "lock:10" ou "lock:True".
Techniquement on imagine un Booking.auto_cancellation_datetime (None par défaut) qui s'il existe, effectue un cancel de la réservation à l'instant donné, via un cron lancé chaque 5 minute.
La difficulité : on ne pourra pas dans w.c.s. stocker l'id de la pré-réservation ainsi obtenue. Donc, dans le workflow, lors du vrai fillslot, il faudra accepter même si booking existe, et dans ce cas le confirmer.
Updated by Thomas Noël almost 5 years ago
- Status changed from Nouveau to En cours
- Priority changed from Haut to Normal
Voici une première proposition de fonctionnement.
Tout se passe à travers un "code de blocage" (lock_code) qui doit être unique dans chrono. Via wcs on utilisera la référence de la demande (nnn-mmm).
- On cherche d'abord la liste des dispo pour ce code, disons "MYLOCK". C'est la liste des dispos habituelle.
- Une fois le choix fait, on lance la réservation en mode "lock", en envoyant un fillslot avec le lock_code
- cela se fera via une condition de sortie de page
- cela créé un booking, qui a comme particularité d'avoir un lockcode et une date d'expiration de ce lock
- A partir de là, la liste des dispos ne contient plus cette résa
- et donc les autres personnes ne voient plus la dispo
- et même si elles l'avaient vu, elles ne peuvent plus la réserver (la condition de sortie va échouer)
- Cependant, si on envoie le lockcode MYLOCK, la dispo est bien visible (cas d'un aller-retour sur la page de choix des resa)
- Si on lock une autre dispo avec MYLOCK, la résa précédemment lockée ne l'est plus
- à la fin, pour valider vraiment la réservation, on la renvoie, avec le même lockcode et un confirm_after_lock=True. Cela créé un "vrai" booking, ie sans lockcode, ie non temporaire
Par défaut, un lock dure 10 minutes, ça se change via les settings ou lors de l'appel. Il manque encore dans ce code un cron de nettoyage des lock périmés.
Voilà, avant de chercher à peaufiner et notamment étendre les tests, je veux bien un avis sur :- le principe
- l'intérêt de la chose sur les agenda events ? (je n'y ai pas trop reflechi)
- la façon dont c'est codé, parce qu'il y a une partie un peu obscure qui est la suppression des lockcode qui suppriment des events sur un agenda de type meeting (rendez-vous), ces events étant en effet bidons.
Updated by Thomas Noël almost 5 years ago
- File 0001-api-handle-lock-on-fillslots-and-datetimes-17685.patch 0001-api-handle-lock-on-fillslots-and-datetimes-17685.patch added
- Patch proposed changed from No to Yes
Et le patch, donc.
Updated by Benjamin Dauvergne almost 5 years ago
Je ne comprends pas bien le besoin du lock_code, pourquoi ne pas simplement ajouter un paramètre booléen lock
si celui-ci présent on pose lock_expiration. On renvoie le event.id ou un truc du genre. Sur un nouvel appel avec cet event_id on vire toutes les histoires de lock, c'est la confirmation de la réservation.
Ça évitera d'avoir à générer un lock_code coté w.c.s. (maintenant c'est peut-être volontaire de pouvoir envoyer un truc un peu intelligible comme l'identifiant du formulaire je ne sais pas trop).
Pour moi le code serait plus clair avec deux web-services plutôt que de tout surcharger sur un un seul (un de pré-réservation l'autre de réservation).
Updated by Thomas Noël almost 5 years ago
Benjamin Dauvergne a écrit :
Je ne comprends pas bien le besoin du lock_code, pourquoi ne pas simplement ajouter un paramètre booléen
lock
si celui-ci présent on pose lock_expiration. On renvoie le event.id ou un truc du genre. Sur un nouvel appel avec cet event_id on vire toutes les histoires de lock, c'est la confirmation de la réservation.
Ouaip, c'est ce qui semble naturel... Mais la contrainte vient de w.c.s. : il peut rien stocker lors du formulaire. Il va juste lancer une condition de sortie, c'est tout (il ne peut rien faire du résultat). À ce stade le lock_code choisi par l'appelant est la seule solution que j'ai trouvée.
Un petit avantage, c'est que si le lock a finalement expiré entre temps mais que la confirmation réussi quand même (parce qu'il reste quand même une place), et bien tout se passe tranquillement. (parce qu'on ne cherche pas à valider une résa existante, mais bien à faire une résa, en considérant le lock comme libre).
Updated by Benjamin Dauvergne almost 5 years ago
Et du disposes de quoi pour construire ton lock code ? Un numéro de session ? Un id de draft ?
Updated by Thomas Noël almost 5 years ago
Benjamin Dauvergne a écrit :
Et du disposes de quoi pour construire ton lock code ? Un numéro de session ? Un id de draft ?
Oui, je pensais à form_number.
Updated by Thomas Noël almost 5 years ago
- Patch proposed changed from Yes to No
En fait ça ne marche pas, parce qu'on n'a pas de lock_code disponible à tout coup (form_number n'existe pas lors d'une saisie sur une demande qui n'a pas de mode brouillon)
Updated by Benjamin Dauvergne almost 5 years ago
- Related to Development #24635: Générer un identifiant pour un formdata dès la saisie added
Updated by Thomas Noël over 3 years ago
On a maintenant session_hash_id #39784 qui peut faire l'affaire en première approximation ; ce ticket peut reprendre vie.
Updated by Benjamin Dauvergne about 2 years ago
- File 0001-api-add-lock_code-parameter-to-fillslot-and-datetime.patch 0001-api-add-lock_code-parameter-to-fillslot-and-datetime.patch added
- Status changed from En cours to Solution proposée
- Patch proposed changed from No to Yes
J'ai commencé par reprendre le patch de Thomas et puis en bossant dessus je me suis rendu compte que c'était bien compliqué d'ajouter encore de nouveaux champs à Booking/Event&co. J'ai préféré introduire un nouveau modèle sur le coté pour stocker juste les locks. Il se nettoye tout seul (Delete à chaque fois que qu'on le requête) et ça permet de gérer les locks aussi sur les ressources au passage. Je n'ai pas particulièrement optimisé les requêtes dans get_all_slots() parce qu'il faut supposer qu'il n'y aura que ttrès peu de locks vivant en même temps (à peu près autant que d'utilisateurs concurrent sur une période de longueur CHRONO_LOCK_DURATION).
Je n'ai pas particulièrement réfléchi à la réponse de fillslot quand on utilise lock_code, là ça retourne juste {'err': 0}
.
J'ai ajouté le même genre d'index d'exclusion que sur les Event mais sans prévenir les bloquages en cas d'absence de l'extension, je me suis dit que ce stade était passé.
Le seul bémol par rapport à une implémentation plus compliqué c'est qu'il reste une très faible possibilité qu'un fillslot ait lieu au moment d'un fillslot?lock_code=XXX. La meilleur protection c'est de recommencer le fillslot?lock_code=XXX en fin de formulaire (et même à chaque page, ainsi ça prolonge le lock à chaque fois). Dans ce cas c'est impossible d'avoir d'erreur (si le formulaire a plus d'une page et que le choix du rdv n'est pas sur la dernière).
PS: j'ai aussi complètement bloqué la fonctionnalité pour le cas des évènements, je n'avais pas de test sous la main et je me suis dit que ce serait mieux de reprendre ça à froid si ça devient nécessaire un jour.
PS2: et bien sûr, mais ça n'a pas été dit avant explicitement, ça ne pourra jamais marcher avec un formulaire ayant des brouillons, ou plutôt si on prévoit des conditions de sortie de page un peu partout, ça devrait lever une erreur à la récupération du brouillon.
Updated by Benjamin Dauvergne about 2 years ago
3 tests dont le comportement change vers minuit : https://jenkins.entrouvert.org/job/chrono-wip/job/wip%252F17685--preblocage-d-une-reservation/2/
Updated by Benjamin Dauvergne about 2 years ago
- File 0001-api-add-lock_code-parameter-to-fillslot-and-datetime.patch 0001-api-add-lock_code-parameter-to-fillslot-and-datetime.patch added
Ça devient un peu fatiguant de rebaser.
Updated by Valentin Deniaud almost 2 years ago
Qu'est-ce que fait le SQL de la migration ? EXCLUDE USING GIST(desk_id WITH =, tstzrange(start_datetime, end_datetime) WITH &&)
franchement qui a inventé une syntaxe pareille. (l'explication peut s'accompagner d'un renommage de sql_forwards
en quelque chose de plus parlant)
Mais avant de relire le code je voulais tester et justement, en essayant d'appliquer la migration,
Applying agendas.0088_lease...Traceback (most recent call last): File "/home/vdeniaud/envs/publik-env-py3/lib/python3.7/site-packages/django/db/backends/utils.py", line 82, in _execute return self.cursor.execute(sql) psycopg2.errors.UndefinedObject: data type integer has no default operator class for access method "gist" HINT: You must specify an operator class for the index or define a default operator class for the data type.
Updated by Frédéric Péters almost 2 years ago
je voulais tester
De bout en bout ? i.e. avec une démarche wcs ?
(ici appel du pied à Benjamin pour qu'il la partage)
Updated by Benjamin Dauvergne almost 2 years ago
- Status changed from Solution proposée to En cours
Ok, j
Valentin Deniaud a écrit :
Qu'est-ce que fait le SQL de la migration ?
EXCLUDE USING GIST(desk_id WITH =, tstzrange(start_datetime, end_datetime) WITH &&)
franchement qui a inventé une syntaxe pareille. (l'explication peut s'accompagner d'un renommage desql_forwards
en quelque chose de plus parlant)
Si c'est la première fois que la syntaxe SQL te parait bizarre c'est que tu es t
Mais avant de relire le code je voulais tester et justement, en essayant d'appliquer la migration,
[...]
C'est bizarre parce qu'on utilise déjà une contrainte équivalente sur les event:
chrono/agendas/migrations/0053_event_date_range_constraint.py:EXCLUDE USING GIST(desk_id WITH =, tstzrange(start_datetime, _end_datetime) WITH &&)
et que ça passe crême sur jenkins et chez moi.
Est-ce que la migration 0053 n'échouerait pas chez toi ? (ton user n'est pas administrateur de postgresql).
Alors je peux cacher la non création de l'index de contrainte, encore comme dans 0053, si vous voulez.
Frédéric Péters a écrit :
De bout en bout ? i.e. avec une démarche wcs ?
(ici appel du pied à Benjamin pour qu'il la partage)
Ok je fais ça.
Updated by Benjamin Dauvergne almost 2 years ago
- File 0003-to-fixup-rename-lease-migration.patch 0003-to-fixup-rename-lease-migration.patch added
- File 0004-to-fixup-adapt-dependency-of-lease-migration.patch 0004-to-fixup-adapt-dependency-of-lease-migration.patch added
- File 0002-to-fixup-rename-sql-snippets.patch 0002-to-fixup-rename-sql-snippets.patch added
- File 0001-api-add-lock_code-parameter-to-fillslot-and-datetime.patch 0001-api-add-lock_code-parameter-to-fillslot-and-datetime.patch added
Rebasé.
Updated by Valentin Deniaud almost 2 years ago
Benjamin Dauvergne a écrit :
Si c'est la première fois que la syntaxe SQL te parait bizarre c'est que tu es t
Je n'ose imaginer de quel nom tu voulais me traiter !
Est-ce que la migration 0053 n'échouerait pas chez toi ? (ton user n'est pas administrateur de postgresql).
Pas testé, pas envie de shooter ma db ni de créer un autre tenant juste pour voir. Par contre j'ai googlé et il me manquait un CREATE EXTENSION btree_gist;
et c'est désormais bon.
Updated by Benjamin Dauvergne almost 2 years ago
Valentin Deniaud a écrit :
Benjamin Dauvergne a écrit :
Si c'est la première fois que la syntaxe SQL te parait bizarre c'est que tu es t
Je n'ose imaginer de quel nom tu voulais me traiter !
Je voulais écrire très tolérant :)
Est-ce que la migration 0053 n'échouerait pas chez toi ? (ton user n'est pas administrateur de postgresql).
Pas testé, pas envie de shooter ma db ni de créer un autre tenant juste pour voir. Par contre j'ai googlé et il me manquait un
CREATE EXTENSION btree_gist;
et c'est désormais bon.
Le CREATE EXTENSION btree_gist
est justement dans la migration 0053.
Updated by Benjamin Dauvergne almost 2 years ago
- File datasource-agenda.wcs datasource-agenda.wcs added
- File wscall-lock_rdv.wcs wscall-lock_rdv.wcs added
- File workflow-rdv.wcs workflow-rdv.wcs added
- File form-test-agenda-lock.wcs form-test-agenda-lock.wcs added
- Status changed from En cours to Solution validée
- un appel webservice vers api_fillslot_url avec un seul paramètre
lock_code={{ session_hash_id }}
pour la condition de sortie de la page qui contient le sélecteur de RdV, pour vérifier la dispo et verrouiller (si un verrou pour le lock_code existe déjà il est supprimé et ça passe, donc on peut revenir en arrière et changer son RdV); l'appel peut resservir pour tous les formulaires, si le champ liste a toujours le même nom de variable (je vois que dans la doc on incite fortement à toujours le nommer "event", dans ce cas ça marchera, comme je ne connaissais pas l'usage ici la variable se nomme "rdv"), - une source de donnée agenda spécifique parce qu'elle doit préciser
?lock_code={{ session_hash_id|urlencode }}
pour ne pas cacher le RdV préalablement verrouillé en cas de retour en arrière (et éviter le message d'erreur "la sélection est invalide") - un formulaire/workflow de réservation de RdV bateau, juste l'appel à api_fillslot_url doit ajouter
lock_code={{ session_hash_id }}
etconfirm_after_lock=True
(en expression python).
Il faudrait voir pour que ça marche avec les sources de donnée agenda auto-générées en ajoutant automatiquement lock_code={{ session_hash_id|urlencode }}
, Ça ne devrait pas poser de problème car si le formulaire n'utilise pas le verrouillage ça n'aura pas de conséquence. De même on pourrait automatiquement créer un web-service "lock_event" qui marchera, si la variable s'appelle "event" par convention.
Je pense que j'ai tout dit, de mes premiers tests ça marche nickel (un formulaire ouvert en navigation privé et un en navigation classique, le passage de la première page exclut bien le RdV pour l'autre formulaire avec un message "Désolé, ce rendez-vous est déjà pris.", des retours en arrière puis changement de RdV débloquent la situation).
Updated by Benjamin Dauvergne almost 2 years ago
- Status changed from Solution validée to Solution proposée
Updated by Valentin Deniaud almost 2 years ago
Ça marche bien chez moi aussi, pas vraiment de remarques sur le fond, sur la forme :
Renommage de migration, mettre 0091 et pas juste 91.
+ If it is booked but match the lock code, report the slot as open. + """
Ligne blanche en trop.
+ # only lock related to on of the agenda or the resource
to one of (commentaire pas si utile ceci-dit)
La méthode as_interval de Lease n'est pas utilisée.
Dans fillslot tu as mis un allow_blank=True et ensuite tu vérifies que le champ n'est pas vide, il faudrait virer les deux et ça marcherait pareil.
Updated by Benjamin Dauvergne almost 2 years ago
- File 0007-to-fixup-remove-useless-comment.patch 0007-to-fixup-remove-useless-comment.patch added
- File 0003-to-fixup-rename-lease-migration.patch 0003-to-fixup-rename-lease-migration.patch added
- File 0004-to-fixup-adapt-dependency-of-lease-migration.patch 0004-to-fixup-adapt-dependency-of-lease-migration.patch added
- File 0002-to-fixup-rename-sql-snippets.patch 0002-to-fixup-rename-sql-snippets.patch added
- File 0009-to-fixup-let-DRF-handle-empty-lock_code-errors.patch 0009-to-fixup-let-DRF-handle-empty-lock_code-errors.patch added
- File 0008-to-fixup-removed-unused-Lease.as_interval.patch 0008-to-fixup-removed-unused-Lease.as_interval.patch added
- File 0001-api-add-lock_code-parameter-to-fillslot-and-datetime.patch 0001-api-add-lock_code-parameter-to-fillslot-and-datetime.patch added
- File 0005-to-fixup-rename-migration-from-91-to-0091.patch 0005-to-fixup-rename-migration-from-91-to-0091.patch added
- File 0006-to-fixup-remove-blank-line.patch 0006-to-fixup-remove-blank-line.patch added
Remarques prises en compte.
Updated by Valentin Deniaud almost 2 years ago
- Status changed from Solution proposée to Solution validée
Updated by Benjamin Dauvergne almost 2 years ago
- Status changed from Solution validée to Résolu (à déployer)
commit 2f9bf16a57bab183b3f2650d882ecf37348ed053 Author: Benjamin Dauvergne <bdauvergne@entrouvert.com> Date: Tue Mar 16 14:29:41 2021 +0100 api: add lock_code parameter to fillslot and datetimes (#17685)
Updated by Benjamin Dauvergne almost 2 years ago
- Related to Development #54956: Revert de #17685 « "préblocage" d'une réservation » added
Updated by Benjamin Dauvergne almost 2 years ago
- Status changed from Résolu (à déployer) to Nouveau
Updated by Frédéric Péters almost 2 years ago
De #54780#note-26,
Le code de pré-blocage peut fonctionner, mais plus mal, sans l'index, les gens qui n'ont pas une base à jour auront juste des bugs qu'on aura pas.
Identifiés, ils peuvent être pris en compte dans une nouvelle version de ce patch ? Ou bien il s'agit de bug "quand même parfois sous forte charge il y aura une réservation qui passera" ?
Aussi/surtout, à passer sur les chaines à traduire j'ai vu "lock_code does not work with events" qui correspond à ce commentaire plus haut :
PS: j'ai aussi complètement bloqué la fonctionnalité pour le cas des évènements, je n'avais pas de test sous la main et je me suis dit que ce serait mieux de reprendre ça à froid si ça devient nécessaire un jour.
Les grosses affluences ça a souvent été pour les événements, je pense par exemple aux piscines à Toulouse.
De la description du ticket d'ailleurs : « choisir un événement » (c'est plus loin qu'il commence à être question de rendez-vous et conflits de réservation).
Updated by Benjamin Dauvergne over 1 year ago
- Assignee deleted (
Benjamin Dauvergne)
Je me débranche de ce ticket, je préfère laisser ça à la la team chrono.
Updated by Olivier Renard 3 months ago
Je me permets de réveiller ticket, suite à #75543.
Je ne sais pas si le préblocage de rendez vous est couvert dans ce ticket.
Updated by Benjamin Dauvergne 3 months ago
Olivier Renard a écrit :
Je me permets de réveiller ticket, suite à #75543.
Je ne sais pas si le préblocage de rendez vous est couvert dans ce ticket.
Justement ça ne couvre que les rendez-vous, pour moi ce n'était pas un problème.
Updated by Olivier Renard 3 months ago
Oui, peut être qu'il faut dissocier le préblocage d'un rdv et le préblocage d'une place d'un événement (je ne connais pas la complexité technique à le faire).
Il n'y a pas la même criticité.
Une événement a souvent un nombre de places, a potentiellement une file d'attente, ce qui donne moins de criticité entre le choix de la place dans le formulaire et l'appel webservice de réservation.
Un rdv a plus de criticité (de mon point de vue).
Updated by Pierre Cros 3 months ago
La description du ticket c'est les événements et Fred citait l'exemple des piscines prises d'assaut.
Mais c'est vrai qu'un cas d'usage souvent évoqué pour ce lock c'est les RdV pour avoir une carte de séjour qui sont épuisés dès la mise à disposition des créneaux avec un trafic (dans tous les sens du terme) de dingue.
Et bien sûr c'est mieux d'avoir ça au moins pour les RdV que de ne rien avoir du tout. Après ce serait pas mal que ce qui est fait pour les RdV ne soit pas à détricoter le jour où on se souciera des événements, quand même. Je ne sais pas si cette dimension peut être prise en compte.
Updated by Benjamin Dauvergne 3 months ago
Pierre Cros a écrit :
Et bien sûr c'est mieux d'avoir ça au moins pour les RdV que de ne rien avoir du tout. Après ce serait pas mal que ce qui est fait pour les RdV ne soit pas à détricoter le jour où on se souciera des événements, quand même. Je ne sais pas si cette dimension peut être prise en compte.
C'est totalement indépendant, il faudra une solution complètement différente de toute façon, parce que déjà gérer les évènements et les rdv dans la même application n'avait pas de sens même initialement, on aurait pu faire deux applications. Le partage des modèles est totalement artificiel, il n'y a rien de commun dans le code entre les deux, à part qu'un évènement et un rdv ont des date de début et de fin.
Updated by Benjamin Dauvergne 3 months ago
Le principal souci pour les événements c'est de déterminer un comportement quand une liste d'attente est configurée. Des gens pourront se retrouver en liste d'attente alors que les réservations précédentes verrouillées se libérent. Il faudrait une gestion automatique de la liste d'attente pour repasser ces personnes sur la liste principale avant de nouvelles réservations; et il faudrait pouvoir notifier w.c.s. de ce changement sans que celui-ci ait à poller l'état de la réservation. Comportement qui aurait son intérêt même pour le cas des annulations. Ou alors ne pas accepter de réservation en liste d'attente tant que la liste principale n'est pas rempli de réservations fermes.
Updated by Robot Gitea 2 months ago
- Status changed from Nouveau to En cours
Benjamin Dauvergne (bdauvergne) a ouvert une pull request sur Gitea concernant cette demande :
- URL : https://git.entrouvert.org/entrouvert/chrono/pulls/58
- Titre : WIP: "préblocage" d'une réservation (#17685)
- Modifications : https://git.entrouvert.org/entrouvert/chrono/pulls/58/files
Updated by Benjamin Dauvergne 2 months ago
J'ai rebasé mon code et je suis reparti en partie du concept de Thomas, Lease est désormais un modèle fils de Booking ça donne le support sur les rendez-vous et les évènements. Je dois rajouter encore des tests sur les évènements et faire le tour des nouvelles APIs pour voir si ça doit impacter ou pas leur fonctionnement.
api: add lock_code parameter to fillslot and datetimes (#17685)