Opened 7 years ago

Last modified 2 years ago

#8467 new New feature

For ManyToMany manager, we should convert objects being added or removed to the pk type if they are not.

Reported by: Wonlay Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords: Duplicate entry, add, remove, ManyToManyField
Cc: wonlay@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

For example:

class A(models.Model):
	name = models.CharField(blank=True)

class B(models.Model):
	a = models.ManyToManyField(A)

a = A(name='aaa')
a.save()
b = B()
b.save()

# This line works fine:
b.a.add('1')

# But this one will raise 'Duplicate entry' error:
b.a.add('1')

This is caused by '1' is not an integer, the duplication checking code in add() fails when checking the string type '1'.

The same problem is applied to the remove() method.

If we convert the objects to its pk type of the model, code will run correctly.

And my patch is attached.

Attachments (1)

smart_many2many_add_remove.patch (1.2 KB) - added by Wonlay 7 years ago.

Download all attachments as: .zip

Change History (10)

Changed 7 years ago by Wonlay

comment:1 Changed 7 years ago by ubernostrum

  • milestone changed from 1.0 to post-1.0
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 6 years ago by anonymous

  • milestone post-1.0 deleted

Milestone post-1.0 deleted

comment:3 Changed 6 years ago by jacob

  • Triage Stage changed from Unreviewed to Design decision needed

comment:4 Changed 4 years ago by lukeplant

  • Severity set to Normal
  • Type set to New feature

comment:5 Changed 3 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:6 Changed 3 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:7 Changed 2 years ago by aaugustin

  • Component changed from Core (Other) to Database layer (models, ORM)

comment:8 Changed 2 years ago by akaariai

  • Triage Stage changed from Design decision needed to Accepted

I think this fix makes sense. The patch should be made aware of from_field/to_field if possible, even it is not possible to create such ManyToManys currently this feature will very likely appear some day.

comment:9 Changed 2 years ago by timo

  • Needs tests set
  • Patch needs improvement set

Patch needs improvement per @akaariai's comment as well as tests.

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