fix: assetId not unique

[buckhalt]

Some protocols were failing to import because of a duplicate protocol issue. This was due to the way we use the assetManifest key as the assetId. We cannot actually assume that the assetManifest key is unique given that we allow protocols that are copies of one another if they have made changes.

This PR makes the assetId truly unique by appending a cuid to the key.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
fresco	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Mar 26, 2024 3:27pm

[jthrilly]

This is an interesting one. I think this hinges on one question: are "forked" assets, which are identical to one another in terms of content, and which share the same UUID, different assets? The way you fixed this assumes that they are, but my view is that they aren't. The asset UUID is generated by Architect when an asset is created. It can't be changed. and it has pretty strong likelihood of global uniqueness. As always, a UUID intends for a record to be unique within a system first and foremost. and it clearly passes that test.

From that perspective, the fact that duplicated protocols have assets that share a UUID is actually a feature, not a bug. So given that, the real issue here is that we are failing on insertion when an asset with a UID already exists, rather than simply linking to it and saving ourself from creating a new db record and S3 bucket item.

There are two obvious problems with doing this, though:

1. What is there is a natural collision? This seems extremely unlikely unless someone manually authored a protocol.
2. What happens when deleting protocols - how do we know if we can safely delete any media assets? This would require checking to see if an asset is linked to any other Protocol records at delete time.

So either way we implement this we have a drawback. With your approach we are being space inefficient but we can track asset deletion. With my approach we are (I believe) maintaining the correct behaviour of the system, but we add a lot of complexity to the upload system (which has to work very differently if we can't just assume that assets can be uploaded right after validation completes) and the delete protocol functionality.

Aside from that, there's one reason I don't like your fix specifically though: we are taking a UUID that we have already generated, but now know not to be unique, and we are just adding on some more uniqueness to the end. Why bother using it at all? Why not have our own unique identifier for this system, and store the current UUID in a different column (if we arent already) with more relaxed constraints?

[buckhalt]

Refactored the protocol upload and deletion functionality to allow protocols with assets that already exist to be uploaded.

This change involves:

• Creating a many-to-many relationship between Protocol and Asset
• Protocol import: checking if assets already exist, and if so, connecting them to the protocol. If asset is not yet in the database, create a database record and S3 bucket item.
• Protocol deletion: select assets that are ONLY associated with the protocols to be deleted before deleting assets. If an asset is used by a protocol not being deleted, leave it in the database.

[mrkarimoff]

That looks great! Took me a little longer to understand, probably because I haven't coded in Fresco for a while 😄. I think we are handling the deleting process much efficient now. Anyway, I didn't spot any issue here, good work Caden