Projet

Général

Profil

Bug #68729

Mauvaises plages horaires générées par Chrono lorsqu'un agenda virtuel est utilisé avec des plages exceptions

Ajouté par A. Berriot il y a plus d'un an. Mis à jour il y a plus d'un an.

Statut:
Nouveau
Priorité:
Normal
Assigné à:
-
Catégorie:
-
Version cible:
-
Début:
05 septembre 2022
Echéance:
% réalisé:

0%

Temps estimé:
Patch proposed:
Non
Planning:
Non

Description

Pour reproduire le problème:


@pytest.mark.freeze_time('2022-09-04 08:00')  # Sunday
def test_datetimes_api_meetings_virtual_agenda_with_exclusions(app):
    agenda1 = Agenda.objects.create(
        label='Agenda 1', kind='meetings', minimal_booking_delay=0, maximal_booking_delay=7
    )
    agenda2 = Agenda.objects.create(
        label='Agenda 2', kind='meetings', minimal_booking_delay=0, maximal_booking_delay=7
    )
    desk1 = Desk.objects.create(agenda=agenda1, label='desk1')
    desk2 = Desk.objects.create(agenda=agenda2, label='desk2')
    meeting_type1 = MeetingType.objects.create(agenda=agenda1, label='meeting_type', duration=20)
    meeting_type2 = MeetingType.objects.create(agenda=agenda2, label='meeting_type', duration=20)

    time_period1 = TimePeriod.objects.create(
        desk=desk1,
        weekday=0,
        start_time=datetime.time(8, 10),
        end_time=datetime.time(9, 50),
    )
    time_period2 = TimePeriod.objects.create(
        desk=desk2,
        weekday=0,
        start_time=datetime.time(8, 15),
        end_time=datetime.time(9, 55),
    )

    virtual_agenda = Agenda.objects.create(label='Virtual Agenda', kind='virtual')
    virtual_agenda.real_agendas.add(agenda1, agenda2)

    api_url = '/api/agenda/%s/meetings/%s/datetimes/' % (virtual_agenda.slug, meeting_type1.slug)
    resp = app.get(api_url)
    assert [x['datetime'] for x in resp.json['data']] == [
        '2022-09-05 08:10:00',
        '2022-09-05 08:15:00',
        '2022-09-05 08:30:00',
        '2022-09-05 08:35:00',
        '2022-09-05 08:50:00',
        '2022-09-05 08:55:00',
        '2022-09-05 09:10:00',
        '2022-09-05 09:15:00',
        '2022-09-05 09:30:00',
        '2022-09-05 09:35:00',
    ]

    TimePeriod.objects.create(
        weekday=0, start_time=datetime.time(8, 10), end_time=datetime.time(8, 20), agenda=virtual_agenda
    )

    resp = app.get(api_url)
    assert [x['datetime'] for x in resp.json['data']] == [
        '2022-09-05 08:30:00',
        '2022-09-05 08:35:00',
        '2022-09-05 08:50:00',
        '2022-09-05 08:55:00',
        '2022-09-05 09:10:00',
        '2022-09-05 09:15:00',
        '2022-09-05 09:30:00',
        '2022-09-05 09:35:00',
    ]

Ça plante sur le deuxième assert et reproduis le comportement (erroné) visible dans #68703

Historique

#2

Mis à jour par A. Berriot il y a plus d'un an

l'origine du problème est quelque part dans la fonction get_all_slots() mais elle fait 350 lignes donc pour le moment je n'ai pas réussi à isoler plus que ça

#3

Mis à jour par Frédéric Péters il y a plus d'un an

L'intention est peut-être erronée, pour être complet le résultat actuellement obtenu est :

['2022-09-05 08:20:00', '2022-09-05 08:40:00', '2022-09-05 09:00:00', '2022-09-05 09:20:00']

et ça correspond à des créneaux qui peuvent être pris dans tous les agendas, vu qu'ils sont tous les 2 configurés, in fine, pour ouvrir à 8h20 et offrir des créneaux de 20 minutes.

Je pense qu'il peut y avoir un problème à la base sur ce qui est attendu, qui me semble être que les créneaux restent calculés par rapport à leurs agendas indépendants.

