Opened 9 years ago

Closed 9 years ago

#2751 closed defect (duplicate)

ManyToManyField in models with edit_inline breaks the admin

Reported by: bangcok_dangerus@… Owned by: adrian
Component: contrib.admin Version: master
Severity: normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

A model that contains a ForeignKey with the edit_inline option breaks the admin if it also contains a ManyToManyField. The two problems I've found are:

1) Contrary to the docs, 'core=True' doesn't seem to do apply to a ManyToManyField. Example:

class Car(models.Model):
	driver = models.ForeignKey(Driver, edit_inline=models.STACKED)
	parts = models.ManyToManyField(Part, core=True)

The above code produces the error

test.car: At least one field in Car should have core=True, because it's being edited inline by driver.Driver.

2) Once the above problem is fixed using something like the code below, the admin interface will successfully display the inline Car objects, including the ManyToManyField. As long the Car objects are created and edited using their own admin pages (i.e. NOT inline), things seem to work fine.

class Car(models.Model):
	name = models.CharField(maxlength=20, core=True)
	driver = models.ForeignKey(Driver, edit_inline=models.STACKED)
	parts = models.ManyToManyField(Part, core=True)

However, attempting to create or edit a car object inline with a Driver creates the following traceback:

Traceback (most recent call last):
File "/usr/local/lib/python2.4/site-packages/django/core/handlers/base.py" in get_response
  74. response = callback(request, *callback_args, **callback_kwargs)
File "/usr/local/lib/python2.4/site-packages/django/contrib/admin/views/decorators.py" in _checklogin
  55. return view_func(request, *args, **kwargs)
File "/usr/local/lib/python2.4/site-packages/django/views/decorators/cache.py" in _wrapped_view_func
  39. response = view_func(request, *args, **kwargs)
File "/usr/local/lib/python2.4/site-packages/django/contrib/admin/views/main.py" in change_stage
  329. new_object = manipulator.save(new_data)
File "/usr/local/lib/python2.4/site-packages/django/db/models/manipulators.py" in save
  218. was_changed = getattr(new_rel_obj, 'set_%s' % f.name)(rel_new_data[f.attname])

  AttributeError at /admin/test/driver/1/
  'Car' object has no attribute 'set_parts'

Additionally, trying to add a Driver (with the inline Cars left untouched) creates the traceback:

Traceback (most recent call last):
File "/usr/local/lib/python2.4/site-packages/django/core/handlers/base.py" in get_response
  74. response = callback(request, *callback_args, **callback_kwargs)
File "/usr/local/lib/python2.4/site-packages/django/contrib/admin/views/decorators.py" in _checklogin
  55. return view_func(request, *args, **kwargs)
File "/usr/local/lib/python2.4/site-packages/django/views/decorators/cache.py" in _wrapped_view_func
  39. response = view_func(request, *args, **kwargs)
File "/usr/local/lib/python2.4/site-packages/django/contrib/admin/views/main.py" in add_stage
  254. new_object = manipulator.save(new_data)
File "/usr/local/lib/python2.4/site-packages/django/db/models/manipulators.py" in save
  198. new_rel_obj.save()
File "/usr/local/lib/python2.4/site-packages/django/db/models/base.py" in save
  204. ','.join(placeholders)), db_values)
File "/usr/local/lib/python2.4/site-packages/django/db/backends/util.py" in execute
  12. return self.cursor.execute(sql, params)

  IntegrityError at /admin/test/driver/add/
  ERROR: null value in column "name" violates not-null constraint INSERT INTO "test_car" ("driver_id") VALUES (10)

The model used for the driver is simply:

class Driver(models.Model):
	name = models.CharField(maxlength=20)

Even more strangely, returning to the admin interface after the exception reveals that the driver WAS actually created.

-Basil.

Change History (8)

comment:1 Changed 9 years ago by ramiro

Hi, could you please post your test/models.py file?. I'm trying to reproduce this bug, but I would like to clone your environment as close as possible.

Regards,

comment:2 Changed 9 years ago by bangcok_dangerus (at) hotmail.com

sure:

from django.db import models

class Driver(models.Model):
	name = models.CharField(maxlength=20)
	class Admin:
		pass
	def __str__(self):
		return self.name

class Part(models.Model):
	name=models.CharField(maxlength=20)
	class Admin:
		pass
	def __str__(self):
		return self.name

class Car(models.Model):
	name = models.CharField(maxlength=20, core=True)
	driver = models.ForeignKey(Driver, edit_inline=models.STACKED)
	parts = models.ManyToManyField(Part, core=True)
	class Admin:
		pass
	def __str__(self):
		return self.name

-Basil.

comment:3 Changed 9 years ago by ramiro

Basil,

I've reproduced the conditions you describe, but reading the documentation I see
there is no explicit nor implicit affirmation that the core parameter
applies to relationships, the option is only described as a field option.

That would explaing the

