Add catalog links for Folio

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 catalog links for Folio #564

Closed mjgiarlo closed 1 year ago

mjgiarlo commented 1 year ago

Why was this change made? 🤔

Connects to #561

Includes:

Turn CatalogLink model into a union type
- Allows either Symphony-type catalog links ("catkeys" or "ckeys") or Folio-type catalog links ("HRIDs"). This split allows each type of catalog link to validate its own catalog string and catalogRecordId regex properties.
Discontinue allowing non-number characters in catkeys
- Tightens the regular expression governing what is a valid Symphony catalog record ID. Arcadia confirms this is the right value, and it is aligned with the regex for the new Folio catalog links (of the migrated-from-symphony variety). Additionally, I ran a report in DSA (https://github.com/sul-dlss/dor-services-app/pull/4357) confirming there are zero catkeys in QA, stage, and prod that contain non-number characters.
Regenerate all models.

How was this change tested? 🤨

CI, and tested in QA, stage, and prod envs on sdr-infra

justinlittman commented 1 year ago

Any chance we can split this up into something more manageable?

mjgiarlo commented 1 year ago

@justinlittman 💬

Any chance we can split this up into something more manageable?

For sure. My intent with this draft PR was primarily to get feedback on the new approach to re-using custom types. I should have been clearer about that. In the meantime, here's a start:

https://github.com/sul-dlss/cocina-models/pull/565

mjgiarlo commented 1 year ago

@ndushay 💬

I'm with JLitt - it's hard to figure out which file changes are about the approach you want us to vet for the ticket vs. which are about changes you made for other reasons. Can you please split this or redo this so there is a single focus PR for what you want us to check?

I've already split out three PRs with a fourth coming soon 🕺🏻

I'm not sure we gain a whole lot from what you did vs. just having a regex allow "a" "L" or "in" before the digits for Folio regexp. That's an @arcadiafalcone question, I think -- is it worth having the separate definitions in the API? Will we ever want to hone in on one group and need a way to do it with the API definitions vs. the regex?

I figure better safe than sorry, lest we say "OK" to a catalog link that looks like folio in one way and like symphony in another way.

mjgiarlo commented 1 year ago

@justinlittman Could you take another look at this? It's boiled down to just the Folio-related changes now.