Pour moi il faut prendre beaucoup de recul sur ce ticket, convoquer toutes les intentions et cas d'usages qui ont mené aux agendas virtuels, avant même de considérer qu'il y a un bug.

#4

Mis à jour par Pierre Cros il y a plus d'un an

Le comportement attendu me semble assez clair : https://dev.entrouvert.org/issues/68703#note-4

L'utilisation de l'agenda virtuel vient du fait qu'on ne peut pas utiliser un agenda unique avec plusieurs guichets quand on souhaite pouvoir réserver spécifiquement sur l'un ou sur l'autre.

Et donc création de 3 agendas (à la place de 3 guichets), puis d'un agenda virtuel pour partager les exceptions entre les 3 (pour ne pas avoir à positionner manuellement les exceptions sur chacun des agendas physiques et pour permettre d'exposer à l'usager un agenda unique)

Je peux entendre que le comportement est normal si on m'explique mais c'est de toute façon peu intuitif que de voir les créneaux offerts changer sur des périodes qui ne sont pas concernées par l'exception.

#5

Mis à jour par Frédéric Péters il y a plus d'un an

Le comportement attendu me semble assez clair : https://dev.entrouvert.org/issues/68703#note-4

Oui mais je parle de la nécessité de dépasser ce cas, prendre du recul.

J'aimerais parler en me limitant au cas peut-être simplifié de ce ticket, créé pour ressembler à la situation d'origine mais pas nécessairement en tout point semblable. Pour exposer ce ticket la situation est un agenda avec des créneaux de 20 minutes et un seul guichet qui ouvre à 8h10; un autre agenda avec des créneaux pareil 20 minutes et un seul guichet qui ouvre à 8h15. Ces deux agendas sont ensuite réunis dans un agenda virtuel. Et dans cet agenda virtuel une exclusion est posée, de 8h10 à 8h20.

Ce que ça crée actuellement, ce sont des créneaux de 20 minutes, qui commencent à 8h20, qui réservés, iront sur l'un ou l'autre agenda.

Le changement exprimé ce serait que plutôt il y ait des créneaux 8h30, 8h50, 9h10, etc. (tous les créneaux agendas 1 après 8h20) et 8h35, 8h55, 9h15, etc. (tous les créneaux agendas 2 après 8h20).

On a peut-être existé jusqu'à présent avec l'idée que les agendas virtuels étaient totalement alignés, on imposait l'identique pour la durée/type de rendez-vous, on a peut-être oublié de la réflexion l'heure de début que ça donnait.

Pour rester sur le cas de ce ticket, avant l'exclusion, les créneaux proposés sont 8h10, 8h15, 8h30, 8h35, 8h50, 8h55, etc. ça sonne pour moi assez étrange et pas conforme à cette idée que je me faisais des agendas virtuels : exposer des créneaux communs/possibles sur différents agendas. Pour moi la réponse aurait très bien pu être 8h10, 8h30, 8h50, 9h10, etc. : ça me semblerait adéquat d'avoir des créneaux de 20 minutes présentés à l'usager, pour celui de 8h10 ça ira nécessairement sur l'agenda 1 mais les autres iront vers l'un ou l'autre. Mais on pourrait peut-être dire que par rapport à l'horaire d'ouverture de l'agenda 2 on a les premières 15 minutes jamais occupées (vu que le premier créneau proposé dessus est à 8h30), c'est vrai.

Avec l'exclusion posée, il y a alignement des horaires de début, donc 8h20, 8h40, 9h, etc. Les réservations qui sont prises ne gaspillent rien. Avec changement de comportement il y aurait un premier rendez-vous possible 8h30 (premier agenda, on y perd 10 minutes) et un rendez-vous à 8h35 (second agenda, on y perd 15 minutes).

Cela pour peut-être dire que le critère "minutes perdues" n'est peut-être pas optimal. (oui je sais que personne n'en a parlé)

Pour revenir sur la conception des agendas virtuels, dans la documentation « les créneaux exposés par l'agenda virtuel seront la somme de tous les créneaux disponibles dans les agendas inclus » donc clairement ça n'y est pas ma perspective, et quelque chose en adéquation avec le changement souhaité ici.

