Project

General

Profile

Bug #5233

Fonctionnement des périodiques

Added by Mikaël Ates almost 10 years ago. Updated over 8 years ago.

Status:
Rejeté
Priority:
Normal
Assignee:
-
Target version:
Start date:
30 July 2014
Due date:
% Done:

100%

Estimated time:
Patch proposed:
No
Planning:

Description

Rappels

  • today_occurrence est la fonction utilisée pour afficher les rdv sur les agendas. Elle retourne l'occurrence d'un périodique sous la forme d'une instance de Event. Si cette instance est sauvée, cela crée une exception au périodique.
  • Une exception n'a qu'un seul acte d'associé.

Principe à mettre en oeuvre

Un périodique n'est pas supprimable dès lors qu'il a d'associé, ou associé à une de ses exceptions, un acte déjà facturé (propriété already_billed d'un acte).
Lors de la 'suppression' d'un périodique, tous les actes qui lui sont associés sont supprimés, toutes les exceptions sont canceled, leur acte associé est supprimé (s'il existe), le périodique est canceled.

Un périodique n'est plus modifiable dès lors qu'il a d'associé, ou associé à une de ses exceptions, un acte déjà facturé (propriété already_billed d'un acte) à l'exception du commentaire, de la ressource, de la date de début et de la date de fin de récurrence.
La modification de la date de fin d'un périodique n'est possible que qi la date est postérieur au dernier acte déjà facturé. Si tel est le cas, les actes postérieurs associés au périodique sont supprimés, les exceptions postérieures sont canceled et leur acte associé supprimé (s'il existe).
La modification de la date de début d'un périodique n'est possible que si la date est antérieur au premier acte déjà facturé. Si tel est le cas, les actes antérieurs associés au périodique sont supprimés, les exceptions antérieures sont canceled et leur acte associé supprimé (s'il existe).
Lors de la modification d'un autre champs du périodique, les actes associés au périodique sont modifiés, les exceptions ne sont pas modifiés (donc leur acte associé non plus).

Les occurrences des périodiques ne peuvent être 'supprimés' s'ils ont un acte d'associé déja facturé.
Les occurrences des périodiques ne peuvent être modifiés s'ils ont un acte d'associé facturé, sauf le champs description.
La modification d'une occurrence crée une exception.

Les rdv patient simples ne peuvent être 'supprimés' s'ils ont un acte d'associé déja facturé.
Les rdv patient simples ne peuvent être modifiés s'ils ont un acte d'associé facturé, sauf le champs description.

Les périodiques et rdv simples qui ne sont pas des rdv patients ne sont pas soumis à ces contraintes.


Files

agenda.patch (19.1 KB) agenda.patch Mikaël Ates, 30 July 2014 05:31 PM

History

#1

Updated by Mikaël Ates almost 10 years ago

  • Description updated (diff)
  • Assignee set to Mikaël Ates
  • Target version set to 1.1.3
#2

Updated by Mikaël Ates almost 10 years ago

#3

Updated by Benjamin Dauvergne almost 10 years ago

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.

#4

Updated by Mikaël Ates almost 10 years ago

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 ?
#5

Updated by Benjamin Dauvergne almost 10 years ago

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.

#6

Updated by Serghei Mihai almost 10 years ago

  • Status changed from Nouveau to Résolu (à déployer)
#7

Updated by Mikaël Ates almost 10 years ago

  • Status changed from Résolu (à déployer) to En cours
  • Target version changed from 1.1.3 to 1.3.1 Améliorations agenda

Je remet la demande in progress et la déplace vers la prochaine version pour prendre en compte les améliorations proposées par Benjamin.

#8

Updated by Benjamin Dauvergne almost 10 years ago

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.

#9

Updated by Frédéric Péters about 9 years ago

  • Patch proposed changed from Yes to No
#10

Updated by Mikaël Ates over 8 years ago

  • Assignee deleted (Mikaël Ates)
  • Target version changed from 1.3.1 Améliorations agenda to 2.0

Also available in: Atom PDF