Fix propagation of embargoed date from dandisets to assets

[h-mayorquin]

Fixes #2697

[yarikoptic]

@h-mayorquin I think it broke tests -- do they pass for you locally?

copilot's take on it

The failing job 60823788006 encountered a KeyError: 'embargoedUntil' when serializing the metadata field in AssetDetailSerializer. This typically means the code attempted to access metadata['access'][0]['embargoedUntil'] but the key 'embargoedUntil' was not present.

Solution:
You need to ensure that the key exists before accessing it, or handle cases where it may be missing.

Code Fix

Example for serializer or any place where the field is accessed:

embargoed_until = None
access = first_version.metadata.get('access', [])
if access and isinstance(access[0], dict):
    embargoed_until = access[0].get('embargoedUntil')
# Use embargoed_until (could be None if not present)

If you need the value to always be present, add a default:

embargoed_until = access[0].get('embargoedUntil', 'default-value')

Alternatively, handle the missing field in your serializer:

• Use Django REST Framework’s SerializerMethodField or override to_representation to catch and handle KeyError.

Additional suggestion

• Check the tests which failed (such as test_asset_create_embargo, test_asset_rest_update_embargo, etc.) and ensure your test setup/fixtures include the expected field in the metadata, or update the tests to handle assets where 'embargoedUntil' may not exist.

---

Summary:
Guard access to 'embargoedUntil' with .get() or explicit checks, and update serializer code accordingly. This should resolve the KeyError and prevent 500 errors on serialization.

[waxlamp]

I understand that this is a small-seeming change that fixes immediate problems, but my alarm bells are ringing at the idea of blindly copying embargo-level metadata from an associated version to the asset itself.

@jjnesbitt, can you assess the longer-term danger here, in case we need to merge this fix in the short term? If we do that, we need to create an issue to look at the deeper problem here and fix at that level.

[waxlamp]

Will this always be correct? We may one day have a model where some versions are embargoed while others are not (although, I admit, this seems unlikely). In a case like that, this would become wrong again.

[jjnesbitt]

I think for the time being the assumption that embargo is a blocker for publishing (that is, embargoed dandisets can only have a draft version) is okay. If that changes, many more places than this will need to be changed anyway.

[waxlamp]

As you pointed out in the associated issue

That said, I am not sure why we have embargoedUntil at the asset level at all since this is only a dandiset parameter.

A better fix would be to take a deeper look at "denormalizing" embargo metadata onto assets as well as Dandisets.