Opened 13 years ago

Closed 13 years ago

#18005 closed New feature (wontfix)

Add support for ManyToManyFields on proxy models

Reported by: Bradley Ayers <bradley.ayers@…> Owned by: nobody
Component: Database layer (models, ORM) Version: 1.4
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

It appears Django already supports this, but explicitly checks for and prevents it from working:

https://code.djangoproject.com/browser/django/trunk/django/db/models/base.py#L123

if (new_class._meta.local_fields:
        or new_class._meta.local_many_to_many):
    raise FieldError("Proxy model '%s' contains model fields." % name)

By simply removing the second half of the condition, ManyToManyFields work on
proxy models.

In my current project I'm proxying the auth.User as users.User, and I want to add a M2M
relationship to core.District. I don't want to modify core, so I'd like to
add the relationship to the users.User proxy model:

from django.contrib.auth.models import User as AuthUser

class User(AuthUser):
    districts = models.ManyToManyField("core.District")

    class Meta:
        proxy = True

Running ./manage.py sql users shows that the correct SQL is generated:

BEGIN;
CREATE TABLE "users_user_districts" (
    "id" integer NOT NULL PRIMARY KEY,
    "user_id" integer NOT NULL,
    "district_id" integer NOT NULL REFERENCES "core_district" ("id"),
    UNIQUE ("user_id", "district_id")
)
;
COMMIT;

I wrote a crude test case, and it all seems to work:

user = User.objects.create_user("username", "fake@example.com", "password")
district = District.objects.create(name="Foo")
user.districts.add(district)

user = User.objects.get(pk=user.pk)  # use fresh version
assert district.users.count() == 1
assert district.users.all()[0].pk == user.pk

user.districts.clear()
user = User.objects.get(pk=user.pk)  # use fresh version
assert user.districts.count() == 0
assert list(user.districts.all()) == []

The Django test suite passes using SQLite:

$ ./runtests.py --settings=test_sqlite
Creating test database for alias 'default'...
Creating test database for alias 'other'...
...<snip>....
----------------------------------------------------------------------
Ran 4657 tests in 1007.250s

OK (skipped=115, expected failures=2)
Destroying test database for alias 'default'...
Destroying test database for alias 'other'...

Are there any arguments against this?

Change History (1)

comment:1 by Anssi Kääriäinen, 13 years ago

Resolution: wontfix
Status: newclosed

Based on a long and interesting #django-dev discussion it was decided that the answer to this issue is wontfix. As an alternative for this different ways of altering existing models was discussed. I will try to write a posting to django-developers about the ideas presented.

The reason for wontfixing is that Proxy models should not define additional fields. A counter argument is that it is already possible to add a foreign key pointing to a proxy model, but even then the fundamental idea of a Proxy model is that it has the same data as the base class, just different methods. Even the current foreign-key to proxy model is causing some problems.

Note: See TracTickets for help on using tickets.
Back to Top