wagtail / django-modelcluster Goto Github PK

View Code? Open in Web Editor NEW

473.0 23.0 65.0 421 KB

Django extension to allow working with 'clusters' of models as a single unit, independently of the database

License: BSD 3-Clause "New" or "Revised" License

Python 100.00%

django-modelcluster's People

Stargazers

Watchers

Forkers

alexsnet bufke artursmet wethegit vlprans neon-jungle gasman theju thenewguy helenwarren nickdtodd simudream alilozano mikedingjan-forks dgo-list zerolab founders4schools ashiazed adamchainz lucasmoeskops hjwp michael-borisov benoss wnp orus-io smendes haydngreatnews alee skirsdeda joshyjosh therefromhere sam-costigan frmdstryr gassan mattijsstam ababic ompom orientalperil chosak marteinn juyrjola osmanhankir ferthus tomkins santosh653 guma44 mafermazu backbayrider marmik2003 suprasummus wimfeijen hpoul engineervix jakubmusil jsma th3hamm0r aaronhaslett arpitjain799 laymonage irtazaakram theonlykingpin qge traderknows kira009

django-modelcluster's Issues

unique_together is not working in ClusterableModel

Hi,

is it possible to use the unique_together constraint somehow in a ClusterableModel?

Thank You!

Specify django-taggit as an optional dependency via extras_require

The new 3.0 version requires django-taggit>=0.20 but it's not specified in the install requires section of setup.py.

Since my project doesn't depend directly on taggit, upgrading to 3.0 is breaking my build. Having this requirement in the setup.py would have upgraded the downstream dependency automatically.

Was this a conscious decision or was it forgotten?

Prefetch related not working for parental many to many relations

👋

I want to apologize in advance, if I'm missing something obvious, but having the following example:

from django.db import models

from modelcluster.models import ClusterableModel
from modelcluster.fields import ParentalManyToManyField

class Movie(ClusterableModel):
   name = models.CharField(max_length=250)
   actors = ParentalManyToManyField('Actors', related_name='movies')


class Actors(models.Model):
   name = models.CharField(max_length=250)

When I do

Movie.objects.prefetch_related('actors')

I get the following error:

ValueError: 'actors' does not resolve to an item that supports prefetching - this is an invalid parameter to prefetch_related().

The question is - how am I supposed to do the prefetch_related on ParentalManyToManyField?

The context is Wagtail, but I'm posting this here, since the issues seems to be from the clusterable model.

Thanks!

`FakeQuerySet.values_list` returns list of objects instead of list of pks

If model have relations, FakeQuerySet.values_list returns list of related objects instead of list of primary keys.

See Django's QuerySet.values_list documentation.

problem with manage.py loaddata

from an app i'm working on

Traceback (most recent call last):
  File "manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File ".../lib/python3.6/site-packages/django/core/management/__init__.py", line 364, in execute_from_command_line
    utility.execute()
  File ".../lib/python3.6/site-packages/django/core/management/__init__.py", line 356, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File ".../lib/python3.6/site-packages/django/core/management/base.py", line 283, in run_from_argv
    self.execute(*args, **cmd_options)
  File ".../lib/python3.6/site-packages/django/core/management/base.py", line 330, in execute
    output = self.handle(*args, **options)
  File ".../lib/python3.6/site-packages/django/core/management/commands/loaddata.py", line 69, in handle
    self.loaddata(fixture_labels)
  File ".../lib/python3.6/site-packages/django/core/management/commands/loaddata.py", line 109, in loaddata
    self.load_label(fixture_label)
  File ".../lib/python3.6/site-packages/django/core/management/commands/loaddata.py", line 175, in load_label
    obj.save(using=self.using)
  File ".../lib/python3.6/site-packages/django/core/serializers/base.py", line 208, in save
    getattr(self.object, accessor_name).set(object_list)
  File ".../lib/python3.6/site-packages/modelcluster/fields.py", line 406, in set
    sort_by_fields(objs, rel_model._meta.ordering)
  File ".../lib/python3.6/site-packages/modelcluster/utils.py", line 19, in sort_by_fields
    items.sort(key=lambda x: (getattr(x, key, None) is not None, getattr(x, key)), reverse=reverse)
  File ".../lib/python3.6/site-packages/modelcluster/utils.py", line 19, in <lambda>
    items.sort(key=lambda x: (getattr(x, key) is not None, getattr(x, key)), reverse=reverse)
AttributeError: Problem installing fixture '.../prod.dump.json': 'int' object has no attribute 'title'

will try and do a minimal repro if i get a spare mo.

Clarification about on_delete in Django 2.0

Will ParentalKey require the on_delete argument to be set when Django 2.0 support is implemented? Reflecting the requirement for ForeignKey?

It might be good to clarify the default in the readme if it is not required and will continue to default to models.CASCADE. Also, this might be a factor to consider for Django 2.0 support in general.

Context

Django 2.0 will require on_delete to be set for ForeignKey.

A many-to-one relationship. Requires two positional arguments: the class to which the model is related and the on_delete option.
https://docs.djangoproject.com/en/2.0/ref/models/fields/#foreignkey

Django 1.11 did not require it, and set it to the default models.CASCADE
https://docs.djangoproject.com/en/1.11/ref/models/fields/#foreignkey

Related issues

Django 2.0 support - #85

Wagtail query about on_delete not in docs, but throwing error when not provided to ParentalKey - wagtail/wagtail#4126

FakeQuerySet doesn't support order_by('?')

The FakeQuerySet object does not appear to have support for randomizing the order using order_by('?'). It treats ? as the name of a field on the model.

Implement QuerySet.distinct in FakeQuerySet

