Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better Integration with django-mptt #246

Open
barseghyanartur opened this issue Jun 4, 2015 · 4 comments
Open

Better Integration with django-mptt #246

barseghyanartur opened this issue Jun 4, 2015 · 4 comments

Comments

@barseghyanartur
Copy link

I have read this (http://django-hvad.readthedocs.org/en/latest/public/faq.html#how-do-i-use-hvad-with-mptt).

I have models (see below). Admin works, ORM works.

from django.db import models
from django.utils.translation import ugettext_lazy as _

from mptt.models import MPTTModel, TreeForeignKey
from mptt.managers import TreeManager

from hvad.models import TranslatableModel, TranslatedFields
from hvad.manager import TranslationManager

class AbstractFacility(TranslatableModel):
    translations = TranslatedFields(
        name=models.CharField(_("Name"), max_length=255),
        slug=models.SlugField(_("Slug"), max_length=255),
     )

    class Meta:
        abstract = True

    def __unicode__(self):
        return self.safe_translation_getter('name', unicode(self.name))


class MPTTFacilityManager(TranslationManager, TreeManager):
    use_for_related_fields = True


class AbstractMPTTFacility(MPTTModel, AbstractFacility):
    parent = TreeForeignKey('self', null=True, blank=True,
                            related_name='children', db_index=True)

    objects = MPTTFacilityManager()

    class Meta:
        abstract = True


class MyModel(AbstractMPTTFacility):
    translations = TranslatedFields()

    class Meta:
        verbose_name = _('My model')
        verbose_name_plural = _('My models')

I want now to use the those models in forms and make use of the django-mptt fields.

https://django-mptt.github.io/django-mptt/forms.html#fields

For instance, the TreeNodeMultipleChoiceField.

from django import forms
from django.utils.translation import ugettext_lazy as _
from mptt.forms import TreeNodeMultipleChoiceField

class MyForm(forms.Form):
    def __init__(self, *args, **kwargs):
        super(MyForm, self).__init__(*args, **kwargs)
        queryset = MyModel.objects.language('en')
        self.fields['my_models'] = TreeNodeMultipleChoiceField(queryset, label=_("My models"))

When feeding the queryset to the TreeNodeMultipleChoiceField, I get errors.

Applying this hack helps:

queryset.model._mptt_meta = MyModel._mptt_meta

I'm not sure if django-hvad is supposed to go that far, but perhaps some copying some essential properties of the original model into translated model could be automatised or the workarounds documented.

@spectras
Copy link
Collaborator

spectras commented Jun 4, 2015

Hello,

Well, it seems unwise to blindly copy unknown third-party module's properties into another model. It might work, it might break other modules, or it might even corrupt the database.

You can do it on a case-by-case basis, just like you did (I can tell you that hvad will not have any problem with what you did here), but systematizing the process would be too dangerous.

Interaction between queryset-redefining apps is not clearly defined by Django, which makes it a bit random. It happens to work pretty well with MPTT because what they change in the queryset happens to be mostly compatible with what hvad does with it (with some edge cases like you just found).

I guess this specific case could deserve a compatibility package, or maybe creating some hvad.contrib.mptt submodule, so it can be well defined, well behaved and well tested. There are quite some opportunities for optimization as well, if someone is willing to dig into it.

@barseghyanartur
Copy link
Author

@spectras:

You're absolutely right, but I wasn't proposing (wouldn't want) more than improved/better django-mtpp integration.

@spectras
Copy link
Collaborator

spectras commented Jun 5, 2015

I'd surely love that too.
I fear it would overextend the scope of the core hvad package too much and I would not have the time to keep up with both Django and MPTT updates, though. Until I have more time, or I find someone willing to maintain such an integration module and maybe coordinate a joint effort with MPTT developers, I'd rather keep it focused.

If you (or any people stumbling upon this) happen to be interested, here is what would be required for such a feature to make it into hvad:

  • the actual compatibility module, probably hosted in hvad.contrib.mptt
  • documentation.
  • thorough testing of all features, including making sure unsupported ones raise an appropriate NotImplementedError.
  • ongoing support, such as keeping track of changes in MPTT and making sure hvad does not lag behind when new releases come out.

@spectras spectras changed the title Integration with django-mptt Better Integration with django-mptt Dec 4, 2015
@spectras
Copy link
Collaborator

Note: there will be some time until hvad2 is released, but compatibility with third-party modules should be improved a lot, and make this kind of issues moot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants