Derive asset access field from asset blob
#2010
Conversation
Determine if asset is open access or embargoed based on if the asset blob is embargoed.
| # Use the postgres jsonb '-' operator to delete the 'access' field from metadata | ||
| Asset.objects.filter(metadata__access__isnull=False).update( | ||
| metadata=RawSQL("metadata - 'access'", []) | ||
| ) |
There was a problem hiding this comment.
Just a note, I tested this locally and it works as expected. Here is the generated SQL:
UPDATE "api_asset"
SET "metadata" = (metadata - 'access')
WHERE ("api_asset"."metadata" -> 'access') IS NOT NULL
There was a problem hiding this comment.
Why not use Django ORM style for this? i.e., replace Asset.metadata with the access-less copy, followed by .save()?
There was a problem hiding this comment.
We could do that, but that means we'd be doing as many .save calls are there are assets, which seems unnecessarily burdensome, as well as the potential for a race condition (if someone were to update the asset at the same time, either their update may be lost, or this one could be). It would also probably take much longer.
|
The This also made me realize we're just storing the |
Since the |
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. |
I think that would be ok. |
Published assets actually MUST have this field present, and so removing it here would violate a check constraint
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:OpenAccessmost 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 anaccessvalue ofdandi:OpenAccess.This PR changes this behavior by instead using the asset blob
embargoedfield to determine the value of 'access'.