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

resolve_computed_field does not consider update_fields from signal pre_save #16

Open
URUKOVV opened this issue Jun 18, 2020 · 2 comments

Comments

@URUKOVV
Copy link

URUKOVV commented Jun 18, 2020

Would be better to consider that)

@brechin
Copy link
Owner

brechin commented Jun 18, 2020

That's a good idea! To clarify what you're suggesting:

If update_fields is None, or the computed field name appears, then compute the value and save to the database.
If update_fields is set to some other set of fields not including the computed field, then don't save it to the database.

So, a contrived example to show a possibly confusing side effect of this:

class Foo(models.Model):
    value = models.IntegerField()
    doubled = ComputedIntegerField(compute_from='compute_val')

    @property
    def compute_val(self):
        return 2 * self.value


my_foo = Foo.objects.create(value=4)
# (db should have value=4, doubled=8)
my_foo.value = 5
# my_foo.doubled == 10
my_foo.save(update_fields=['value'])
# (db has value=5, doubled=8)  <--- this is what might be surprising

I can where there might be situations where this is desired. Is this the behavior that you are after?

@URUKOVV
Copy link
Author

URUKOVV commented Jun 18, 2020

The situation I encountered is this: for a new computed_field i should implement 'compute_from' function in the migration file for every computed fields implemented before, could not just import these functions couse of migration and code compatability, and thats disgusting. U just showed the other side effect, thank you!

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