Pour rester sur la documentation, les utilisations évoquées sont :

  • "Proposer les premiers créneaux disponibles parmi l’ensemble des agendas inclus dans l'agenda virtuel", c'est peut-être de loin la nécessité de droits individuels en visualisation d'un agenda, pas tout à fait sûr.
  • "Réserver des créneaux uniquement pour enregistrement par les agents", c'est le cas avec une exclusion posée côté agenda virtuel pour par exemple laisser les créneaux du vendredi réservables uniquement par les agents (qui utilisent eux directement les agendas, pas l'agenda virtuel).

Pour ces deux utilisations je n'ai pas l'impression qu'il y ait blocage à faire évoluer le comportement pour totalement correspondre à "la somme de tous les créneaux disponibles". (mais il y a d'autres usages en pratique ?)

Le pire truc que j'imagine comme conséquence fonctionnelle à ce changement c'est du temps de pause gagné grâce à des créneaux qui ne commenceraient pas directement après la période d'exclusion. (et qui se plaindra de temps de pause ?).

Par contre, pire conséquence technique possible, ce serait une explosion des performances, parce que sur un usage actuel comme le nord avec un agenda virtuel avec une dizaine d'agendas d'agents derrière, si jamais on se trouvait devoir calculer agenda par agenda, ça diviserait gravement les perfs. (mais peut-être qu'on n'aura pas à faire à ça)

Pour rejoindre Agate,

l'origine du problème est quelque part dans la fonction get_all_slots() mais elle fait 350 lignes donc pour le moment je n'ai pas réussi à isoler plus que ça

Totalement incapable également d'apporter la moindre certitude sur les changements qu'implirait ce ticket, et encore moins les conséquences possibles sur les perfs.

Aussi c'est juste mon recul sur ce ticket je reste intéressé par la lecture que d'autres personnes en feraient.

#6

Mis à jour par Lauréline Guérin il y a plus d'un an

Plus précisément, ça se situe dans la méthode get_effective_time_periods_virtual:

                for weektime_interval in IntervalSet.simple(*time_period_interval) - closed_hours_by_days:
                    yield SharedTimePeriod.from_weektime_interval(weektime_interval, desks=desks)

Ce qui fait bien démarrer la période au début de l'exclusion, pour tous les agendas/desks/timeperiod de l'exemple. On ne respecte pas ici les créneaux d'origine, donnés par le meeting_type demandé.
(et c'est pas illogique)

Si on voulait respecter les créneaux d'origine, et juste enlever ceux qui sont incompatibles avec les exclusions, on pourrait faire un truc comme ça pour ajuster le démarrage de la période en sautant au créneau suivant:

                for weektime_interval in IntervalSet.simple(*time_period_interval) - closed_hours_by_days:
                    begin = weektime_interval.begin
                    weektime_begin = datetime.datetime.combine(datetime.date.min, begin.time)
                    period_begin = datetime.datetime.combine(
                        datetime.date.min, time_period_interval.begin.time
                    )
                    begin_delta = (weektime_begin - period_begin).seconds / 60
                    if begin_delta % meeting_type.duration:
                        # adjust begining of the interval
                        weektime_begin += datetime.timedelta(
                            minutes=-(begin_delta % meeting_type.duration) + meeting_type.duration
                        )
                        begin = WeekTime(begin.weekday, begin.weekday_indexes, weektime_begin.time())
                    yield SharedTimePeriod.from_weektime_interval(
                        Interval(begin, weektime_interval.end), desks=desks
                    )

(à reprendre, il est tard)
Niveau perf, ça me semble acceptable ?

#7

Mis à jour par Pierre Cros il y a plus d'un an

Peut-être pas nécessaire après l'intervention de Lauréline mais je réponds à 2/3 trucs.

Frédéric Péters a écrit :

Avec l'exclusion posée, il y a alignement des horaires de début, donc 8h20, 8h40, 9h, etc. Les réservations qui sont prises ne gaspillent rien. Avec changement de comportement il y aurait un premier rendez-vous possible 8h30 (premier agenda, on y perd 10 minutes) et un rendez-vous à 8h35 (second agenda, on y perd 15 minutes).

Cela pour peut-être dire que le critère "minutes perdues" n'est peut-être pas optimal. (oui je sais que personne n'en a parlé)

