Fix bug where ModificationTimestampField would not automatically update its value.

Review Request #13261 — Created Sept. 6, 2023 and submitted — Latest diff uploaded

Information

Djblets
release-4.x

Reviewers

As discussed in /r/13260, it was discovered that the
ModificationTimestampField had a bug where sometimes the field wouldn't
automatically update to a new value when it was supposed to. Review Board uses
this field for its "last updated" timestamp on review request and drafts, and
this timestamp sometimes wasn't being updated when we would save changes on a
review request or draft. Specifically, this would happen whenever we tried to
save changes on objects that were fetched through a model manager (i.e. objects
from Model.objects).

ModificationTimeStampField takes care to not overwrite manually set values by
tracking modification state in its __set__ method. If __set__ was called,
then we assume that the field value was manually set and we give the field a
modified state. But when an object is fetched through a manager, the
__set__ method for the field gets called when creating the object instance.
This makes the ModificationTimestampField think that the value has been
manually set, even though it hasn't been. Previously we were using a
first_set state to handle the case when __set__ is called during object
initialization, but this wasn't hooked up correctly and didn't actually work.
This change addresses the case by checking if the field value actually exists
in the object's attributes dictionary, and if not then we know that the field
is being set for the first time during object creation and its not being
manually set. We also add unit tests for this case.

  • Ran unit tests.
  • Ran unit tests (including my newly added ones) without the fix to
    ModificationTimestampField, saw that the cases where we fetch objects
    through a model manager and save those would not properly update the
    timestamp.
  • Used in a Review Board change.

Commits

Files