entrouvert
/
chrono
14
1
Fork 0
Code Issues Pull Requests 8 Releases Wiki Activity

get_all_slots() : l'exclusion des créneaux pour un même user_external_id ne prend pas en compte tous les agendas (#75587) #59

Merged
bdauvergne merged 2 commits from wip/75587-get-all-slots-l-exclusion-des-cr into main 2023-03-24 15:48:11 +01:00
Conversation 7 Commits 2 Files Changed 2 +72 -2
bdauvergne commented 2023-03-20 00:15:36 +01:00
Owner
Copy Link
No description provided.
lguerin commented 2023-03-20 09:14:37 +01:00
Owner
Copy Link

Ca me va, tu pourrais écrire un test ?

Ca me va, tu pourrais écrire un test ?
bdauvergne commented 2023-03-20 09:23:49 +01:00
Author
Owner
Copy Link

Ca me va, tu pourrais écrire un test ?

Oui j'attendais qu'on soit d'accord sur le comportement avant de m'y mettre.

> Ca me va, tu pourrais écrire un test ? Oui j'attendais qu'on soit d'accord sur le comportement avant de m'y mettre.
bdauvergne force-pushed wip/75587-get-all-slots-l-exclusion-des-cr from 41b166d738 to 49e57af1b3 2023-03-20 11:04:03 +01:00 Compare
bdauvergne commented 2023-03-20 11:09:20 +01:00
Author
Owner
Copy Link

Voila j'ai ajouté un test, ça vérifie l'exclusion entre plusieurs agendas rdv avec des Timeperiod différentes (ça check le fait qu'on utilise plus agenda_ids, sans timeperiod différent on avait un seul agenda_ids et ça marchait, certainement pour cela qu'on a pas eu de souci particulier avec ça pour l'instant sur des agendas virtuels qui doivent représenter 99% des usages) et aussi sur un agenda RdV depuis un booking sur un agenda évènement. Pour ce dernier point ça a nécessité d'ajouter la récupération de Event.duration en plus de MeetingType.duration pour s'adapter.

Il faut vraiment être certain du comportement parce maintenant si j'ai pris un RdV sur l'agenda A avec mon compte pour ma fille, je ne peux plus prendre aucune agenda sur un agenda B qui n'a aucune rapport à la même heure pour mon fils (enfin ça dépend du user_external_id qui est utilisé mais si c'est le NameID ça va bloquer), si ces réservations utilisent l'exclusion par user_external_id.

Sur le datetimes des agendas évènements l'exclusion ne se fait que vis à vis des réservations du même évènement, je n'ai pas changé ce comportement.

