grantmcconnaughey/django-field-history

ManyToManyField inside tracked field breaks field_value function

Opened this issue · 9 comments

ljle commented

Consider the following models:

class OKB_Article(models.Model):
    VISUALIZATION_CHOICES = (
        ('is_private', 'Privado'),
        ('is_public', 'Publico'),
    )
    id_record = models.BigIntegerField()
    title = models.CharField(max_length=200)
    visualization = models.CharField(
        max_length=50, choices=VISUALIZATION_CHOICES, default='is_private')
    article_grouping = models.ForeignKey(
            OKB_ArticleGrouping,
            blank=True,
            null=True)
    author = models.ForeignKey(User, related_name='user_author')
    motive_type = models.ForeignKey(OSD_Motive, blank=True, null=True)
    article_type = models.ForeignKey(OKB_ArticleType, blank=True, null=True)
    content = models.TextField()
    approved_by = models.ForeignKey(
            User, related_name='approved_by', blank=True, null=True)
    created_date = models.DateTimeField(auto_now_add=True)
    status_type = models.ForeignKey(OSD_Status)

    history = FieldHistoryTracker([
        'motive_type', 'article_grouping', 'article_type',
        'visualization', 'status_type'
        ])

    class Meta:
        db_table = 'okb_article'

    def __unicode__(self):
        return self.title
class OSD_Status(models.Model):
    name = models.CharField(max_length=50)
    color = models.CharField(max_length=6, choices=COLOR_CHOICES,
                             default="ffce93")
    description = models.CharField(max_length=200)
    behavior = models.ForeignKey(OSD_Behavior)
    motive = models.ManyToManyField(OSD_Motive, through='OSD_StatusMotive')
    status = models.BooleanField(default=True)

    class Meta:
        db_table = 'osd_status'

    def __unicode__(self):
        return "%s - %s" % (self.name, self.behavior)

As you can see the field status_type in OKB_Article has a ManyToManyField called motive.

If I'm tracking the status_type field, the field_value function only works for that field; I can't access the field_value of any other field, it throws a DeserializationError mentioning the status_type field even though I'm not accesing it. See the screenshot where I reproduce and mimick what the field_value function does inside an ipdb debugger:

image

However, this only happens when I'm tracking said field. If I remove it from the FieldHistoryTracker list, I can access all the other fields without a problem.

I'm using the latest version of Django 1.9.x along with the latest Python 2.7.x on Linux.

Just typing out some notes to help me wrap my head around this.

So this seems to happen under these circumstances:

Model A tracks a field that is a foreign key to Model B. And Model B has a many-to-many relationship with Model C.

Hey @ljle, I have a branch to test this issue.

Take a look at the test here. It actually passes just fine. :\

I noticed your ManyToMany field has a through argument. I'm wondering if that could be causing the issue. Could you post your OSD_StatusMotive model in this issue so I can see what it is doing?

ljle commented

Hey @grantmcconnaughey, I checked out to the branch, ran the tests and it is indeed passing.

Here is the model:

class OSD_StatusMotive(models.Model):
    status_type = models.ForeignKey(OSD_Status)
    motive = models.ForeignKey(OSD_Motive)

    class Meta:
        db_table = 'osd_statusmotive'

    def __unicode__(self):
        return "(estado: %s) - (motivo: %s) - %s" % (self.status_type.name,
                                                     self.motive,
                                                     self.motive.entity.model_name)

Btw, I noticed that Django>=1.7 is missing from requirements-test.txt.

Ugh, even with a through it works just fine. Here are the models...

class Window(models.Model):
    pass


class Building(models.Model):
    windows = models.ManyToManyField(Window, through='BuildingWindows')


class BuildingWindows(models.Model):
    building = models.ForeignKey(Building)
    window = models.ForeignKey(Window)


class Restaurant(models.Model):
    building = models.ForeignKey(Building)

    field_history = FieldHistoryTracker(['building'])

and here is the test:

def test_field_history_works_with_foreign_key_that_has_many_to_many(self):
    window = Window.objects.create()
    building = Building.objects.create()
    BuildingWindows.objects.create(building=building, window=window)
    restaurant = Restaurant.objects.create(building=building)

    self.assertEqual(FieldHistory.objects.count(), 1)
    history = FieldHistory.objects.get()
    self.assertEqual(history.object, restaurant)
    self.assertEqual(history.field_name, 'building')
    self.assertEqual(history.field_value, building)
    self.assertIsNotNone(history.date_created)

Yes, I think I left Django out of requirements because it gets installed in .travis.yml. There was some reason I had to do it that way. You'll have to pip install Django yourself.

The only thing left I can think of is to run your tests again and spit out the JSON in serialized_data. Do that for any of the FieldHistory objects where field_value fails. I'd like to see what that is, because maybe that got messed up somehow. Could you add that to this issue when you get a chance, please?

ljle commented

Sure. I ran your test and it passes. I added a restaurant_type and a name field to the Restaurant model and started tracking them:

class RestaurantType(models.Model):
    name = models.CharField(max_length=50)

class Restaurant(models.Model):
    name = models.CharField(max_length=50)
    restaurant_type = models.ForeignKey(RestaurantType)
    building = models.ForeignKey(Building)

    field_history = FieldHistoryTracker(['building', 'restaurant_type', 'name'])

When I access serialized_data for each FieldHistory object respectively:

[{"model": "tests.restaurant", "pk": 1, "fields": {"restaurant_type": 1}}]
[{"model": "tests.restaurant", "pk": 1, "fields": {"name": "McDonalds"}}]

When I access field_value, for both:

*** DeserializationError: Restaurant has no building.

Thanks, @ljle. Now that we have a failing test I can start to dig into why it is happening. Let me know if you come up with anything, and thanks a lot for reporting this!

ljle commented

No problem, @grantmcconnaughey. If I come up with anything I'll update the issue.