Opened 6 years ago

Closed 16 months ago

Last modified 16 months ago

#14196 closed New feature (fixed)

Objects that come from something_set, should have their parent object filled in

Reported by: Mark Jones Owned by:
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

def avatar_url(user, size=80):
    if not isinstance(user, User):
        try:
            user = User.objects.get(username=user)
        except User.DoesNotExist:
            return AVATAR_DEFAULT_URL
    avatars = user.avatar_set.order_by('-date_uploaded')
    primary = avatars.filter(primary=True)
    if primary.count() > 0:
        avatar = primary[0]
    elif avatars.count() > 0:
        avatar = avatars[0]
    else:
        avatar = None
    if avatar is not None:
        avatar.user = user # prevent an extra lookup because the avatar doesn't know he came from user.avatar_set

If I don't put in the last line in that sample, accessing avatar.user will again query the database to get the "parent" object. It seems to me that when you do user.avatar_set.all(), the user in the avatar object would get set before the object is returned, but that isn't the case, instead it loads another user object from the db and stuffs it into the avatar object.

Change History (11)

comment:1 Changed 6 years ago by niall

Component: UncategorizedDatabase layer (models, ORM)
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted
Version: 1.2SVN

comment:2 Changed 5 years ago by Graham King

Severity: Normal
Type: New feature

comment:3 Changed 5 years ago by Aymeric Augustin

UI/UX: unset

Change UI/UX from NULL to False.

comment:4 Changed 5 years ago by Aymeric Augustin

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:5 Changed 16 months ago by Rigel Di Scala

Owner: changed from nobody to Rigel Di Scala
Status: newassigned

Assigning to myself for work.

comment:6 Changed 16 months ago by Rigel Di Scala

I cannot reproduce this using the following test:

    def test_prepopulated_parent_of_a_set(self):
        my_user = User.objects.create(username="MyUser")
        LogEntry.objects.create(user=my_user, action_flag=0)

        user_logs = my_user.logentry_set.all()
        first_log = user_logs[0]
        with self.assertNumQueries(0):
            log_user = first_log.user
            self.assertEquals(my_user.username, log_user.username)

This test passes, meaning no additional queries were executed when attempting to lookup the parent user. Maybe I'm missing something obvious?

comment:7 Changed 16 months ago by Rigel Di Scala

Owner: Rigel Di Scala deleted
Status: assignednew

comment:8 Changed 16 months ago by Tim Graham

Resolution: fixed
Status: newclosed

comment:9 Changed 16 months ago by Carl Meyer

Thanks for investigating!

comment:10 Changed 16 months ago by Mark Jones

I'm betting over the course of the last 5 years this got fixed. Might be a good idea to add the test code to prevent regression though.

comment:11 Changed 16 months ago by Aymeric Augustin

known_related_objects.tests.ExistingRelatedInstancesTests.test_foreign_key tests this.

I couldn't find easily when it was introduced because the modeltests/regressiontests merge breaks git log.

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