Re: [Django] #35890: pre_save field in parent models are not called during update in update_or_create

[Django]

#35890: pre_save field in parent models are not called during update in
update_or_create
-+-
 Reporter:  Gagaro   |Owner:  (none)
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-
Comment (by Gagaro):

 Indeed, I see your point.

 I didn't know that `auto_now` was supposed to be deprecated. It isn't
 obvious from the
 
[https://docs.djangoproject.com/en/5.1/ref/models/fields/#django.db.models.DateField.auto_now
 documentation] either.
-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion visit 
https://groups.google.com/d/msgid/django-updates/01070193260a1051-5788482a-bd43-480d-9c34-c2c074dbcd64-00%40eu-central-1.amazonses.com.

Re: [Django] #35890: pre_save field in parent models are not called during update in update_or_create

[Django]

#35890: pre_save field in parent models are not called during update in
update_or_create
-+-
 Reporter:  Gagaro   |Owner:  (none)
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Simon Charette):

 * cc: Simon Charette, Florian Apolloner, Sarah Boyce (added)

Comment:

 I think we need someone else to chime in here to get a consensus before
 moving forward with either solution.

 The `Field.auto_now` pattern is a feature on its way out #22995 and as it
 doesn't play nicely with `update_fields` #22981 particularly in the
 context of `update_or_create` (#35014 and associated
 [https://forum.djangoproject.com/t/update-or-create-behavior/25944 forum
 discussion]).


 In your particular case, `Child.save(update_fields={...})`, won't update
 `"modified_at"` unless it's specified explicitly so your expectations are
 already broken for any direct or indirect (through third-party apps) usage
 of `save(update_fields)`; `update_or_create` is only a single instance of
 that.

 The changes made to handle local concrete fields in #32095
 ([https://github.com/django/django/pull/13526 original PR by Florian],
 [https://github.com/django/django/pull/16072 eventually merged changes by
 Sarah]) unfortunately it didn't cover this use case,
 [https://github.com/django/django/pull/13526#discussion_r503304168 even if
 it was pointed out as a point of ambiguity], as neither `Book` or
 `Publisher` are using MTI.

 > Why do you think it should be that way?

 From my perspective, given the intent #32095 was to avoid unnecessary
 field updates, performing MTI inherited tables updates when non of their
 fields are explicitly requested to be updated because they use a pattern
 meant to be deprecated is not the right approach.
-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion visit 
https://groups.google.com/d/msgid/django-updates/0107019325f01fce-16a0230a-8e67-4c12-9fbf-d7e7de449844-00%40eu-central-1.amazonses.com.

Re: [Django] #35890: pre_save field in parent models are not called during update in update_or_create

[Django]

#35890: pre_save field in parent models are not called during update in
update_or_create
-+-
 Reporter:  Gagaro   |Owner:  (none)
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-
Comment (by Gagaro):

 I had more thoughts about it, and from a user perspective, I think having
 some fields but not others updated would be an unexpected behavior. The
 fields on the parent model are parts of the child one, it is transparent
 when we try to access or change them on the instance. I'd expect them to
 be updated in the same way the child ones are.
-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion visit 
https://groups.google.com/d/msgid/django-updates/0107019305b7ad7a-eb9b9abb-4f99-4e2e-be12-cbdcc59b8855-00%40eu-central-1.amazonses.com.

Re: [Django] #35890: pre_save field in parent models are not called during update in update_or_create

[Django]

#35890: pre_save field in parent models are not called during update in
update_or_create
-+-
 Reporter:  Gagaro   |Owner:  (none)
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-
Comment (by Gagaro):

 Why do you think it should be that way?

 In our cases, we have an `auto_now` field on the parent model only, and we
 need it to be updated every time, even if there is only a single change in
 a field from a child model.
-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion visit 
https://groups.google.com/d/msgid/django-updates/01070193023e9178-77458725-4f1b-4990-aebc-22865d65394c-00%40eu-central-1.amazonses.com.

Re: [Django] #35890: pre_save field in parent models are not called during update in update_or_create

[Django]

#35890: pre_save field in parent models are not called during update in
update_or_create
-+-
 Reporter:  Gagaro   |Owner:  (none)
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Simon Charette):

 * needs_better_patch:  0 => 1
 * stage:  Unreviewed => Accepted

Comment:

 I see no reasons why this should only work for local concrete fields given
 `update_or_create` works with MTI.

 There's a nuance not captured in the patch though that I believe should be
 addressed at the same time. Only pre-save fields for the model associated
 with at-least-one update field should be augmented. What I mean by that is
 that given the following models

 {{{#!python
 class Location(models.Model):
 location_updated_at = models.DateTimeField(auto_now=True)
 location_street = models.TextField()

 class Restaurant(Location):
 restaurant_updated_at = models.DateTimeField(auto_now=True)
 menu = models.JSONField()
 }}}

 Then

 {{{#!python
 Restaurant.objects.update_or_create(
...,
defaults={"menu": ...}  # Only restaurant_updated_at should be included
 in `update_fields`
 )
 Restaurant.objects.update_or_create(
...,
defaults={"location_street": ...}  # Only location_updated_at should be
 included in `update_fields`
 )
 Restaurant.objects.update_or_create(
...,
defaults={"menu": ..., "location_street": ...}  # Both
 restaurant_updated_at and location_updated_at should be included in
 `update_fields`
 )
 }}}