FakeQuerySet doesn't fully implement the API of Django's QuerySet which can lead to confusing bugs (wagtail/wagtail#1305)

There are a few other missing features such as values and values_list

See https://docs.djangoproject.com/en/1.8/ref/models/querysets/ for a full list

inherit Parentalkey + prefecth_related bug

the following case does not return the results you would expect :

class Parent(ClusterableModel):
    pass

class OneChild(Parent):
    parent = models.OneToOneField(to=Parent, parent_link=True,on_delete=models.CASCADE)

class ManyChild(models.Model):
    parent = ParentalKey('cms.Parent', null=True, related_name='manyChild')
    data = models.CharField(max_length=50)
    data2 = models.CharField(max_length=50)

def populate():
    Parent.objects.all().delete()
    OneChild.objects.all().delete()
    ManyChild.objects.all().delete()
    p1 = Parent.objects.create()
    p2 = Parent.objects.create()
    c1 = OneChild.objects.create(parent=p1)
    c2 = OneChild.objects.create(parent=p2)
    m1 = ManyChild.objects.create(parent=c1,data='1',data2='a')
    m1 = ManyChild.objects.create(parent=c1, data='2', data2='b')
    m1 = ManyChild.objects.create(parent=c2, data='1', data2='c')
    m1 = ManyChild.objects.create(parent=c2, data='2', data2='d')

def testresult():
    queryset = OneChild.objects \
        .prefetch_related(
        Prefetch(
            "manyChild",
            queryset=ManyChild.objects.filter(data='1')
        )
    )
  
  return [queryset.all()[0].manyChild.values(),queryset.all()[1].manyChild.values()]

populate()

print(testresult())

expected 1 result per queryset, gets all results that mach the filter. relation seems to be lost

in this case you would expect the result to be :

[<QuerySet [{'id': 1, 'parent_id': 10, 'data': '1', 'data2': 'a'}]>,<QuerySet [{'id': 3, 'parent_id': 11, 'data': '1', 'data2': 'c'}]>]

But it returns :

[<QuerySet [{'id': 1, 'parent_id': 10, 'data': '1', 'data2': 'a'},{'id': 3, 'parent_id': 11, 'data': '1', 'data2': 'c'}]>,<QuerySet [{'id': 1, 'parent_id': 10, 'data': '1', 'data2': 'a'},{'id': 3, 'parent_id': 11, 'data': '1', 'data2': 'c'}]>]

This issue doesnt appear if you use models.ForgienKey('cms.Parent', null=True, on_delete=models.CASCADE, related_name='manyChild') instead of ParentalKey('cms.Parent', null=True, related_name='manyChild')

using this lib in combination with wagtail. but this seems to be unrelated to them

Using
python 3.6
Django 2.1.2

Drop support for django 1.6?

Please update tox.ini to remove support for django 1.6 now that it is unsupported. You may also consider removing Python 2.6 which isn't available on any Linux based distribution. RHEL 7, which is the slowest in terms of updates, also comes with Python 2.7.

Implement .values and .values_list in FakeQuerySet

This one seems to catch us out a lot.

Searching for limit_choices_to for ParentalManyToManyField

For a plain Django many-to-many field exists an argument limit_choices_to[1].

Is it possible to have an argument like that for a modelcluster ParentalManyToManyField?

Background is a wagtail project, were I would like to limit the choices for a ParentalManyToManyField to a specific value of the related model field (pseudocode):

# models.py
MyPage(Page):
    authors = ParentalManyToManyField(
        'profile.ProfilePage',
        blank=True,
        limit_choices_to={'is_author': True},
)

[1] https://docs.djangoproject.com/en/2.0/ref/models/fields/#django.db.models.ManyToManyField.limit_choices_to

Doesn't work with swappable models

I'm trying to create swappable models for wagtail-blog and seeing that makemigrations breaks because django's migration app cache isn't able to produce a correct lazy reference for ParentalKey.

Traceback:

Traceback (most recent call last):
  File "manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "site-packages/django/core/management/__init__.py", line 367, in execute_from_command_line
    utility.execute()
  File "site-packages/django/core/management/__init__.py", line 359, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "site-packages/django/core/management/base.py", line 294, in run_from_argv
    self.execute(*args, **cmd_options)
  File "site-packages/django/core/management/base.py", line 345, in execute
    output = self.handle(*args, **options)
  File "site-packages/django/core/management/commands/makemigrations.py", line 176, in handle
    migration_name=self.migration_name,
  File "site-packages/django/db/migrations/autodetector.py", line 47, in changes
    changes = self._detect_changes(convert_apps, graph)
  File "site-packages/django/db/migrations/autodetector.py", line 132, in _detect_changes
    self.old_apps = self.from_state.concrete_apps
  File "site-packages/django/db/migrations/state.py", line 180, in concrete_apps
    self.apps = StateApps(self.real_apps, self.models, ignore_swappable=True)
  File "site-packages/django/db/migrations/state.py", line 249, in __init__
    raise ValueError("\n".join(error.msg for error in errors))
ValueError: <function resolve_related_class at 0x7f4d8539b578> contains a lazy reference to pages.blogpagetag, but app 'pages' doesn't provide model 'blogpagetag'.
<function resolve_through_model at 0x7f4d8539b8c0> contains a lazy reference to pages.blogcategoryblogpage, but app 'pages' doesn't provide model 'blogcategoryblogpage'.
The field blog.BlogCategoryBlogPage.page was declared with a lazy reference to 'pages.blogpage', but app 'pages' doesn't provide model 'blogpage'.
The field blog.BlogPageTag.content_object was declared with a lazy reference to 'pages.blogpage', but app 'pages' doesn't provide model 'blogpage'.

In the traceback, blog.BlogCategoryBlogPage.page and blog.BlogPageTag.content_object are both ParentalKey fields. They both belong to models which have been swapped out.

The question is how to stop model_checks._check_lazy_references from checking these fields.

Commit method in DeferringRelatedManager deletes related records before adding.

Summary

When publishing a Page, child pages connected via a ParentalKey are deleted prior to being added. This results in a Page Not Found error. The issue occurs between lines 196 - 207 in fields.py

     live_items = list(original_manager.get_queryset())
            for item in live_items:
                if item not in final_items:  # <-- Both live_items and final_items point to same page but this returns false
                    item.delete()  # <-- Page deleted

            for item in final_items:
                # Django 1.9+ bulk updates items by default which assumes
                # that they have already been saved to the database.
                # Disable this behaviour.
                # https://code.djangoproject.com/ticket/18556
                # https://github.com/django/django/commit/adc0c4fbac98f9cb975e8fa8220323b2de638b46
                original_manager.add(item, bulk=False)  # <-- Page does not exist here.

Environment

Python 3.6
Django 2.2.4
django-modelcluster 4.4
wagtail 2.6.1

Models

class BulkGood(Page):
    ....

        def save(self, **kwargs):
          # clear cached listing template
          cache_key = make_template_fragment_key('bulks')
          cache.delete(cache_key)
          # Update our title and slug fields based on our name
          self.title = self.name
          self.slug = slugify(self.name)
          return super().save(**kwargs)


class FinishedGood(Page):
    ....
   bulk = ParentalKey(
        'faqs.BulkGood',
        related_name='bulk_products',
        null=True, blank=True,
        on_delete=models.SET_NULL,
    )
    ....

Please let me know if there are any further details required or perhaps a more "proper" way to relate these entities.

AttributeError: 'ModelName' object has no attribute '+'

Hi folks! I find that suppressing a backwards relation to a subclass of ModelCluster appears to cause an issue when saving the parent model. I hope this is the right place to open an issue.

Here's the full backtrace from our app and some related model code: https://gist.github.com/jasonm/a6f514026bc2b97c1b94

In the Wagtail CMS I'm working on, an Event is tagged with EventLanguageTag and EventRegionTag instances and the backwards relation is suppressed by specifying related_name='+':

class EventLanguageTag(TaggedItemBase):
    content_object = ParentalKey('cms.Event', related_name='+')

class EventRegionTag(TaggedItemBase):
    content_object = ParentalKey('cms.Event', related_name='+')

@register_snippet
class Event(ClusterableModel, index.Indexed):
    language_tags = TaggableManager(through=EventLanguageTag, blank=True, related_name="event_language", verbose_name="Languages")
    region_tags = TaggableManager(through=EventRegionTag, blank=True, related_name="event_region", verbose_name="Regions")
    # ...

This causes django-modelcluster to look for relations named +:

# shell_plus session
In [13]: from modelcluster.models import get_all_child_relations

In [14]: get_all_child_relations(Event())
Out[14]: [<ManyToOneRel: cms.eventlanguagetag>, <ManyToOneRel: cms.eventregiontag>]

In [15]: [rel.get_accessor_name() for rel in get_all_child_relations(Event())]
Out[15]: [u'+', u'+']

which it then cannot find to save at https://github.com/torchbox/django-modelcluster/blob/eca7709065a6f82914ba47268e0127dbc8f6fe1a/modelcluster/models.py#L201

This is on Django 1.8.4, wagtail 1.2, django-taggit 0.17.1, django-modelcluster 1.1

ClusterForm: Recurse over nested relationships

This relates to the following issue with wagtail: wagtail/wagtail#1952

When ClusterFormMetaclass.__new__() is called, it iterates over child relations for the given model and creates a Formset class by calling the factory function childformset_factory. The form created by the factory function is a subclass of django's base ModelForm, which means that ClusterFormMetaclass isn't used for those forms. If the factory function were to subclass from ClusterForm instead, then ClusterFormMetaclass.__new__() would be called recursively and Formset classes would be created for nested relationships.

It would then be the responsibility of ClusterForm to recurse over nested relationships on form initialisation and save. This would allow for creating nested models in wagtail using InlinePanels, a feature that I would love to see implemented.

Is this something you would be willing to consider?

AttributeError when ParentalManyToManyField is specified with a through model

Django throws AttributeError when ParentalManyToManyField is specified with a through model.

Demo with Wagtail:

from django.db import models
from wagtail.wagtailcore.models import Page
from modelcluster.fields import ParentalManyToManyField


class FooModel(models.Model):
    name = models.CharField(max_length=256)

    def __str__(self):
        return self.name


class BarToFoo(models.Model):
    foo = models.ForeignKey('appname.FooModel', blank=True, null=True)
    page = models.ForeignKey('BarPage', related_name='+')


class BarPage(Page):
    asset_programs = ParentalManyToManyField(
        to='appname.FooModel', through='appname.BarToFoo'
    )

Traceback:

File "/Users/mikalai/.virtualenvs/venv_name/lib/python3.4/site-packages/django/core/handlers/exception.py" in inner
  42.             response = get_response(request)

File "/Users/mikalai/.virtualenvs/venv_name/lib/python3.4/site-packages/django/core/handlers/base.py" in _get_response
  187.                 response = self.process_exception_by_middleware(e, request)

File "/Users/mikalai/.virtualenvs/venv_name/lib/python3.4/site-packages/django/core/handlers/base.py" in _get_response
  185.                 response = wrapped_callback(request, *callback_args, **callback_kwargs)

File "/Users/mikalai/.virtualenvs/venv_name/lib/python3.4/site-packages/django/views/decorators/cache.py" in _cache_controlled
  43.             response = viewfunc(request, *args, **kw)

File "/Users/mikalai/.virtualenvs/venv_name/lib/python3.4/site-packages/wagtail/wagtailadmin/decorators.py" in decorated_view
  31.             return view_func(request, *args, **kwargs)

File "/Users/mikalai/.virtualenvs/venv_name/lib/python3.4/site-packages/wagtail/wagtailadmin/views/pages.py" in edit
  341.                 revision.publish()

File "/Users/mikalai/.virtualenvs/venv_name/lib/python3.4/site-packages/wagtail/wagtailcore/models.py" in publish
  1539.         page.save()

File "/Users/mikalai/.pyenv/versions/3.4.6/lib/python3.4/contextlib.py" in inner
  30.                 return func(*args, **kwds)

File "/Users/mikalai/.virtualenvs/venv_name/lib/python3.4/site-packages/wagtail/wagtailcore/models.py" in save
  477.         result = super(Page, self).save(*args, **kwargs)

File "/Users/mikalai/.virtualenvs/venv_name/lib/python3.4/site-packages/modelcluster/models.py" in save
  209.             getattr(self, field).commit()

File "/Users/mikalai/.virtualenvs/venv_name/lib/python3.4/site-packages/modelcluster/fields.py" in commit
  442.                 original_manager.add(*items_to_add)

File "/Users/mikalai/.virtualenvs/venv_name/lib/python3.4/site-packages/django/db/models/fields/related_descriptors.py" in add
  876.                     (opts.app_label, opts.object_name)

Exception Type: AttributeError at /admin/pages/133/edit/
Exception Value: Cannot use add() on a ManyToManyField which specifies an intermediary model. Use appname.BarToFoo's Manager instead.

Django==1.10.7
wagtail==1.11.1

ParentalKey relation .get() issue

I'm using modelcluster with wagtail CMS and faced a regress after packages upgrade (wagtail 1.13.1 > 2.0, django-modelcluster 3.1 > 4.1). For example:

from django.db import models

from wagtail.core.models import Page
from wagtail.admin.edit_handlers import InlinePanel

from modelcluster.models import ParentalKey


class Product(Page):

    content_panels = Page.content_panels + [
        InlinePanel('prices')
    ]

    def get_price_by_type(self, price_type):
        return self.prices.get(price_type=price_type)


class PriceType(models.Model):

    name = models.CharField(
        max_length=16,
        unique=True,
    )


class ProductPrice(models.Model):

    class Meta:
        unique_together = (('price_type', 'product'), )

    value = models.DecimalField()

    price_type = models.ForeignKey(
        PriceType,
        on_delete=models.CASCADE,
        related_name='+',
    )

    product = ParentalKey(
        Product,
        on_delete=models.CASCADE,
        related_name='prices',
    )

Calling get_price_by_type with modelcluster 3.1 results in:

SELECT "appname_productprice"."id", "appname_productprice"."value", "appname_productprice"."price_type_id", "appname_productprice"."product_id" FROM "appname_productprice" WHERE ("appname_productprice"."product_id" = 40 AND "appname_productprice"."price_type_id" = 1);

And with modelcluster 4.1:

SELECT "appname_productprice"."id", "appname_productprice"."value", "appname_productprice"."price_type_id", "appname_productprice"."product_id" FROM "appname_productprice" WHERE ("appname_productprice"."price_type_id" = 1 AND "appname_productprice"."price_type_id" = 1);

FakeQuerySets can't delete

I'm wondering about a strange situation I'm in. I'm doing something like this:

   model_name.related_name.all().delete()

And I get:

AttributeError: 'FakeQuerySet' object has no attribute 'delete'

There are related items and they are also not shown by the FakeQuerySet, so there's another problem there. This only happens in web requests, not on the console, and I have no idea where the FakeQuerySet comes from.

I can think of two things that might be improved here:

Don't let FakeQuerySets leak: I don't get them on the console and they're not documented as far as I can see, so why do I get one? The issue is repeatable for me, so I can investigate if needed.
A FakeQuerySet might want to be able to delete its contents.

Django 1.8 support

I am getting this error while trying to use with 1.8.

ImportError: cannot import name RelatedObject

_ClusterTaggableManager breaking manage.py update_index

When running manage.py update_index we are encountering this error:

Traceback (most recent call last):
  File "/var/praekelt/molo-tuneme/manage.py", line 11, in <module>
    execute_from_command_line(sys.argv)
  File "/var/praekelt/python/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 354, in execute_from_command_line
    utility.execute()
  File "/var/praekelt/python/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 346, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/var/praekelt/python/local/lib/python2.7/site-packages/django/core/management/base.py", line 394, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/var/praekelt/python/local/lib/python2.7/site-packages/django/core/management/base.py", line 445, in execute
    output = self.handle(*args, **options)
  File "/var/praekelt/python/local/lib/python2.7/site-packages/wagtail/wagtailsearch/management/commands/update_index.py", line 87, in handle
    self.update_backend(backend_name, object_list)
  File "/var/praekelt/python/local/lib/python2.7/site-packages/wagtail/wagtailsearch/management/commands/update_index.py", line 48, in update_backend
    for chunk in self.print_iter_progress(self.queryset_chunks(queryset)):
  File "/var/praekelt/python/local/lib/python2.7/site-packages/wagtail/wagtailsearch/management/commands/update_index.py", line 103, in print_iter_progress
    for i, value in enumerate(iterable, start=1):
  File "/var/praekelt/python/local/lib/python2.7/site-packages/wagtail/wagtailsearch/management/commands/update_index.py", line 127, in queryset_chunks
    items = list(qs[i * chunk_size:][:chunk_size])
  File "/var/praekelt/python/local/lib/python2.7/site-packages/django/db/models/query.py", line 162, in __iter__
    self._fetch_all()
  File "/var/praekelt/python/local/lib/python2.7/site-packages/django/db/models/query.py", line 967, in _fetch_all
    self._prefetch_related_objects()
  File "/var/praekelt/python/local/lib/python2.7/site-packages/django/db/models/query.py", line 591, in _prefetch_related_objects
    prefetch_related_objects(self._result_cache, self._prefetch_related_lookups)
  File "/var/praekelt/python/local/lib/python2.7/site-packages/django/db/models/query.py", line 1516, in prefetch_related_objects
    obj_list, additional_lookups = prefetch_one_level(obj_list, prefetcher, lookup, level)
  File "/var/praekelt/python/local/lib/python2.7/site-packages/django/db/models/query.py", line 1615, in prefetch_one_level
    prefetcher.get_prefetch_queryset(instances, lookup.get_current_queryset(level)))
  File "/var/praekelt/python/local/lib/python2.7/site-packages/taggit/managers.py", line 137, in get_prefetch_queryset
    qs = self.get_queryset(query).using(db).extra(
TypeError: get_query_set() takes exactly 1 argument (2 given)

Running print type(self) just before the break gives:
<class 'modelcluster.contrib.taggit._ClusterTaggableManager'>

The cause seems to be a call to _TaggableManager.get_prefetch_queryset() which calls self.get_query_set(query). While _TaggableManager.get_query_set() accepts an optional parameter, the the overridden implementation in _ClusterTaggableManager doesn't.

ParentalManyToManyField.value_from_object fails on unsaved instances on Django 1.10

In Django 1.10, ManyToManyField.value_from_object always returns an empty set of related objects when passed an instance without a primary key:
django/django@f529d0c

This is not valid for ParentalManyToManyField, since an unsaved instance can have related objects; we should therefore override value_from_object to bypass this test.

Can't save model with ManyToMany Field

I've found a bug in save of m2m related objects.
Steps to reproduce:

Install wagtail CMS
Create single Page model
Create standard Django model
Add ManyToManyField from your Page model to standard django model
Try to create page instance

When you try to save it, you'll receive following error:

Traceback:
File "/home/artur/Projekty/demo/local/lib/python2.7/site-packages/django/core/handlers/base.py" in get_response
  114.                     response = wrapped_callback(request, *callback_args, **callback_kwargs)
File "/home/artur/Projekty/demo/local/lib/python2.7/site-packages/django/contrib/auth/decorators.py" in _wrapped_view
  22.                 return view_func(request, *args, **kwargs)
File "/home/artur/Projekty/demo/src/wagtail/wagtail/wagtailadmin/views/pages.py" in create
  172.             page = form.save(commit=False)  # don't save yet, as we need treebeard to assign tree params
File "/home/artur/Projekty/demo/local/lib/python2.7/site-packages/modelcluster/forms.py" in save
  257.             self.save_m2m()
File "/home/artur/Projekty/demo/local/lib/python2.7/site-packages/django/forms/models.py" in save_m2m
  96.                 f.save_form_data(instance, cleaned_data[f.name])
File "/home/artur/Projekty/demo/local/lib/python2.7/site-packages/django/db/models/fields/related.py" in save_form_data
  1527.         setattr(instance, self.attname, data)
File "/home/artur/Projekty/demo/local/lib/python2.7/site-packages/django/db/models/fields/related.py" in __set__
  835.         manager = self.__get__(instance)
File "/home/artur/Projekty/demo/local/lib/python2.7/site-packages/django/db/models/fields/related.py" in __get__
  825.             through=self.field.rel.through,
File "/home/artur/Projekty/demo/local/lib/python2.7/site-packages/django/db/models/fields/related.py" in __init__
  522.                                  (instance, source_field_name))

