Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#23611 closed Bug (fixed)

update_or_create doesn't retain related manager reference

Reported by: Ryan Hiebert Owned by: André Ericson
Component: Database layer (models, ORM) Version: 1.7
Severity: Normal Keywords:
Cc: Loic Bistuer Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Ryan Hiebert)

The update_or_create method on a related manager doesn't seem to retain the reference from the object that it came from, as I've come to expect from other methods (get, create, get_or_create). For example, the following models.py:

from django.db import models

class Spam(models.Model):
    pass

class Egg(models.Model):
    spam = models.ForeignKey(Spam, related_name='eggs')

And these tests:

from django.test import TestCase

from .models import Spam, Egg

class TestUpdateOrCreate(TestCase):
    def test_update_or_create_on_model_manager(self):
        spam = Spam.objects.create()
        Egg.objects.update_or_create(id=7, defaults={'spam': spam})

    def test_update_or_create_on_related_manager(self):
        spam = Spam.objects.create()
        spam.eggs.update_or_create(id=8)

    def test_create_on_related_manager(self):
        spam = Spam.objects.create()
        spam.eggs.create(id=9)

    def test_get_or_create_on_related_manager(self):
        spam = Spam.objects.create()
        spam.eggs.get_or_create(id=9)

All the tests but test_update_or_create_on_related_manager succeed. That one fails with an IntegrityError:

django.db.utils.IntegrityError: NOT NULL constraint failed: django_related_update_egg.spam_id

This is the case in the 1.7 release, the latest of the 1.7.x branch, and in the master branch.

Change History (16)

comment:1 Changed 6 years ago by Ryan Hiebert

Description: modified (diff)

comment:2 Changed 6 years ago by André Ericson

Owner: changed from nobody to André Ericson
Status: newassigned

comment:3 Changed 6 years ago by Konrad Świat

I would like to work on this one too, if You (aericson) have nothing against it. I have experienced similar problems with related managers recently and have done already some research.

comment:4 Changed 6 years ago by André Ericson

@kswiat I have submitted a pull request at https://github.com/django/django/pull/3321 let me know if you have any suggestions.

Last edited 6 years ago by André Ericson (previous) (diff)

comment:5 Changed 6 years ago by André Ericson

Has patch: set

comment:6 Changed 6 years ago by Ryan Hiebert

Thank you @aericson. The PR is against master, and I'm unfamiliar with the process. How do changes like this that affect 1.7 get backported to the 1.7 branch? Would this one qualify?

comment:7 Changed 6 years ago by Collin Anderson

It qualifies for a backport if only if 1.7 is behaving differently than version 1.6. What happens in version 1.6?

comment:8 Changed 6 years ago by André Ericson

I believe update_or_create was added at 1.7, wasn't it?

comment:9 Changed 6 years ago by Ryan Hiebert

update_or_create was added in 1.7, but the behavior fixed in this patch has precedent in 1.6 with get, get_or_create, create, and others.

comment:10 Changed 6 years ago by André Ericson

I'm not exactly sure how the process works neither.

Should I make a pull request against stable/1.7.x?
I believe the fix should apply to that aswell.

Last edited 6 years ago by André Ericson (previous) (diff)

comment:11 Changed 6 years ago by Collin Anderson

Triage Stage: UnreviewedAccepted

Ahh. I forgot update_or_create was new. Not sure on backporting, but, in any case the pull request is correctly against master.

comment:12 Changed 6 years ago by Loic Bistuer

Cc: Loic Bistuer added

It's missing from GenericRelatedObjectManager; note that GenericRelatedObjectManager.get_or_create() is missing too.

Since it's a bug in a new feature, I think it's worthy of a backport.

comment:13 in reply to:  12 Changed 6 years ago by André Ericson

Replying to loic:

It's missing from GenericRelatedObjectManager; note that GenericRelatedObjectManager.get_or_create() is missing too.

Since it's a bug in a new feature, I think it's worthy of a backport.

I'll be adding them to GenericRelatedObjectManager. Thanks

comment:14 Changed 6 years ago by André Ericson

Pull request updated.

comment:15 Changed 6 years ago by Loic Bistuer <loic.bistuer@…>

Resolution: fixed
Status: assignedclosed

In ed37f7e979186c99a6f351c289eb486461601d80:

Fixed #23611 -- update_or_create failing from a related manager

Added update_or_create to RelatedManager, ManyRelatedManager and
GenericRelatedObjectManager.
Added missing get_or_create to GenericRelatedObjectManager.

comment:16 Changed 6 years ago by Loic Bistuer <loic.bistuer@…>

In fa4b6482df08d308fe88044b8c8bf981c6225fb8:

[1.7.x] Fixed #23611 -- update_or_create failing from a related manager

Added update_or_create to RelatedManager, ManyRelatedManager and
GenericRelatedObjectManager.
Added missing get_or_create to GenericRelatedObjectManager.

Conflicts:

tests/generic_relations/tests.py
tests/get_or_create/tests.py

Backport of ed37f7e979 from master

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