Voila j'ai ajouté un test, ça vérifie l'exclusion entre plusieurs agendas rdv avec des Timeperiod différentes (ça check le fait qu'on utilise plus agenda_ids, sans timeperiod différent on avait un seul agenda_ids et ça marchait, certainement pour cela qu'on a pas eu de souci particulier avec ça pour l'instant sur des agendas virtuels qui doivent représenter 99% des usages) et aussi sur un agenda RdV depuis un booking sur un agenda évènement. Pour ce dernier point ça a nécessité d'ajouter la récupération de Event.duration en plus de MeetingType.duration pour s'adapter. Il faut vraiment être certain du comportement parce maintenant si j'ai pris un RdV sur l'agenda A avec mon compte pour ma fille, je ne peux plus prendre aucune agenda sur un agenda B qui n'a aucune rapport à la même heure pour mon fils (enfin ça dépend du user_external_id qui est utilisé mais si c'est le NameID ça va bloquer), si ces réservations utilisent l'exclusion par user_external_id. Sur le datetimes des agendas évènements l'exclusion ne se fait que vis à vis des réservations du même évènement, je n'ai pas changé ce comportement.
lguerin commented 2023-03-20 21:04:45 +01:00
Owner
Copy Link

jenkins fail, et la PR est toujours en draft

jenkins fail, et la PR est toujours en draft
bdauvergne commented 2023-03-21 08:55:34 +01:00
Author
Owner
Copy Link

jenkins fail, et la PR est toujours en draft

Oui c'est wip mais c'est un warning de pylint ça peut être commenté.

> jenkins fail, et la PR est toujours en draft Oui c'est wip mais c'est un warning de pylint ça peut être commenté.
lguerin commented 2023-03-21 08:56:46 +01:00
Owner
Copy Link

Non attends ça ne va pas du tout:
"il me semble que le but est d'exclure tous les créneaux de quelque agenda qu'ils viennent pour le même user_external_id."

C'est faux, on veut empêcher le user de réserver un même créneau sur l'agenda visé, pas sur tous les agendas qui existent.

Donc oui, le code ne fonctionne pas pour les agendas virtuels, et il faut corriger ça en filtrant les bookings sur tous les agendas de l'agenda virtuel, mais il ne faut pas empêcher la réservation d'un créneau sur un agenda rdv parce qu'il y a une réservation sur un agenda event à la même heure. Ce n'était pas le but de ce dev.

D'ailleurs, sur les endpoint datetimes réservé aux agendas de type events, on ne regarde que ce qui se passe sur les agenda concernés par cet appel: pas tous les agendas events, et encore moins les agendas meetings.

Non attends ça ne va pas du tout: "il me semble que le but est d'exclure tous les créneaux de quelque agenda qu'ils viennent pour le même user_external_id." C'est faux, on veut empêcher le user de réserver un même créneau sur l'agenda visé, pas sur tous les agendas qui existent. Donc oui, le code ne fonctionne pas pour les agendas virtuels, et il faut corriger ça en filtrant les bookings sur tous les agendas de l'agenda virtuel, mais il ne faut pas empêcher la réservation d'un créneau sur un agenda rdv parce qu'il y a une réservation sur un agenda event à la même heure. Ce n'était pas le but de ce dev. D'ailleurs, sur les endpoint datetimes réservé aux agendas de type events, on ne regarde que ce qui se passe sur les agenda concernés par cet appel: pas tous les agendas events, et encore moins les agendas meetings.
bdauvergne commented 2023-03-21 09:02:52 +01:00
Author
Owner
Copy Link

C'est faux, on veut empêcher le user de réserver un même créneau sur l'agenda visé, pas sur tous les agendas qui existent.

C'est pas faute d'avoir posé la question plusieurs fois dans le ticket et sur le salon, donc la fonctionnalité ne concerne vraiment que les agenda virtuels et dans ce cas il faut regarder tous les agendas le composant. J'ai compris.

> C'est faux, on veut empêcher le user de réserver un même créneau sur l'agenda visé, pas sur tous les agendas qui existent. C'est pas faute d'avoir posé la question plusieurs fois dans le ticket et sur le salon, donc la fonctionnalité ne concerne vraiment que les agenda virtuels et dans ce cas il faut regarder tous les agendas le composant. J'ai compris.
bdauvergne force-pushed wip/75587-get-all-slots-l-exclusion-des-cr from 49e57af1b3 to ae8de3dd39 2023-03-21 09:23:35 +01:00 Compare
lguerin reviewed 2023-03-21 09:28:32 +01:00
chrono/api/views.py
@ -275,2 +275,2 @@
.order_by('start_datetime', 'meeting_type__duration')
.values_list('start_datetime', 'meeting_type__duration')
.order_by('start_datetime', 'meeting_type__duration', 'duration')
.values_list('start_datetime', 'meeting_type__duration', 'duration')
lguerin commented 2023-03-21 09:28:32 +01:00
Owner
Copy Link

plus besoin de duration, si ?

plus besoin de duration, si ?
bdauvergne commented 2023-03-21 09:33:10 +01:00
Author
Owner
Copy Link

Retiré.

Retiré.
lguerin marked this conversation as resolved
bdauvergne force-pushed wip/75587-get-all-slots-l-exclusion-des-cr from ae8de3dd39 to c1bfeaa33a 2023-03-21 09:32:58 +01:00 Compare
bdauvergne changed title from WIP: get_all_slots() : l'exclusion des créneaux pour un même user_external_id ne prend pas en compte tous les agendas (#75587) to get_all_slots() : l'exclusion des créneaux pour un même user_external_id ne prend pas en compte tous les agendas (#75587) 2023-03-21 09:33:37 +01:00
bdauvergne force-pushed wip/75587-get-all-slots-l-exclusion-des-cr from c1bfeaa33a to dd9a28ff84 2023-03-21 09:39:05 +01:00 Compare
lguerin approved these changes 2023-03-21 09:43:26 +01:00
bdauvergne force-pushed wip/75587-get-all-slots-l-exclusion-des-cr from dd9a28ff84 to 22e8d00e31 2023-03-24 15:41:55 +01:00 Compare
bdauvergne merged commit 22e8d00e31 into main 2023-03-24 15:48:11 +01:00
bdauvergne deleted branch wip/75587-get-all-slots-l-exclusion-des-cr 2023-03-24 15:48:11 +01:00
Sign in to join this conversation.
Reviewers
No reviewers
lguerin
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: entrouvert/chrono#59
No description provided.
Delete Branch "wip/75587-get-all-slots-l-exclusion-des-cr"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?