dandi / dandi-archive

DANDI API server and Web app
https://dandiarchive.org
13 stars 11 forks source link

Only update modified date from the API #458

Open dchiquito opened 3 years ago

dchiquito commented 3 years ago

Currently, the modified field is set to update on save, which means any change to a Dandiset/Version/Asset will update the modified field, including updating validation status or any other fields not directly related to content. This means that the modified date gets updated if, for example, an asset is revalidated.

We should only be updating the modified field when changes are made to the content of the model, or at least only when changes are made through the REST API.

Related issues (edit this list as more are found):

waxlamp commented 2 years ago

To me, it's a question of whether we should be using TimeStampedModel for these. It seems useful to have a modified time that is generic in that way that extension provides for, and then perhaps we should have more of a semantic modification field that tracks DANDI-specific actions. Or we could dump TimeStampedModel and just roll in our own semantic logic for tracking the mod time.

Thoughts, @dchiquito?

dchiquito commented 2 years ago

I agree we need to clarify what exactly modified means. TimeStampedModel gives us for free a modified that is updated whenever save() is called. If we want it to mean something more dandi specific, we need to define exactly what changes constitute a modification. Metadata definitely should, uploading/deleting assets should, but should changing ownership? publishing? validation completion?

Once we have that list we can rip out the save trigger for modified and only update it when appropriate.

waxlamp commented 2 years ago

Let's do a (small) design doc for this to capture all the appropriate events, send it around for review, and then do as you suggested.

danlamanna commented 2 years ago

I recently spoke to @brianhelba about this feature more generally. The tldr in the context of dandi would be that we should stop using autogenerated modified fields and start purposefully tracking when things are updated based on our business requirements.

So we need to know a couple of things: 1) What does Dandi need to precisely know "last updated" times for. Think of this less as a field in a database and more as a property of an entity. 2) When should the "last updated" times be modified? Only on user input? Or when performing a database migration, inserting a checksum that was deferred during upload, etc.

satra commented 2 years ago

for a dandiset, the last modified should be based on end user based changes (metadata modification, asset CRUD).

however, we would want some part of the system tracking changes made by the system as well. there is a separate issue on auditing. an audit logger should contain all actions made on assets/dandisets.

danlamanna commented 2 years ago

however, we would want some part of the system tracking changes made by the system as well. there is a separate issue on auditing. an audit logger should contain all actions made on assets/dandisets.

Sure - Cross linking #525 and #61.