Personne n'en a parlé mais tu fais bien de le faire puisque ça pourrait surgir ailleurs. Cela étant je suis pas certain d'être d'accord avec ta façon de compter, je me fais des nœuds dès qu'on a plusieurs types de RdV et plus simplement des RdV de 20 min (ça dépasse le cas d'usage du ticket mais tu voulais penser at large).

Pour ces deux utilisations je n'ai pas l'impression qu'il y ait blocage à faire évoluer le comportement pour totalement correspondre à "la somme de tous les créneaux disponibles". (mais il y a d'autres usages en pratique ?)

Rien que j'ai identifié et qui serait problématique mais c'est pas une garantie.

Le pire truc que j'imagine comme conséquence fonctionnelle à ce changement c'est du temps de pause gagné grâce à des créneaux qui ne commenceraient pas directement après la période d'exclusion. (et qui se plaindra de temps de pause ?).

On peut compter sur de zélés lean managers mais on ne travaille pas pour eux.

Par contre, pire conséquence technique possible, ce serait une explosion des performances, parce que sur un usage actuel comme le nord avec un agenda virtuel avec une dizaine d'agendas d'agents derrière, si jamais on se trouvait devoir calculer agenda par agenda, ça diviserait gravement les perfs. (mais peut-être qu'on n'aura pas à faire à ça)

Le max derrière un agenda virtuel au Nord c'était 5 agendas physiques quand j'ai fait le truc, je serai surpris que ça ait beaucoup changé. Et surtout il n'y a pas l'hétérogénéité qu'on a ici au niveau des horaires de démarrage.

#9

Mis à jour par Emmanuel Cazenave il y a plus d'un an

En relisant le ticket de départ #68503, ils sont bien sur un cas d'usage standard de l'agenda virtuel : masquer des créneaux aux usagers via un agenda virtuel pour que ces créneaux soit alloués directement par des agents (via l'agenda réel).

