django-dirtyfields icon indicating copy to clipboard operation
django-dirtyfields copied to clipboard

Support for the DateTime field where we have set auto_now as True

Open pradeep-sukhwani opened this issue 3 years ago • 3 comments

Description

Currently the django-dirtyfields detects the fields which have been updated. This changes is for the fields which gets updated once we save the object. Fields such as last_updated = models.DateTime(auto_now=True) doesn't get detected. So to make sure that such fields get detected, we have added a check and updated the field's value. We have also made sure that it consider the timezone if the support for timezone is enabled.

Test case

  • [x] New test cases

pradeep-sukhwani avatar Feb 09 '22 08:02 pradeep-sukhwani

Hi thanks for the PR,

Is there a particular reason or example code for why you need this change?

LincolnPuzey avatar Feb 28 '22 10:02 LincolnPuzey

@LincolnPuzey Appreciate your response

The main reason is we've been evaluating using django-dirtyfields instead of using .save(update_fields=[...]). The main reasons for this would be

  • We only "update" the fields that have been touched, hence there's a performance benefit.
  • It would also avoid accidentally skipping some fields in the update_fields parameter after making a change; and this is hard to catch in pull requests.

It seems that while django-dirtyfields does update all the touched fields. We do have timestamp fields like last_updated (auto_now=True, which get updated by django itself). They also need to updated alongside because otherwise the data in the timestamp field is now stale, as opposed to if we'd just called .save()

Perhaps this behaviour of updating timestamp fields also can be made configurable?

@pssukhwani Feel free to add if i left out anything or stated something incorrectly.

sidmitra avatar Feb 28 '22 13:02 sidmitra

Hi @LincolnPuzey, Thanks for your response.

@sidmitra thanks for the detailed overview. Adding to your point, the last commit also make sure that we consider the timezone if it is enabled.

pradeep-sukhwani avatar Feb 28 '22 14:02 pradeep-sukhwani