search

sul-dlss / cocina-models

Cocina repository data model (implemented in Ruby)
https://sul-dlss.github.io/cocina-models/
3 stars 0 forks source link

Add validator ensuring no more than one refresh catalog link is present #389

Closed mjgiarlo closed 2 years ago

mjgiarlo commented 2 years ago

Fixes #379

Why was this change made? 🤔

To continue adding smarter semantic validation to cocina-models, and this is the next step. Having a single catalog link with the refresh property set to true makes it clear which catalog link ("catkey") to use for refreshing metadata.

How was this change tested? 🤨

CI

arcadiafalcone commented 2 years ago

@mjgiarlo @justinlittman One exception is objects with the tag "Warning : Do Not Refresh" - these are digital serials that should be linked to a catkey but should not be refreshed from source.

mjgiarlo commented 2 years ago

@arcadiafalcone 💬

@mjgiarlo @justinlittman One exception is objects with the tag "Warning : Do Not Refresh" - these are digital serials that should be linked to a catkey but should not be refreshed from source.

so for those objects, should we also throw a cocina-models error if any catkeys have refresh set to true? I'm not sure this approach is going to work, as the tags are not stored in the cocina model. Perhaps that check should happen in DSA proper, where the admin tags live? I don't see any code around this currently. Is this a thing we should already be handling? I'm thinking this might could be a high-priority new DSA issue. cc: @justinlittman @jcoyne

mjgiarlo commented 2 years ago

This will be accommodated in https://github.com/sul-dlss/dor-services-app/issues/3767

arcadiafalcone commented 2 years ago

@mjgiarlo @justinlittman @jcoyne The tag is a stopgap measure for indicating those records until a more robust approach (the refresh flag) was available. At some point those records should have refresh set to false. Whether that happens in the mapping or as a one-time remediation after migration doesn't matter so much as making sure those records are not left defaulted to refresh=true.