Dans cette configuration (un agenda réel avec plusieurs guichets aux périodes horaires différentes, un agenda virtuel qui inclue uniquement cet agenda, avec des d'exclusion), ils constatent des "horaires pas respectés".

Ce ticket se termine en queue de poisson sur une histoire d'impossibilité de sélectionner le guichet sur lequel sera pris le rdv et ils disent "ah ok". Mais nulle part ils n'expriment le besoin de choisir un guichet, la conclusion me semble être une incompréhension mutuelle, et de là ça embraye sur #68703 qui est une configuration différente (plusieurs agendas réels avec un guichet chacun, un agenda virtuel).

Tout ça pour dire qu'on devrait repartir de leur première configuration et aussi creuser ce qui se cache derrière le "horaires pas respectés" parce qu'on ne sait pas s'ils sont juste surpris par le fonctionnement ou si ça leur pose un vrai problème du type temps de guichet non utilisé.

#10

Mis à jour par Frédéric Péters il y a plus d'un an

"horaires pas respectés"

Je l'explique dans #68503#note-18; en simplifiant, il y a deux guichets, un qui ouvre à 8h10, un autre à 8h15 et des rendez-vous de dix minutes. On a un calcul qui fait que les débuts de créneaux sont 8h10, 8h15, 8h20, 8h25, etc. l'usager qui choisit le créneau 8h25, sa réservation va aller au premier guichet où il y a de la place à 8h25, ça va être le guichet 1. Alors qu'il était imaginé/souhaité que ça parte sur le guichet 2, sur l'idée que le guichet 1 proposait 8h10, 8h20, 8h30, et guichet 2 8h15, 8h25, 8h35.

#11

Mis à jour par Emmanuel Cazenave il y a plus d'un an

Oui oui mais je voulais insister sur le "il était imaginé/souhaité", on ne sait pas si ça leur pose un vrai problème dans l'organisation du travail des agents ou autre, ça n’apparaît pas clairement dans les échanges.

Ça me semble devoir être creusé, pour motiver ou non un changement de comportement dans chrono et un développeur à s'y coller.

#12

Mis à jour par Frédéric Péters il y a plus d'un an

Il y a une tentative d'expliciter les choses dans #68503#note-10 :

Elle souhaite un rendez vous à xxh10, le deuxième à xxh15, le troisième à xxh20 sachant qu'elle n'a que 2 agents,
et que l'agent qui traite le rdv de xxh10 gérera aussi le rdv de xxh20 car les rdv durent seulement 10 minutes en général
Mais elle ne souhaite pas créer seulement 2 guichets avec des rdv de 10 minutes, car le rdv peut être parfois plus long que 10 minutes,
et si le rdv dure seulement 10 minutes comme c'est souvent le cas, l'agent a d'autres actions à effectuer pendant les 10 minutes libres...

mais perso j'ai beau relire plusieurs fois pour paraphraser et expliciter en fait je n'arrive pas à être sûr de comprendre le sens. Peut-être qu'il y a une utilité fonctionnelle à avoir les débuts à des heures différentes, type c'est plus simple à gérer à l'accueil en pouvant se dire "les rendez-vous à la minute 5 c'est toujours guichet 2 ?

#13

Mis à jour par Pierre Cros il y a plus d'un an

Dans https://dev.entrouvert.org/issues/68503#note-19 j'écris à la suite de Fred :

Vous imaginez que :

    les rendez-vous 8h10, 8h30, 8h50 etc. vont aller guichet 1
    les rendez-vous 8h15, 8h35, 8h55 etc. vont aller guichet 2
    les rendez-vous 8h20, 8h40, 9h00 etc. vont aller guichet 3

Et c'est bien ce qui était imaginé Par Meyzieu, c'est pour cela qu'ils ferment le ticket, donc on sait ce qui est souhaité. Et personnellement je trouve que pouvoir traduire ce souhait en passant par un agenda virtuel et des périodes d'exclusion tiendrait la route conceptuellement alors que la situation actuelle est peu compréhensible.

Tout cela est indépendant du "pourquoi Meyzieux fait cela ?". Si on veut savoir pourquoi c'est souhaité par Meyzieu sous cette forme, il faut discuter avec le chef de service qui a demandé cette mise en place, c'est ce que proposait Bernard Arnaudon et j'ai pas embrayé parce que je fais du support et pas un projet.

Si Stéphane (CPF) souhaite faire un projet, il faudra en discuter avec lui j'imagine et vérifier que Meyzieu veut bien le financer.

#14

Mis à jour par Frédéric Péters il y a plus d'un an

les rendez-vous 8h10, 8h30, 8h50 etc. vont aller guichet 1
les rendez-vous 8h15, 8h35, 8h55 etc. vont aller guichet 2;
les rendez-vous 8h20, 8h40, 9h00 etc. vont aller guichet 3

et donc l'agenda virtuel fonctionne pour eux.

Jusqu'au moment où ils y ajoutent une période d'exclusion, qui fait commencer tous les guichets à la même heure, et donc tous les créneaux proposés démarrent sur cette heure d'ouverture. Alors que le souhait est de garder le décalage des 5 minutes.

Et le "je pose une exclusion sur l'agenda virtuel global pour ne pas ouvrir au public avant 9h parce qu'il y a des travaux exceptionnels à l'accueil" donnerait "en fait le guichet 1 va ouvrir à 9h mais le guichet 2 va commencer par une pause de 5 minutes, pour une ouverture à 9h05". (pour revenir à mes interrogations "temps de pause" plus haut).

#15

Mis à jour par Pierre Cros il y a plus d'un an

Frédéric Péters a écrit :

Et le "je pose une exclusion sur l'agenda virtuel global pour ne pas ouvrir au public avant 9h parce qu'il y a des travaux exceptionnels à l'accueil" donnerait "en fait le guichet 1 va ouvrir à 9h mais le guichet 2 va commencer par une pause de 5 minutes, pour une ouverture à 9h05". (pour revenir à mes interrogations "temps de pause" plus haut).

C'est bien ce qui est souhaité et tant pis pour les pauses. Les gens pour qui ces pauses sont un soucis devront faire des exceptions agenda physique par agenda physique (et perdre la possibilité d'exposer un truc différents aux agents, oui).

#16

Mis à jour par A. Berriot il y a plus d'un an

  • Assigné à A. Berriot supprimé

Formats disponibles : Atom PDF