django-field-history icon indicating copy to clipboard operation
django-field-history copied to clipboard

ManyToManyField inside tracked field breaks field_value function

Open ljle opened this issue 8 years ago • 9 comments

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.

ljle avatar Jul 03 '16 01:07 ljle

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.

grantmcconnaughey avatar Jul 09 '16 16:07 grantmcconnaughey

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?

grantmcconnaughey avatar Jul 09 '16 17:07 grantmcconnaughey

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.

ljle avatar Jul 09 '16 19:07 ljle

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)

grantmcconnaughey avatar Jul 09 '16 19:07 grantmcconnaughey

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.

grantmcconnaughey avatar Jul 09 '16 19:07 grantmcconnaughey

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?

grantmcconnaughey avatar Jul 09 '16 19:07 grantmcconnaughey

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.

ljle avatar Jul 13 '16 03:07 ljle

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!

grantmcconnaughey avatar Jul 13 '16 13:07 grantmcconnaughey

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

ljle avatar Jul 15 '16 01:07 ljle