test.car: At least one field in Car should have core=True, because it's being edited inline by driver.Driver.

error you are getting when the models are validated.

comment:4 Changed 9 years ago by bangcok_dangerus (at) hotmail.com

Yes, I see your point. 'core=True' works just fine on a ForeignKey though, so it would make sense to extend it to other relationships too. For example, the following model works fine:

class Driver(models.Model):
	name = models.CharField(maxlength=20)
	class Admin:
		pass
	def __str__(self):
		return self.name

class Dealer(models.Model):
	name=models.CharField(maxlength=20)
	class Admin:
		pass
	def __str__(self):
		return self.name

class Car(models.Model):
	driver = models.ForeignKey(Driver, edit_inline=models.STACKED)
	bought_from = models.ForeignKey(Dealer, core=True)
	class Admin:
		pass
	def __str__(self):
		return str(self.id) +str(self.driver)+ str(self.bought_from)

It would be great if the model worked with a ManyToManyField in place of the ForeignKey.

Furthermore, the problem I described in part 2) of the ticket is another matter entirely (I should've opened two separate tickets). The admin still breaks for a legal model if an object that's edited inline contains a ManyToManyField. An example is the full model I posted two posts before this one, with or without 'core=True' on the ManyToManyField.

For some reason I'm not getting the IntegrityError traceback anymore when I try and create a Driver object and leave the inline cars untouched. The AttributeError ('Car' object has no attribute 'set_parts') is the main problem I think.

Thanks for all the quick feedback by the way.

-Basil

comment:5 Changed 9 years ago by rbemrose@…

  • Version changed from SVN to 0.95

I am also having problems with ManyToManyField and edit_inline.

My models are as follows (copied from three different model files with correct imports):

class Developer(models.Model):
    name = models.CharField('Company Name', maxlength=200)
    description = models.TextField('Company Description')
    website = models.URLField('Company URL')
    def __str__(self):
        return self.name
    class Admin:
        pass

class System(models.Model):
    name = models.CharField('System Name', maxlength=200)
    developer = models.ForeignKey(Developer, verbose_name='System Manufacturer')
    description = models.TextField('Description of game system')
    def __str__(self):
        return self.name
    class Admin:
        list_display = ('name', 'developer', 'description')
        list_filter = ['name']
        search_fields = ['name']

class Game(models.Model):
    default_name = models.CharField('Default Name (used in search)', maxlength=100)
    directory = models.CharField('Directory', maxlength=200)
    developer = models.ForeignKey(Developer, verbose_name='Game Developer')
    def __str__(self):
        return self.default_name
    class Admin:
        list_display = ('default_name', 'directory', 'developer')
        list_filter = ['default_name']
        search_fields = ['default_name']

class GameName(models.Model):
    game = models.ForeignKey(Game, edit_inline=models.STACKED, num_in_admin=3)
    name = models.CharField('Game Name', maxlength=200, core=True)
    system = models.ManyToManyField(System)

    def __str__(self):
        return self.name

and it throws

AttributeError at /admin/game/game/add/
'GameName' object has no attribute 'set_system'

It would be a real boon if this was working, as this data needs to be on the same page or the QC manager is going to complain.

comment:6 Changed 9 years ago by rbemrose@…

  • Version changed from 0.95 to SVN

Oops, didn't mean to change the version.

comment:7 Changed 9 years ago by bangcok_dangerus (at) hotmail.com

I feel your pain :)

I've tried to use my [kinda basic] python skills to figure out what's going wrong. The problem seems to be line 218 in [python site-packages directory]/django/db/models/manipulators.py :

was_changed = getattr(new_rel_obj, 'set_%s' % f.name)(rel_new_data[f.attname])

where new_rel_obj is the newly created instance of the object that's being edited inline. The aim here is to save the data for the ManyToManyField of the object that's being edited inline. The code above is doing this by calling a method it thinks is called 'set_[many_to_many model name here], i.e. for my Car example it'll try call set_parts or in rbemrose's model above set_system. The problem is that this method doesn't exist.

I've searched the DB API for any methods called set_[anything] and drew a total blank. I'm guessing this is either very old code that needs to be updated to a newer db API, or just a typo. I took a look at all the methods available to my Car object to try find something that would do the job... I decided to take a look at get_or_create but found that it behaves rather strangely, so I didn't try putting it into manipulators.py to see what would happen. I'll probably open another ticket about get_or_create at some point, but if you're curious to see the behaviour I'm talking about take a look at http://paste.e-scribe.com/1630/, http://paste.e-scribe.com/1634/, and http://paste.e-scribe.com/1635/.

I'm going to keep trying to figure out manipulators.py out of sheer frustration, but it'd be great if a coder who knew what was going on here could step in :)

-Basil

comment:8 Changed 9 years ago by rbemrose@…

  • Resolution set to duplicate
  • Status changed from new to closed

I found out that ticket 2243 also deals with this bug. The second patch there fixes this problem.

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