Derive asset access field from asset blob

[jjnesbitt]

Closes #1997

Previously, this field was left unaltered, which meant it assumed whatever value was provided during creation through the API. This meant a default of dandi:OpenAccess most of the time, but it could have actually had any value. This has led to almost all of the currently embargoed assets to mistakenly display an access value of dandi:OpenAccess.

This PR changes this behavior by instead using the asset blob embargoed field to determine the value of 'access'.

TODO

• migration

[mvandenburgh]

The AuditRecord model also stores a copy of Asset.metadata, which means that any asset audit records created up until this PR is merged will have an access field, while future ones won't.

This also made me realize we're just storing the metadata, not full_metadata in the asset audit record; i.e. all the "computed" fields are being left out of the audit records. I wonder if we should do the latter (@waxlamp).

[jjnesbitt]

The AuditRecord model also stores a copy of Asset.metadata, which means that any asset audit records created up until this PR is merged will have an access field, while future ones won't.

Since the access field for assets has always been superfluous, I think it would be appropriate to also purge it from the existing audit records. What do you think @waxlamp?

[waxlamp]

This also made me realize we're just storing the metadata, not full_metadata in the asset audit record; i.e. all the "computed" fields are being left out of the audit records. I wonder if we should do the latter (@waxlamp).

I'm open to this. If the full metadata can be derived from the "small" metadata, I think leaving it as is would be ok too. But let's discuss next week.

[waxlamp]

The AuditRecord model also stores a copy of Asset.metadata, which means that any asset audit records created up until this PR is merged will have an access field, while future ones won't.

Since the access field for assets has always been superfluous, I think it would be appropriate to also purge it from the existing audit records. What do you think @waxlamp?

I think that would be ok.

[mvandenburgh]

LGTM, just need the migration for the audit record table 👍

[yarikoptic]

PR gained approval 3 weeks ago. @jjnesbitt is there anything else to be done here or could be just merged?

[waxlamp]

PR gained approval 3 weeks ago. @jjnesbitt is there anything else to be done here or could be just merged?

This will require a migration; as such I'll move it back to draft status.

[yarikoptic]

I have added to the PR description the TODO "section" with a checkbox for migration. Would be great if similar approach is adopted for other PRs to make it clear(er) on what outstanding TODOs left.

[dandibot]

🚀 PR was released in v0.3.106 🚀