It tries to execute save_m2m() before saving page instance. Is there workaround for this issue?

sort_by_fields() fails when model is ordered through a foreign key

If the ordering field of a model is configured to order the objects over a foreign key relationship (using the __ syntax), sort_by_fields() fails.

E.g.

   class Car(models.Model):
   ...
   class Meta:
       ordering = ['brand__name']

'FakeQuerySet' object has no attribute 'exclude'

Wagtails preview function fails on extended Page models which make use of djangos "exclude" queryset.
https://docs.djangoproject.com/en/1.8/ref/models/querysets/.

Add a check to make sure ClusterableModel is used where needed

This is just a proposal but I think it would be nice if we could the Django check framework in order to raise an error when a ParentalKey or a ParentalManyToManyField is used but the model does not inherit from ClusterableModel.

In my case I had to dig and guess on why my many to many field was not saved although everything was executing properly.

Just a suggestion - I don't know how hard that would be to implement.

Cluster TaggableManager is doing too much queries

I'm using a ClusterTaggableManager on a Wagtail project that I'm currently developing. I realized that using this Manager it performs a huge amount of queries when rendering templates,

while TaggableManager don't.

I've tried to replace get_queryset = get_query_set by get_queryset = _TaggableManager.get_query_set and it seems to work everything (I think) and don't perform that queries. I also saw that there is a comment in that function that begins with "FIXME". Why is this custom get_query_set method needed? Could be improved easily to not perform such amount of queries?