-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion visit 
https://groups.google.com/d/msgid/django-updates/01070193023117ee-0b559ce1-82ee-4198-af5d-52eb755fd4c7-00%40eu-central-1.amazonses.com.

Re: [Django] #35890: pre_save field in parent models are not called during update in update_or_create

[Django]

#35890: pre_save field in parent models are not called during update in
update_or_create
-+-
 Reporter:  Gagaro   |Owner:  (none)
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Description changed by Gagaro:

Old description:

> With the following models:
>
> {{{
> class Parent(models.Model):
> modified_at = models.DateTimeField(auto_now=True)
>

> class Child(Parent):
> modified_at_child = models.DateTimeField(auto_now=True)
> }}}
>
> When calling `update_or_create`, the pre_save of the `modified_at` field
> is not called, and the field is not updated:
>

> {{{
> # Create
>
> >>> instance, _ = Child.objects.update_or_create(pk=1)
> >>> instance.modified_at
> datetime.datetime(2024, 11, 6, 14, 1, 11, 52881,
> tzinfo=datetime.timezone.utc)
> >>> instance.modified_at_child
> datetime.datetime(2024, 11, 6, 14, 1, 11, 54363,
> tzinfo=datetime.timezone.utc)
>

> # Update
>
> >>> instance, _ = Child.objects.update_or_create(pk=1)
> >>> instance.modified_at
> datetime.datetime(2024, 11, 6, 14, 1, 11, 52881,
> tzinfo=datetime.timezone.utc)
> >>> instance.modified_at_child
> datetime.datetime(2024, 11, 6, 14, 1, 21, 517245,
> tzinfo=datetime.timezone.utc)
> }}}
>

> The regression seems to have happened in
> https://github.com/django/django/commit/6cc0f22a73970dd7c0d29d4d8d2ff9e1cc862b30
>
> Using `self.model._meta.concrete_fields` instead of
> `self.model._meta.local_concrete_fields` seems to solve the issue, but I
> don't know the internal API well enough to understand all of the
> implications.
>
> I'm also preparing a PR with a unit test and the fix, I'll update this
> ticket when it will be ready.

New description:

 With the following models:

 {{{
 class Parent(models.Model):
 modified_at = models.DateTimeField(auto_now=True)


 class Child(Parent):
 modified_at_child = models.DateTimeField(auto_now=True)
 }}}

 When calling `update_or_create`, the pre_save of the `modified_at` field
 is not called, and the field is not updated:


 {{{
 # Create

 >>> instance, _ = Child.objects.update_or_create(pk=1)
 >>> instance.modified_at
 datetime.datetime(2024, 11, 6, 14, 1, 11, 52881,
 tzinfo=datetime.timezone.utc)
 >>> instance.modified_at_child
 datetime.datetime(2024, 11, 6, 14, 1, 11, 54363,
 tzinfo=datetime.timezone.utc)


 # Update

 >>> instance, _ = Child.objects.update_or_create(pk=1)
 >>> instance.modified_at
 datetime.datetime(2024, 11, 6, 14, 1, 11, 52881,
 tzinfo=datetime.timezone.utc)
 >>> instance.modified_at_child
 datetime.datetime(2024, 11, 6, 14, 1, 21, 517245,
 tzinfo=datetime.timezone.utc)
 }}}


 The regression seems to have happened in
 
https://github.com/django/django/commit/6cc0f22a73970dd7c0d29d4d8d2ff9e1cc862b30

 Using `self.model._meta.concrete_fields` instead of
 `self.model._meta.local_concrete_fields` seems to solve the issue, but I
 don't know the internal API well enough to understand all of the
 implications.

 PR : https://github.com/django/django/pull/18777

--
-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion visit 
https://groups.google.com/d/msgid/django-updates/0107019301e88c2c-37bdfda2-6926-4394-85b0-c1e675059913-00%40eu-central-1.amazonses.com.

Re: [Django] #35890: pre_save field in parent models are not called during update in update_or_create

[Django]

#35890: pre_save field in parent models are not called during update in
update_or_create
-+-
 Reporter:  Gagaro   |Owner:  (none)
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Gagaro):

 * has_patch:  0 => 1

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion visit 
https://groups.google.com/d/msgid/django-updates/0107019301e8425d-b587781f-9f25-4097-ad0d-08c8645f5333-00%40eu-central-1.amazonses.com.