Solution pour modifier un module existant ?
#1
Bonjour,

Je cherche à rajouter des numéros d'opportunités indépendant de ceux de creme_entity, ce dernier semble regrouper également tout ce qui est contacts, etc... Du coup j'ai créer un champ et une colonne dans la table opportunities_opportunity. J'ai donc du toucher directement au code du module opportunities (modèle, vue). Donc je voulais savoir si Crème propose un meilleur moyen d'ajouter de telles fonctionnalités ?

Pour que vous puissiez mieux voir de quoi je parle, vous pouvez voir les modifications que j'ai apporté : https://github.com/Saggah/Creme_CRM-1.4
Peut-on voir pour intégrer officiellement une fonctionnalité de ce genre dans le module opportunities ?

Par contre, j'ai un soucis pour créer les fichiers de migrations (j'en ai besoin maintenant que j'ai ajouté une colonne dans une table de base de donnée). J'ai mis à jour la version de South (maintenant c'est la 0.8.4) comme il semble conseillé sur ce post et j'ai le même message d'erreur :

Code :
$ python2.7 manage.py schemamigration opportunities --auto
/usr/lib/python2.7/pkgutil.py:186: ImportWarning: Not importing directory '/usr/local/lib/python2.7/dist-packages/virtualenvwrapper': missing __init__.py
  file, filename, etc = imp.find_module(subname, path)
Traceback (most recent call last):
  File "manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/usr/local/lib/python2.7/dist-packages/django/core/management/__init__.py", line 443, in execute_from_command_line
    utility.execute()
  File "/usr/local/lib/python2.7/dist-packages/django/core/management/__init__.py", line 382, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/usr/local/lib/python2.7/dist-packages/django/core/management/base.py", line 196, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/usr/local/lib/python2.7/dist-packages/django/core/management/base.py", line 232, in execute
    output = self.handle(*args, **options)
  File "/home/makina/Documents/kseroux/South-0.8.4/south/management/commands/schemamigration.py", line 105, in handle
    (k, v) for k, v in freezer.freeze_apps([migrations.app_label()]).items()
  File "/home/makina/Documents/kseroux/South-0.8.4/south/creator/freezer.py", line 37, in freeze_apps
    model_defs[model_key(model)] = prep_for_freeze(model)
  File "/home/makina/Documents/kseroux/South-0.8.4/south/creator/freezer.py", line 73, in prep_for_freeze
    fields = modelsinspector.get_model_fields(model, m2m=True)
  File "/home/makina/Documents/kseroux/South-0.8.4/south/modelsinspector.py", line 407, in get_model_fields
    field_defs[field.name] = field.south_field_triple()
  File "/home/makina/Documents/kseroux/creme_crm-1.4/creme/creme_core/models/fields.py", line 80, in south_field_triple
    args, kwargs = introspector(self)
  File "/home/makina/Documents/kseroux/South-0.8.4/south/modelsinspector.py", line 373, in introspector
    kwargs[kwd] = get_value(field, defn)
  File "/home/makina/Documents/kseroux/South-0.8.4/south/modelsinspector.py", line 292, in get_value
    return value_clean(value, options)
  File "/home/makina/Documents/kseroux/South-0.8.4/south/modelsinspector.py", line 350, in value_clean
    value = options['converter'](value)
  File "/home/makina/Documents/kseroux/South-0.8.4/south/modelsinspector.py", line 50, in convert_on_delete_handler
    raise ValueError("South does not support on_delete with SET(function) as values.")
ValueError: South does not support on_delete with SET(function) as values.

Merci d'avance et bon week-end...
  Répondre
#2
Bonsoir,

Pour l'erreur de South, je vous conseille de faire comme dit dans le post, modifier le SET par un protect (ou le mettre en commentaire le temps de la migration).

Celui qui vous cause souci est à priori celui du fichier creme_core/models/fields.py a la ligne 70

Concernant la modification que vous avez apporté.

Plusieurs points :
- vous pouvez ajouter des champs en clic avec les champs personnalisés, effectivement ici vous perdriez votre métier qui consiste à calculer le prochain numéro.
- si vous voulez pouvoir mettre à jour Crème facilement, le mieux est de ne pas toucher directement aux fichiers "Crème". Pour ajouter des champs, il vaut mieux utiliser le mécanisme des contribute_to_model. Vous trouverez un exemple dans le fichier creme_core/models/auth.py, ligne 514 nous ajoutons au modéle User de django les champs que nous définissons dans la classe UserProfile du même fichier (ligne 310). Ensuite pour l'incrémentation automatique vous pouvez utiliser les signaux django par exemple.

Concernant l'intégration d'une telle fonctionnalité. Nous avons des numéros de devis, facture, bon de commande, avec des numéro qui s'incrémente. Pour les opportunités, nous avons décidé pour l'instant de ne pas le faire parce que cela nous semblait pas suffisamment "générique". Et nous ne voulions pas ajouter une fonctionnalité qui ne serait pas utiliser par une grande majorité des utilisateurs. Nous avons mis à la place le champ référence, libre par défaut et qui permet d'avoir quelque chose de plus "souple" qu'un numéro. (par exemple en commençant par un code qui correspondrait à un business unit ou à des familles catalogues ou à des secteurs géographique ou de volume, etc.).
  Répondre
#3
Quelques petites précisions au post de mon collègue.

Nous sommes évidemment ravis à l'idée de recevoir des contributions externes ; n'hésitez donc pas à venir en parler comme vous l'avez fait.

Comme dit plus haut, la façon la plus propre de faire vos modifications spécifiques est de le faire depuis votre propre app que vous aurez créée. Pour cela vous disposez d'un certain nombre d'outils :
  • La modification de modèle, lorsqu'elle ne peut pas se faire via les CustomFields, peut se faire via contribute_to_model() comme expliqué ici.
  • Les formulaires Creme peuvent être hookés (voir les méthodes add_post_clean_callback(), add_post_init_callback() et add_post_save_callback()).
  • Django propose un système de signaux qui sont appelés quand une instance de modèle est construite/sauvée/supprimée par exemple.
  • Django permet de surcharger les templates existants.
  • Vous pouvez créer vos propres types de bloc et les placer sur la detailview des modèles existants.
  • Python permet lui-même de faire des choses intéressantes comme ajouter des méthodes à des classes existantes, voire faire du monkey patching (même si c'est à éviter si possible).
  • ...

À côté de ça, je ne peux que vous encourager à écrire des tests unitaires pour votre code (inspirez vous des tests présents dans les apps Creme) ; ils vous seront particulièrement utiles lorsque vous mettrez à jour la version de Creme sur laquelle vous vous basez.

Et si vous êtes seul à travailler sur votre code, je vous encourage aussi à utilisez une queue de patch mercurial (extension 'mq') ; si vous êtes plusieurs leur utilisation est plus délicate et les avis sont partagés dans l'équipe Smile. Cela vous permettra de facilement garder votre code séparé du code Creme trunk (quand bien même les méthodes indiquées plus haut n'auraient pas suffit et que nous ayez modifié le code trunk directement), et ce même lorsque vous mettrez à jour la version de Creme.

À bientôt j'espère.
  Répondre
#4
Tout d'abord, merci pour vos réponses rapides et super détaillé !


Donc j'ai désactivé South pour l'instant. J'avais déjà essayé de remplacer le SET par un PROTECT (sans comprendre ce que je faisais) sans succès. Après j'avais pas essayé protect (en minuscule) ni en commentant la ligne dont il est question. Je verrai ça plus tard.

Je comprend mieux l'intérêt du champ référence maintenant. Mais une des raisons de la volonté de passer de SugarCRM à Crème CRM était l'absence de l'auto-incrémentation des numéros d'opportunités avec Sugar. Mais aussi parce qu'il est de plus en plus propriétaire.

Et merci, j'ai découvert l'existence de ces champs personnalisés, hooks et signaux pour Django. Quant à l'auto-incrémentation, j'ai fais comme vous m'avez conseillé, c'est-à-dire d'utiliser la fonction contribute_to_model. J'ai utilisé les hooks (add_post_init_callback).
Puis j'arrive presque au même résultat qu'avant mais de façon modulaire donc c'est super ! Voici du coup le code source du module. La seule chose qui manque est le positionnement du nouveau champ en-dessous du nom de l'opportunité. Car il est affiché plutôt en dernier. Ça doit pas être trop compliqué à mettre en oeuvre je pense.

J'ai des questions, sur le lien que tu m'as donné @genglert à propos de contribute_to_model(), tu parles de la disparition de cette fonction lorsque Crème utilisera la version 1.5 de Django, existera-t-il une fonction équivalente ? S'agit-il de ce monkey patching que tu as cité ? Aussi, existe-t-il un moyen de supprimer le champ référence de la classe model pour pas qu'il soit généré dans la table de la bdd ni affiché ? En fait je débute que depuis une semaine avec Python et Django donc pour ça que j'ai plein de questions qui peuvent porter sur Python et Django.

J'ai pour (mauvaise ?) habitude de faire les tests directement dans la fonction qui a besoin d'être testé. En effet tu as raison, il faudrait que je fasse les tests unitaires séparément. C'est pas comme si Django ne proposait rien de ce côté. Smile

Également, je ne connais pas Mercurial, je ne connais que Git et SVN mais merci quand même du conseil pour "la queue de patch mercurial".


Bonne soirée !


EDIT : Pour votre information, j'ai tenté de récréer les fichiers de migrations comme il est bien expliqué ici avec la commande python2.7 manage.py schemamigration opportunity_number --initial. J'ai bien un fichier de migration qui a été créer. D'ailleurs ça a marché tandis que je n'ai que recloner le dépôt de Crème 1.4. Mais ce fichier ne contient pas plus que :

Code :
# -*- coding: utf-8 -*-
from south.utils import datetime_utils as datetime
from south.db import db
from south.v2 import SchemaMigration
from django.db import models


class Migration(SchemaMigration):

    def forwards(self, orm):
        pass

    def backwards(self, orm):
        pass

    models = {
        
    }

    complete_apps = ['opportunity_number']

J'en ai donc déduis donc que les migrations sont incompatible avec contribute_to_model().

Je pense qu'il serait bien d'avoir une interface permettant de choisir quels champs (même natifs à Crème) afficher, déplacer. Et ajouter des propriétés aux champs personnalisés comme l'auto-incrémentation dans le cas d'un IntegerField.
  Répondre
#5
Pour vos problèmes avec south, mon collègue vous a déjà donné la solution dans son premier commentaire, à savoir mettre la ligne 'kwargs['on_delete'] = ...' en commentaire (plus simple qu'utiliser PROTECT qui nécessite plus de travail).

Explication: django gère les suppressions des objets référencés par ForeignKey suivant 4 méthodes : CASCADE, PROTECT, SET_NULL et SET. Les 3 premières sont triviales à comprendre, la dernière prend une fonction qui permet de coder le comportement à utiliser lors de la suppression. Malheureusement south ne gère pas ce dernier cas, et refuse de continuer, comme vous l'avez vu. Or south n'évolue plus en ce moment, car son auteur est en train de l"intégrer directement dans django, et south va devenir à terme la façon standard de gérer les tables (la commande syncdb va plus ou moins disparaître). À terme le problème de SET devrait disparaître (ainsi que des tas de bugs de south venant de sa non intégration), mais pour le moment on n'a pas le choix que de contourner les problèmes. D'où le hack qui consiste à commenter la ligne susnommée, qui fait que south voit cette FK comme étant en mode CASCADE, la valeur par défaut.

Pour le contribute_to_model():
  • Pas de problème la fonction ne va pas disparaître ; django 1.5 permet de définir son propre modèle User, alors que pour le moment nous utilisons contribute_to_model() pour modifier le modèle User de django. Donc avec django 1.5, on pourra se passer d'utiliser contribute_to_model() sur User, mais cela ne veut pas dire que cette dernière va être supprimée. Nous nous en servons sans arrêt pour écrire le code spécifique de nos clients, donc pas de souci.
  • Comme dit au dessus nous nous servons tout le temps de contribute_to_model(), donc nous serions au courant si elle ne fonctionnait pas avec south. En revanche vous vous y prenez mal : vous générez votre migration pour votre propre app (comme l'indique la dernière ligne), alors qu'il faut la générer pour 'opportunities'. En effet votre app n'a pas de modèle propre et se content de modifier ceux de 'opportunities', d'où la migration vide.

Pour gérer l'ordre d'affichage des champs d'une Opportunity, vous avez 2 façons propres de faire:
  • En modifiant l'attribut 'template_name' de OpportunityBlock (dans creme/opportunities/blocks.py) pour mettre votre propre template (vous pouvez vous inspirez de creme/persons/templates/persons/templatetags/block_contact.html par exemple). Cela doit être faisable depuis le creme_core_register.py de votre app.
  • En utilisant la fonctionnalité des blocs configurables décrite ici.

Sinon pour l'idée de l'auto-incrémentation, c 'est une idée intéressante, nous le ferons si ça devient une demande récurrente (chez nos clients par exemple). Il y aurait sûrement bien d'autres amélioration à apporter aux CustomFields ; j'avais par exemple pensé à pouvoir les rendre obligatoires, ou mettre une valeur par défaut, quand je les ai fait il y a déjà quelques années, mais au final cela n'a pas eu l'air de manquer spécialement. Il faut bien voir que les champs supplémentaires s'accompagnent souvent de règles métier, et nécessitent donc du code de toute les façons (donc pas de problème pour ajouter ces champs avec contribute_to_model() ).
  Répondre
#6
En fait pour South je pensais que j'avais exécuté la même commande que lors de mon premier post. J'ai vu que ça a marché (en comprenant pas pourquoi d'ailleurs) donc j'ai supposé que c'était pour une autre raison. Je viens du C++ donc les fonctions comme contribute_to_model() qui permettent de greffer comme ça des fonctionnalités, c'est perturbant. Mais bon tellement pratique qu'il faut avouer. Smile

Du coup, j'ai réussis à créer des fichiers de migrations. Pour l'affichage des champs j'ai utilisé la fonctionnalité des champs configurables. Tout semble fonctionner niquel. Bref, je pense que j'ai presque terminé mon module. Merci beaucoup pour votre aide inestimable !
  Répondre
#7
Je suis content que vous vous en soyez sorti si vite. Je suis bien conscient que les callbacks sur les forms par exemple ne sont pas spécialement bien documentés, et même si nous faisons en sorte que notre code soit le plus clair possible, et qu'il fait souvent office de documentation, cela mériterait sûrement quelque chose de plus formel. Mais visiblement vous avez très bien compris comment ils marchent.

J'aurai quelques petites remarques sur votre code :
  • Vos imports dans votre models/opportunity.py sont un peu en vrac (c'est une broutille mais connaître la façon correcte est toujours mieux).
  • Le fait que tout soit géré dans les formulaires est un peu fragile je pense. Si une autre app décide de créer une Opportunity, elle plantera sur une erreur d'intégrité (votre number sera NULL). Après avoir regardé vite fait, en l'état je pense que seul des test unitaires vont planter, et donc que cela ne soit pas gênant en pratique, en revanche c'est une bonne chose d'en être conscient là encore.
  • Il est assez tordu d'avoir mis les hooks dans creme_config_register.py plutôt que creme_core_register.py. Dans les faits cela fonctionne un peu par hasard, parce que les formulaires d'Opportunity ont des widgets spéciaux (pour les phases de vente par exemple) qui vont forcer la registry de creme_config à se remplir, et donc appeler votre creme_config_register.py. Mais c'est bizarre et fragile comme façon de faire, alors que vous êtes sûr que creme_core_register.py est forcément appelé, et est donc classiquement utilisé pour ce genre ce chose.
  Répondre
#8
J'ai juste eu besoin de savoir quand elles étaient appelés les fonctions callbacks. Ce qui était expliqué sur la doc. Par contre il vaudrait mieux la faire en anglais, non ?

J'ai réorganisés les imports et j'ai mis la déclaration des callbacks dans le creme_core_register. En fait j'avais cru lire que tu m'avais conseillé creme_config_register. Je me suis mélangé avec la documentation. Donc oui c'est uniquement par chance que ça fonctionne. ^^

je crois avoir compris le problème dans le cas où une classe modèle Opportunity est instancié. Mais bon de toute façon je n'autorise pas les valeurs nulles. Mais après, c'est vrai que c'est mieux si la fonctionnalité d'auto-incrément est également présente sans créer de formulaire. Donc du coup je vais utiliser les signaux pour l'auto-incrémentation.

Je vais faire ça aujourd'hui.

EDIT : En fait quand j'utilise le le signal post_init() sur la classe modèle Opportunity, le numéro est bien mis à jour quand il y a création d'une opportunité mais aussi quand je veux afficher la vue de la liste des opportunités. Du coup, toutes les opportunités affichent (car ce n'est pas le cas dans la bdd) le dernier numéro présent dans la table + 1. Donc le signal est émit même quand il affiche la liste. J'ai essayé de connecter le signal et un slot dans la fonction callback (add_post_init_callback). Je pensais aussi déconnecter le signal et le slot à chaque fois que le traitement à été fait (autoincrémentation). Mais la fonction callback utilisé n'est exécuté qu'après que la classe modèle associé au formulaire est instancié. Ça aurait été possible avec une fonction callback exécuté avant genre add_pre_init_callback() mais on perd l'intérêt d'avoir voulu délégué l’auto-incrémentation à la classe modèle...

EDIT2 : Je travaille là sur un nouveau module de recherche par tag et j'ai comme l'impression que contribute_to_model ne marche pas sur les classes abstraites. La recherche par tag nécessite que j'ajoute ce champ à toutes les entités de crème. Donc j'ai vu que les champs en communs sont déclarés dans CremeAbstractEntity (classe parente de CremeEntity) dans creme_core/models/base. Quand je contribue à CremeEntity, la création des fichiers de migration et la migration marchent (pas le reste mais bon c'est normal). Mais quand je contribue à CremeAbstractEntity, là où je devrais normalement, j'ai le droit à de la part de South "nothing seems to have changed". Après le problème vient peut-être de South, je vais le désactiver pour voir ce que ça donne...

EDIT3: Après désactivation, le champ n'est même pas crée ni sa colonne dans la table creme_core_cremeentity. Sinon toujours avec South désactivé, la colonne se crée uniquement quand je contribue à CremeEntity.

La seule solution qui à l'air de bien fonctionné c'est de coder directement dans le module creme_core...

Également, je viens de me rendre compte qu'aujourd'hui qu'il existe un paramètre facultatif pour supprimer certains champs avec contribute_to_model(). J'aurais pu l'utiliser pour le champ "reference" pour le module ajoutant les numéros d'opportunités.

Oui, je détaille tous ce que je fais, mon feedback ne pourra que vous être utile pour que vous soyez conscient de ce qui a besoin d'être amélioré même si l'erreur vient de moi. Smile
  Répondre
#9
Pour la documentation externe, ça serait effectivement mieux qu'elle existe aussi en anglais. C'est juste un manque de ressource si ce n'est pas le cas. Creme ne se veut pas franco-français, mais il faudrait un petit peu de travail pour qu'il soit international de base. C'est plus un problème de configuration qu'autre chose (et tout est faisable au pire par l'IHM), mais par exemple on met de base les taux de TVA français, pareil pour les statuts des entreprises (SARL etc...). Donc comme on ne fait pas encore l'effort de le faire connaître en dehors de la France, ce n'est pas le plus urgent.

contribute_to_model() n'est effectivement pas compatible avec les classes abstraites à cause de l'implémentation actuelle. Quand une classe abstraite est utilisée, django copie les champs dans la classe fille concrète, et l'information de 'dérivation' de classe est perdue (du coup ce n'est plus vraiment une dérivation au sens python) ; et quand contribute_to_model() fait son boulot c'est trop tard, la copie ayant déjà été faite. C'est un problème connu mais que je n'avais pas documenté ; je le ferai pour la version 1.4.2.

Quant au signal, je pensais au signal 'pre_save', qui mettrai une valeur à votre nombre entier dans le cas d'une création où la valeur du nombre n'aurait pas été précisée. Ça ne se substitue pas au travail que vous avez fait dans les formulaire (mais vous pouvez factoriser la fonction de génération du nombre par exemple), en revanche ça permet au code de ne pas planter lorsqu'une Opportunité n'est pas créée par vos formulaire. C'est pourquoi je disais que ce cas de figure ne vous générai peut-être pas en pratique, mais que le code était fragile dans l'absolu.
  Répondre
#10
J'ai opté pour le signal pre_save() et j'ai même abandonné le post_init(). Comme ça 2 opportunités peuvent être créent en même temps (le numéro n'est pas chargé dans le formulaire dès le début).

A propos de contribute_to_model(), il me reste quel choix que de coder directement dans creme_core ? Faire une contribution à toutes les classes qui dérivent de CremeEntity (car ce n'est même pas applicable à CremeEntity -> problème avec creme_populate, on dirait qu'il veut remplir ce nouveau champ qui n'existe pas).
  Répondre


Atteindre :


Utilisateur(s) parcourant ce sujet : 1 visiteur(s)