ClusterForm.is_multipart should return True if any child forms are multipart

wagtail/wagtail#3435

ChildFormSet does not validate unique_together constraints involving the foreign key

Reported here: https://groups.google.com/d/msg/wagtail/zDe5ocpL5xA/_If71AkzCmkJ

With Django's InlineFormSet, this validation happens automatically as part of the standard is_valid method - no need to add a custom validator - so ChildFormSet should do the same. Have created a new branch with failing unit tests: https://github.com/gasman/django-modelcluster/tree/fix/unique_together_validation

It looks like InlineFormSet does this by adding the foreign key field to the form and setting it on the instances, so that ModelFormSet's validation is applied on it:

https://github.com/django/django/blob/681df1aeafb30092430157f7977f713e1ce234ca/django/forms/models.py#L888-893
https://github.com/django/django/blob/681df1aeafb30092430157f7977f713e1ce234ca/django/forms/models.py#L939-944

however, this won't work for us in that form, because we can't guarantee that the parent object has been assigned an ID. It looks like ModelFormSet's validation also ignores any groups of values involving None, which will affect us:

https://github.com/django/django/blob/681df1aeafb30092430157f7977f713e1ce234ca/django/forms/models.py#L676

Add modelcluster to Travis

imho, this repo should be Travis CI-ed.

