La méthode clean() dans forms.py et Event.delete() mériteraient une chacune une petite docstring sur le pourquoi.

La property duration sur Event ne semble pas utilisé dans ce patch bien qu'elle y soit ajouté, pareil pour date et time (je suppose que c'est pour simplifier d'autres endroits dans le code mais faudra le mettre dans ce patch là alors).

La property sur l'élément HTML pour savoir s'il faut un bouton delete ou pas je l'appellerai simplement 'data-can-delete="true/false" de plus jQuery convertit automatiquement les valeurs en JS (i.e. on peut faire if ($(x).data('can-delete')) on est sûr de récupérer un booléen).

Il y a un bout de code qui m'interroge un peu:

            self.canceled = True
            if hasattr(self, 'eventwithact'):
                # Needed
                self.eventwithact.canceled = True

Le champ canceled existe dans les deux classes ou c'est un autre problème ?

Concernant les méthodes one_act_already_billed, etc.. j'aurai juste fait une méthodes acts(self) qui renverrait, (normalement défini sur EventWithAct mais j'ai l'impression qu'on est pas certain de toujours en avoir un):

Act.objects.filter(models.Q(parent_event=self)|models.Q(parent_event__exception_to=self))

Les autres n'étant plus que deux filtres à appliquer related_acts().filter(is_billet=True).exists(). Ces filtres sont éventuellement à mettre dans un ActQuerySet à créer dans calebasse/actes/managers.py plutôt que dans Event.

Je vais committer mes modifs avec quelques corrections suite à ton commentaire car ils sont nécessaires pour ensuite appliquer des corrections sur la base. Par contre je vais poursuivre les modifs en fonction de test commentaires.

• duration est utilisé dans le template.
• J'ajouterai des docstrings
• ok pour la modif du js mais plutôt avec hide-delete est un test if (!$(this).data('hide-delete')) ?
• pour le forçage à self.eventwithact.canceled = True j'ai rencontré un fonctionnement particulier que j'ai contourner comme cela, il faut que je vérifie à nouveau si cela est requis.
• pour le ActQuerySet à créer dans calebasse/actes/managers.py, peux-tu préciser pouquoi tu le vois là plutôt que dans Event/EventQuerySet ?

Mikaël Ates a écrit :

• pour le ActQuerySet à créer dans calebasse/actes/managers.py, peux-tu préciser pouquoi tu le vois là plutôt que dans Event/EventQuerySet ?

Mon commentaire n'est effectivement pas précis, mon idée c'est que plutôt que d'avoir des méthodes comme ça:

class Event:
    ....
    def has_acts_already_billet():
        return self.related_acts().filter(is_billed=True).exists()

j'aurai vu

class ActQuerySet(QuerySet):
   def is_billed(self):
       return self.filter(is_billed=True).exists()

en ensuite dans le reste du code des:

     event.related_acts().is_billed()

C'est vraiment qu'une question de style, l'idée de fond étant de conserver au niveau de chaque modèle les méthodes qui le concernent, au cas où on voudrait s'en resservir dans un contexte totalement différent; par exemple ici sans relation avec un Event.

Autre remarque vraiment très très générale sur le style mais un truc que j'ai bien aimé dans le code de Cliss21 et que je n'avais jamais vu faire dans des applications Django, c'est l'habitude de stocker des filtres de modèles comme des constantes de classe sur les modèles comme ceci:

class Act(Model):
    BILLED = models.Q(is_billed=True)
    NOT_BILLED = models.Q(is_billed=False)

    ALREADY_BILLED = models.Q(is_already_billed=True)
    NOT_ALREADY_BILLED = models.Q(is_already_billed=False)

    BLOCKING_DELETION = BILLED | NOT_ALREADY_BILLED

Et ensuite on écrit plus que .filter(Act.BILLED | Act.NOT_ALREADY_BILLED) ou plus court .filter(Act.BLOCKING_DELETION) (j'invente totalement le critère) ce qui rend le code vraiment agréable à lire je trouve.

Mes suggestions n'étant que sur la forme je ne crois pas que cela nécessite de conserver ce ticket ouvert. Il y a juste le self.eventwithact.canceled = True pour lequel il faudrait voir si ça sert à quelque chose parce que c'est un peu perturbant comme code.