Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ManyToManyField inside tracked field breaks field_value function #10

Open
ljle opened this issue Jul 3, 2016 · 9 comments
Open

ManyToManyField inside tracked field breaks field_value function #10

ljle opened this issue Jul 3, 2016 · 9 comments

Comments

@ljle
Copy link

ljle commented Jul 3, 2016

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.

@grantmcconnaughey
Copy link
Owner

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
Copy link
Owner

grantmcconnaughey commented Jul 9, 2016

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
Copy link
Author

ljle commented Jul 9, 2016

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.

@grantmcconnaughey
Copy link
Owner

grantmcconnaughey commented Jul 9, 2016

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
Copy link
Owner

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
Copy link
Owner

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
Copy link
Author

ljle commented Jul 13, 2016

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.

@grantmcconnaughey
Copy link
Owner

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
Copy link
Author

ljle commented Jul 15, 2016

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants