grantmcconnaughey / django-field-history

A Django app to track changes to model fields.
BSD 3-Clause "New" or "Revised" License
314 stars 33 forks source link

Avoid adding new tracking object for already tracked fields #9

Closed mariodev closed 8 years ago

mariodev commented 8 years ago

Resolves #8

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.02%) to 95.021% when pulling b70a021b6f2fd078a7fa8e3c1bce3c70b6525b50 on mariodev:issue-8-createinitialfieldhistory into 31ee4dcc29c5a168877d8c3c60b3bdae0d1f6b51 on grantmcconnaughey:master.

grantmcconnaughey commented 8 years ago

Hey, thanks for the PR! I like this idea. I think I see a bug, however. This part:

if not FieldHistory.objects.filter(object_id=obj.id, field_name=field).exists():
    # etc.

Should be this:

content_type = ContentType.objects.get_for_model(obj)
if not FieldHistory.objects.filter(object_id=obj.id, content_type=content_type, field_name=field).exists():
    # etc.

That's because object is a GenericForeignKey, so you have to query for object id and content type.

grantmcconnaughey commented 8 years ago

Write a test that fixes that bug, fix the bug, and I'll get it merged soon. Thanks again!

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.06%) to 95.062% when pulling 4ba3a353fd840592162423a69f804241ecff5426 on mariodev:issue-8-createinitialfieldhistory into 31ee4dcc29c5a168877d8c3c60b3bdae0d1f6b51 on grantmcconnaughey:master.

mariodev commented 8 years ago

@grantmcconnaughey Added necessary fixes. Thanks for your help!

grantmcconnaughey commented 8 years ago

Awesome stuff, @mariodev. Thanks a lot. 👍