Produits Entr'ouvert » Combo

Bien sûr #19370, il faudrait commencer à y noter des éléments qui justifient l'existence de la cellule, les manques côté cellule json ou API chrono.

_~~

Ensuite, la description du ticket n'est pour moi pas nette :

Le planning de crèche présente aujourd'hui la semaine courante et la suivante. Il faudrait que le premier jour apparaissant soit aujourd'hui et les 6 suivants (samedi et dimanche inclus comme c'est le cas actuellement). La semaine suivante affichera de J+7 à J+14.

Déjà pour moi la situation actuelle s'est uniquement la semaine courante. Ensuite, une semaine, pour moi, c'est lundi → dimanche. Quand il est question de "la semaine suivante", on parle de quoi ici ? D'un "écran" suivant ? Un écran quand on appuie sur un bouton "suivant" ? Ou la semaine suivante c'est "du 25/10 au 30/10" ?

Les mockups en tout cas ne répondent pas à ces questions. (Et je n'avancerais pas sans que soit nettement validée une proposition complète).

Totalement net par contre, le ticket dit "samedi et dimanche inclus" et le patch dit "exclude saturday and sunday" (en passant : majuscules aux noms de jour en anglais).

_~~

Je zappe donc les commentaires sur le patch pour le moment.

Le calendrier doit pouvoir afficher toutes les disponibilités que Chrono annonce, la pagination ne doit pas disparaitre.

+def get_chunk_ref(event_datetime):
+    chunk = 1
+    today = now().date()
+    while chunk < 4:  # ignore if beyond a month in the future
+        future = today + datetime.timedelta(days=chunk * 7)
+        if today <= event_datetime.date() < future:
+            return future
+        chunk += 1

"ignore if beyond a month in the future" ? je ne comprends pas le sens; ça refuse la possibilité de réservations au-delà du mois ? (et si jamais chrono était configuré ainsi, ça tombe sur un retour de None et ça casse tout derrière ?)

J'ai supprimé cette logique dans ce nouveau patch. Je me base sur la différence de jours entre le jour max et le jour min reçu en faisant des groupes de 7 jours.

J'avais le souvenir que le nombre de jours affichés par la cellule serait configurable, cf "Sur 14 jours paramétrable" dans #19452. C'est quelque chose qu'on ne retrouve pas ici (et c'est ton commentaire sur l'hardcodage de "7 jours" qui m'y fait penser).

chunk_size a verbose_name='Number of days to display'.
On ne peut pas donner un nom plus clair à la variable?

var self = $(this);

Par convention on nomme les variables jquery avec $, ça serait donc $self.
Et donc autant de ne pas rénommer la variable, plutôt $this.

Em...

        if not self.days:
            return    
            yield

Et même si ça fonctionnait.

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

Et même si ça fonctionnait.

Ok. Je vais juste renvoyer une liste.

def mocked_now():
    return now().replace(month=6, day=12)

Et dans deux mois ?

$elem.closest('div').html(data);

Ça m'effraie, un changement dans le balisage, un <div> ajouté pour aider à une mise en page et bim ce ne serait plus le <div> attendu.

combo/apps/calendar/static/css/calendar.css
combo/apps/calendar/static/js/calendar.js

Je veux bien prendre ça comme la nécessité d'un système de compilations complexe, qui minimiserait le nombre de requêtes HTTP à faire. Mais je ne vois personne pour s'en occuper, du coup ça m'irait vraiment bien qu'on évite. (mais je vais laisser pisser…)

J'ai un peu commencé à voir ce patch avec #19924, le rechargement la cellule marche mais il y'a le retour vers la bonne cellule qu'il faudrait que je voye.

Je ne sais pas ce que tu veux dire par là; si ça n'intervient pas dans la réalisation de ce patch, ne nous écartons pas.

_~~

        # find the closest superior value divisible by days_displayed
        days_range = int(self.days_displayed * math.ceil(float(days_diff + 1) / self.days_displayed))

L'intention finale, au-delà du commentaire, il me semble que c'est fournir à la pagination des pages de même taille; s'il y a J et J+1 fournir une seule page qui contiendra de J à J+6; s'il y a des rendez-vous de J à J+10, fournir deux pages, pour aller de J à J+13. Si c'est bien ça, je préférerais que ça soit écrit, plutôt qu'avoir à retrouver la volonté derrière le calcul.

Surtout que cet objet reste curieux, il dépend fortement de son "self.days" (qui est "les jours avec des événements") mais cet attribut se trouve modifié depuis l'extérieur de l'objet (dans get_calendar()).

-    response = requests.get('api/agenda/%s/datetimes/' % chrono_slug, remote_service=chrono, without_user=True)
+    params = {'date_start': now().date().isoformat()}
+    response = requests.get('api/agenda/%s/datetimes/' % chrono_slug, params=params,
+                            remote_service=chrono, without_user=True)

Pourquoi cette modification ? L'API datetimes ne doit jamais fournir de dates dans le passé.

class CalendarContentAjaxView(SingleObjectMixin, View):
    http_method_names = ['get']
    model = BookingCalendar
    template_name = 'calendar/booking_calendar_content.html'
    [...]

Trouver un objet et en faire le rendu à l'aide d'un template, n'est-ce pas exactement DetailView ? Plutôt à mon sens utiliser directement cette classe.

get_paginated_calendar qui modifie et retourne un contexte, ça ne sonne pas terrible.

        days_range = int(self.days_displayed * math.ceil(float(days_diff + 1) / self.days_displayed))
        if not days_range:
            days_range = self.days_displayed  # minimum

On entre comment dans cette condition ? (réponse : si la date la plus éloignée dans le futur vient avant la date la plus proche, non ?).

La pagination, la vue CalendarContentAjaxView, ne sont absolument pas testées ?

Josué Kouka a écrit :

J'ai rajouté des tests.

Une erreur dans les tests que je suis en train de corriger

        if self.days[0].date > today:
            base_day = self.days[0].date
        else:
            base_day = today

Soit self.days[0].date est plus tard qu'aujourd'hui, alors on prend cette date, soit pas, et alors on prend aujourd'hui, mais ce cas-là n'est-il pas également le cas où self.days[0].date == today ? (vraiment il serait utile que self.days porte un nom plus explicite, ou que sa déclaration soit accompagnée d'un commentaire, là plutôt que repasser par le code pour comprendre à nouveau à quoi ça correspondait, je me suis juste basé sur mon commentaire précédent ("self.days" (qui est "les jours avec des événements"))). Bref, si c'est bien ça, il me semble que ça pourrait tout le temps être base_day = self.days[0].date.

Tout à fait en lien avec ça, je remonte à un commentaire précédent, où je citais le ticket : Le planning de crèche présente aujourd'hui la semaine courante et la suivante. Et où je m'interrogeais : « une semaine, pour moi, c'est lundi → dimanche. ».

Ici, si je comprends bien, on fait des tas de sept jours (well, self.days_displayed jours) à partir du jour courant (/ du premier jour où des réservations sont possibles), pas à partir du début de la semaine ("du lundi de la semaine").

Est-ce bien ça qui est attendu ?

la pagination, la vue CalendarContentAjaxView, ne sont absolument pas testées

Me semble qu'il y a juste eu la pagination, pas la vue CalendarContentAjaxView, à avoir gagné des tests.

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

[...]

Soit self.days[0].date est plus tard qu'aujourd'hui, alors on prend cette date, soit pas, et alors on prend aujourd'hui, mais ce cas-là n'est-il pas également le cas où self.days[0].date == today ? (vraiment il serait utile que self.days porte un nom plus explicite, ou que sa déclaration soit accompagnée d'un commentaire, là plutôt que repasser par le code pour comprendre à nouveau à quoi ça correspondait, je me suis juste basé sur mon commentaire précédent ("self.days" (qui est "les jours avec des événements"))). Bref, si c'est bien ça, il me semble que ça pourrait tout le temps être base_day = self.days[0].date.

Ok je prends note.

Tout à fait en lien avec ça, je remonte à un commentaire précédent, où je citais le ticket : Le planning de crèche présente aujourd'hui la semaine courante et la suivante. Et où je m'interrogeais : « une semaine, pour moi, c'est lundi → dimanche. ».

Ici, si je comprends bien, on fait des tas de sept jours (well, self.days_displayed jours) à partir du jour courant (/ du premier jour où des réservations sont possibles), pas à partir du début de la semaine ("du lundi de la semaine").

Est-ce bien ça qui est attendu ?

Oui c'est ce qui est attendu.

la pagination, la vue CalendarContentAjaxView, ne sont absolument pas testées

Me semble qu'il y a juste eu la pagination, pas la vue CalendarContentAjaxView, à avoir gagné des tests.

Dans le test sur la pagination, la vue CalendarContentAjaxView est appelée (ligne 267)

Ok je prends note.

Et ?

Dans le test sur la pagination, la vue CalendarContentAjaxView est appelée (ligne 267)

Ok, raté ça mais c'est alors le rendu de la cellule paginée hors ajax qui manque.

Ce rendu est stocké dans la variable page à la ligne 253.

À cette ligne, un appel sans pagination : page = client.get('/booking/').

(je note #19949 et #19950 en passant)

En navigant via ajax, les cases perdent leur fonctionnement js et il devient possible de cocher n'importe comment.

<a class="next calchunk" data-content-url="{% url 'ajax-calendar-content' pk=cell.pk }?chunk_{{cell.pk}}={{ calendar_days.next_page_number }}">{ trans "next" %}</a>

Étant donné la navigation non-ajax possible et désormais testée, ces liens previous/next pourraient gagner un href. Ça permettrait ainsi de dégager ce calendar.css qui me fatigue. Et dans le même temps, étant donné la piqûre de rappel sur l'existence combo.public.js, le code de calendar.js pourrait y être déplacé, et le fichier dégagé.

<a class="next calchunk" href="{% url 'ajax-calendar-content' pk=cell.pk }?chunk_{{cell.pk}}={{ calendar_days.next_page_number }}">{ trans "next" %}</a>

L'URL en href ne doit pas être la vue ajax.

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

<a class="next calchunk" href="{% url 'ajax-calendar-content' pk=cell.pk }?chunk_{{cell.pk}}={{ calendar_days.next_page_number }}">{ trans "next" %}</a>

L'URL en href ne doit pas être la vue ajax.

Ok, est ce que l'on doit avoir un truc dans le genre <a class="next calchunk" href="#" data-content-url="{% url 'ajax-calendar-content' pk=cell.pk %}?chunk_{{cell.pk}}={{ calendar_days.next_page_number }}">{% trans "next" %}</a> ?

Un clic du milieu sur le bouton "suivant" doit ouvrir un nouvel onglet avec la même page, mais avec le calendrier positionné une semaine plus loin.

Un clic du milieu sur le bouton

Et quand j'écris "un clic du milieu" c'est pour dire "le comportement natif du navigateur sans javascript", ce n'est pas "il faut attraper un event click et détecter le bouton pressé etc.".

i.e. si l'URL présente est https://combo.fred.local.0d.be/foo/blah/calendrier/ l'URL en href du suivant doit être https://combo.fred.local.0d.be/foo/blah/calendrier/?chunk_1=2, tout simplement.

Bien sûr ça échoue s'il y a navigation simultanée dans plusieurs calendriers mais tant pis. ack.