Tests have timezone issue

Pretty minor issue - but two of the tests fail for me due to different timezones. Running tests with python runtests.py

======================================================================
FAIL: test_deserialise_with_utc_datetime (tests.tests.test_serialize.SerializeTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/code/tests/tests/test_serialize.py", line 177, in test_deserialise_with_utc_datetime
    self.assertEqual(log.time, expected_time)
AssertionError: datet[21 chars] 1, 16, 1, 42, tzinfo=<django.utils.timezone.L[34 chars]e80>) != datet[21 chars] 1, 11, 1, 42, tzinfo=<django.utils.timezone.L[34 chars]e80>)

======================================================================
FAIL: test_serialise_with_naive_datetime (tests.tests.test_serialize.SerializeTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/code/tests/tests/test_serialize.py", line 144, in test_serialise_with_naive_datetime
    self.assertEqual(log_json['time'], '2014-08-01T16:01:42Z')
AssertionError: '2014-08-01T11:01:42Z' != '2014-08-01T16:01:42Z'
- 2014-08-01T11:01:42Z
?             ^
+ 2014-08-01T16:01:42Z
?             ^

Validation on models with deferred related model does no fully work

Attempting to validate a Form Page in Wagtail that has FormFields associated with it via a ParentalKey doesn't allow us to validate that certain formfields make sense. In the clean function of the Page, any call to my related object queryset doesn't return any of the about to be created form fields, only ones that already exist.

The requirement: We have some email related fields that rely on certain form fields potentially existing, so if one of those fields is set, we validate that the formfield exists.

The issue: Until the page is actually saved, those form fields don't exist, so validating against form fields that are about to be saved in page.clean doesn't work. We first have to create the form fields, then add to that field, then save again.

Steps to reproduce:

Using a form page based on the below code, go to create a new page.

On that new page set a form field with the name email and populate reply_to_from_field with the same. Save draft. Despite the form field existing reply_to_from_field will raise a validation error.

If you unset reply_to_from_field, then save draft, and then fill reply_to_from_field, and save again, it will now work.

Expected result

Some way to get at the deferred objects when validating the Page model, so we can check if the models about to be created will actually fulfil the requirements of the Page.

Our code

class SafeCaptchaFormBuilder(WagtailCaptchaFormBuilder):

    @property
    def formfields(self):
        fields = super(SafeCaptchaFormBuilder, self).formfields
        if not settings.RECAPTCHA_PUBLIC_KEY:
            fields.pop(self.CAPTCHA_FIELD_NAME)
        return fields


class FormField(AbstractFormField):
    page = ParentalKey('FormPage', related_name='form_fields')


class FormPage(BasePage, WagtailCaptchaEmailForm):
    intro = RichTextField()
    thank_you_text = RichTextField()
    reply_to_from_field = models.CharField(
        max_length=255, blank=True,
        help_text=(
            "Label of the form field to get reply-to from. "
            "Supercedes From Address.")
    )

    content_panels = [
        FormSubmissionsPanel(),
        FieldPanel('title', classname="full title"),
        FieldPanel('intro', classname="full"),
        InlinePanel('form_fields', label="Form fields"),
        FieldPanel('thank_you_text', classname="full"),
        MultiFieldPanel([
            FieldPanel('to_address'),
            FieldRowPanel([
                FieldPanel('reply_to_from_field', classname="col6"),
                FieldPanel('from_address', classname="col6"),
            ]),
            FieldPanel('subject'),
        ], "Email"),
    ]

    def __init__(self, *args, **kwargs):
        super(FormPage, self).__init__(*args, **kwargs)
        self.form_builder = SafeCaptchaFormBuilder

    def clean(self):
        super(FormPage, self).clean()

        if self.reply_to_from_field:
            reply_field = str(slugify(self.reply_to_from_field))
            found = False
            is_email = False
            for field in self.form_fields.all():
                if field.clean_name == reply_field:
                    found = True
                    if field.field_type == "email":
                        is_email = True

            if not found:
                raise ValidationError(
                    {'reply_to_from_field': (
                        'Form field with this label must exist.')}
                )
            if not is_email:
                raise ValidationError(
                    {'reply_to_from_field': (
                        'Form field with this label must be an email field.')}
                )

    def send_mail(self, form):
        addresses = [x.strip() for x in self.to_address.split(',')]
        content = []
        reply_field = str(slugify(self.reply_to_from_field))
        from_address = None
        for field in form:
            value = field.value()
            if isinstance(value, list):
                value = ', '.join(value)
            content.append('{}: {}'.format(field.label, value))
            if str(slugify(field.label)) == reply_field:
                from_address = value
        content = '\n'.join(content)
        if from_address:
            send_mail(self.subject, content, addresses, from_address,)
        else:
            send_mail(self.subject, content, addresses, self.from_address,)

Django 1.8 support

Quite a lot of the related field internals have changed in Django 1.8.

Heres some changes that are causing problems for django-modelcluster:

Formset test failures on Django 1.11a1

Several tests are failing against Django 1.11a1, due to this change:

django/django@181f492
https://code.djangoproject.com/ticket/27416

Creating test database for alias 'default'...
System check identified no issues (0 silenced).
..............................FF.........F..........F........................x..
======================================================================
FAIL: test_formsets_from_model_superclass_are_exposed (tests.tests.test_cluster_form.ClusterFormTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vagrant/libs/django-modelcluster/tests/tests/test_cluster_form.py", line 451, in test_formsets_from_model_superclass_are_exposed
    self.assertEqual(instance.reviews.count(), 1)
AssertionError: 0 != 1

======================================================================
FAIL: test_formsets_from_model_superclass_with_explicit_formsets_def (tests.tests.test_cluster_form.ClusterFormTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vagrant/libs/django-modelcluster/tests/tests/test_cluster_form.py", line 482, in test_formsets_from_model_superclass_with_explicit_formsets_def
    self.assertEqual(instance.reviews.count(), 1)
AssertionError: 0 != 1

======================================================================
FAIL: test_child_updates_without_ids (tests.tests.test_formset.ChildFormsetTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vagrant/libs/django-modelcluster/tests/tests/test_formset.py", line 240, in test_child_updates_without_ids
    self.assertEqual(2, beatles.members.count())
AssertionError: 2 != 1

======================================================================
FAIL: test_incoming_formset_data (tests.tests.test_formset.TransientFormsetTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vagrant/libs/django-modelcluster/tests/tests/test_formset.py", line 43, in test_incoming_formset_data
    self.assertEqual(2, len(members))
AssertionError: 2 != 1

----------------------------------------------------------------------
Ran 80 tests in 0.608s

FAILED (failures=4, expected failures=1)

Serialised datetimes are not timezone aware

Reported by @timheap in: wagtail/wagtail#494 (comment)

When datetimes are serialised, they are not converted to UTC nor any timezone information is saved with them.

The datetimes should be converted to UTC on serialisation and converted back to localtime on deserialisation.

If we do start converting datetimes to/from UTC, it's important to take into account the fact that old datetimes will still be in localtime and may have not been converted.

Missing 3.1 release and git tag

Hi,
I found out there is a release 3.1 on pypi
and in CHANGELOG

django-modelcluster/CHANGELOG.txt

Lines 15 to 24 in 63e2dc4

    
           3.1 (07.04.2017) 
        
           ~~~~~~~~~~~~~~~~ 
        
           * Django 1.11 compatibility 
        
           * Python 3.6 compatibility 
        
           * Added the ability to install the optional dependency `django-taggit` 
        
             using `pip install django-modelcluster[taggit]` 
        
           * Fix: ClusterForm.save(commit=True) now correctly writes ParentalManyToManyField relations back to the database rather than requiring a separate model.save() step 
        
           * Fix: ClusterForm.is_multipart() now returns True when a child form requires multipart submission 
        
           * Fix: ClusterForm.media now includes media defined on child forms 
        
           * Fix: ParentalManyToManyField.value_from_object now returns correct result on unsaved objects

For some reason 3.1 release isn't on github release page and v3.1 tag is missing.

Can we fix it please?

convert rel to remote_field

Implement filters with double-underscore clauses

As per

django-modelcluster/modelcluster/queryset.py

Line 48 in fa1e2fc

    
           raise NotImplementedError("Complex filters with double-underscore clauses are not implemented yet")

. Ref wagtail/wagtail#3712

ClusterForm does not pick up child relations defined on a superclass of the model

On wagtaildemo:

from demo.models import HomePage
from wagtail.wagtailcore.models import Page

class HomePageForm(ClusterForm):
    class Meta:
        model = HomePage

class PageForm(ClusterForm):
    class Meta:
        model = Page

>>> HomePageForm.formsets
{'related_links': <class 'django.forms.formsets.HomePageRelatedLinkFormFormSet'>, 'carousel_items': <class 'django.forms.formsets.HomePageCarouselItemFormFormSet'>}

>>> PageForm.formsets
{'advert_placements': <class 'django.forms.formsets.AdvertPlacementFormFormSet'>}

HomePageForm.formsets should include the advert_placements formset, but doesn't. This is because it consults HomePage._meta.child_relations, which doesn't contain the advert_placements relation. Need to check the semantics of child_relations to figure out whether it ought to include relations on the superclass, or not (and if not, we should fix it within ClusterForm by walking the model's superclasses to build the list of formsets).

Django 2.0 support (dropping Django 1.8 support ?)

We are using both Wagtail and ModelCluster on a project and after trying to run Django 2.0 Beta 1 I found some issues in ModelCluster.

I've made a few changes locally to make it run:

field.rel -> field.remote_field (in django.db.models.fields.Field)
field.rel.to -> field.remote_field.model (in django.db.models.fields.reverse_related.ForeignObjectRel)

Looking through the issues I've found your answer in Wagtail's issue #3599. I was wondering if there are any plans to drop the support of Django 1.8 for ModelCluster and move forward with those changes.

And if that's the case I can provide a PR of those changes if you want.

Move tags.py to contrib/taggit.py?

Shows taggit support is optional
Gives space for other tagging systems to be supported
Removes non-essential stuff from the core part of modelcluster

ClusterTaggableManager broken with django-taggit 0.11

jazzband/django-taggit@4f2e47f adds a new required parameter (prefetch_cache_name) in _TaggableManager.init, which we're not passing in https://github.com/torchbox/django-modelcluster/blob/c713d8f5f0d3261e06537346a78e6fc47ed89878/modelcluster/tags.py#L83 .

Data migration doesn't write to ParentalManyToManyField

I ran into an issue writing a data migration for a ParentalManyToManyField today where the data migration wouldn't write any data to the tables for that field. The migration looked something like this:

# -*- coding: utf-8 -*-
# Generated by Django 1.11.11 on 2018-04-19 14:10
from __future__ import unicode_literals

from django.db import migrations


def forwards(apps, schema_editor):
    """
    Move all data from Topic, Country, and Language to TemporaryTopic,
    TemporaryCountry, and TemporaryLanguage
    """

    TemporaryTopic = apps.get_model('directory', 'TemporaryTopic')
    TemporaryCountry = apps.get_model('directory', 'TemporaryCountry')
    TemporaryLanguage = apps.get_model('directory', 'TemporaryLanguage')
    DirectoryEntry = apps.get_model('directory', 'DirectoryEntry')

    # ...

    # Move DirectoryEntry fks
    for entry in DirectoryEntry.objects.all():
        topic_titles = list(entry.topics.values_list('title', flat=True))
        country_titles = list(entry.countries.values_list('title', flat=True))
        language_titles = list(entry.languages.values_list('title', flat=True))
        entry.temporary_topics.set(
            TemporaryTopic.objects.filter(title__in=topic_titles)
        )
        entry.temporary_countries.set(
            TemporaryCountry.objects.filter(title__in=country_titles)
        )
        entry.temporary_languages.set(
            TemporaryLanguage.objects.filter(title__in=language_titles)
        )
        entry.save()

    # ...

class Migration(migrations.Migration):
    # ...
    operations = [
        migrations.RunPython(forwards),
    ]

@gasman suspects the fake model that gets built by django's migration logic doesn't inherit from ClusterableModel and is therefore missing some of the logic necessary to make this data migration work.

I ended up using a workaround in which I had a migration convert my ParentalManyToManyFields to Django's built-in ManyToManyField before the data migration and then another migration afterward to convert them back, which seems to have worked fine.

_default_manager should be used instead of objects

Ran into an issue where if I used a custom manager ClusterableModels will sometimes throw an error when using the custom manager. It appears the the get_queryset function in fields.py specifically line 337 is defaulting to using objects instead of _default_manager. The issue can be worked around by assigning an objects manager, but _default_manager should be used instead.

Preview doesn't reflect changes made to the order of Orderable items.

I created a detailed description of this problem in the wagtail issue queue: wagtail/wagtail#867. I'm not sure if the bug is in wagtail or django-modelcluster, which is why I'm posting this reference issue in this queue.

<Foo object at ...> is not JSON serializable

Model cluster does not support primary keys that are not of simple types.

If you have a primary key that uses something like this CashField from django's tests that doesn't inherit from one of the basic types known to the DjangoJSONEncoder then you get <Foo object at ...> is not JSON serializable.

The Cash object I linked to inherits from decimal.Decimal but that is because it is only a simple example for testing.

ModelCluster needs to do some sort of is_serializable check at https://github.com/torchbox/django-modelcluster/blob/c72110e7789415196336584c70b2706fe74dbc2d/modelcluster/models.py#L44 and use Field.value_to_string or the like if the object isn't serializable

Nested OneToMany relationship results in 3 empty forms in wagtail admin

I have some Wagtail models that look like this:

events.models.py:

from django.db import models
from wagtail.core.models import Page, Orderable
from modelcluster.models import ClusterableModel
from wagtail.admin.edit_handlers import InlinePanel

class Event(Page):
    ...
    content_panels = Page.content_panels + [
        InlinePanel('event_presentations'),
    ]

class Presentation(Orderable, ClusterableModel):
    event = ParentalKey('events.Event', related_name='event_presentations')
    ...

    panels = [
                InlinePanel('presentation_dates', label='Presentation date times'
    ]

class PresentationDate(models.Model):
    presentation = ParentalKey('events.Presentations', related_name='presentation_dates', on_delete=models.CASCADE)
    date_time = models.DateTimeField("Presentation date and time")

This results in three extra empty 'Presentation Date' forms being present in the admin whenever a Presentation form is created. Changing the extra_form_count in the django-modelcluster ClusterFormMetaclass from 3 to 0 seems to fix the problem (django-modelcluster/forms.py line 215). Is there a way to override this value without editing the source code of django-modelcluster?

RemovedInDjango20Warning: Usage of field.rel has been deprecated. Use field.remote_field instead.

When using python -Wd manage.py test on my codebase, to try to ferret out any changes I need to make in my own code for forward compatibility, I discovered that modelcluster throws this deprecation warning three times, all in fields.py (twice on line 257, once on 270):

RemovedInDjango20Warning: Usage of field.rel has been deprecated. Use field.remote_field instead.

I went in and edited those lines to replace field.rel with field.remote_field, and it seems to have worked fine. But I don't know how to run the modelcluster test suite against that change to find out.

ClusterForm.media should incorporate media from child formsets

If a field within a child formset defines form media, this won't currently get picked up as part of the base form.media property.

A fix for this within Wagtail is pending at wagtail/wagtail#2673, but it would be appropriate to fix this at the ClusterForm level too.

Errors when adding new field

When trying to add a new field, the following error occurred:

$ ./manage.py schemamigration --auto core
/home/vagrant/.virtualenvs/myproject/local/lib/python2.7/site-packages/modelcluster/tags.py:8: DeprecationWarning: The modelcluster.tags module has been moved to modelcluster.contrib.taggit
  "modelcluster.contrib.taggit", DeprecationWarning)

 ! Cannot freeze field 'core.postpage.tags'
 ! (this field has class modelcluster.contrib.taggit.ClusterTaggableManager)

 ! South cannot introspect some fields; this is probably because they are custom
 ! fields. If they worked in 0.6 or below, this is because we have removed the
 ! models parser (it often broke things).
 ! To fix this, read http://south.aeracode.org/wiki/MyFieldsDontWork

For reference:

$ pip freeze
Django==1.6.7
South==1.0
django-modelcluster==0.6.1
wagtail==0.8.5

Running $ pip install -U django-modelcluster==0.5 has solved the issue.
It was suggested that I post this as an issue in here.
Thanks.

	3.1 (07.04.2017)
	~~~~~~~~~~~~~~~~
	* Django 1.11 compatibility
	* Python 3.6 compatibility
	* Added the ability to install the optional dependency `django-taggit`
	using `pip install django-modelcluster[taggit]`
	* Fix: ClusterForm.save(commit=True) now correctly writes ParentalManyToManyField relations back to the database rather than requiring a separate model.save() step
	* Fix: ClusterForm.is_multipart() now returns True when a child form requires multipart submission
	* Fix: ClusterForm.media now includes media defined on child forms
	* Fix: ParentalManyToManyField.value_from_object now returns correct result on unsaved objects