Validate for title.structuredValue.value having a type #675
Conversation
| @@ -64,6 +67,14 @@ def validate_values_for_multiples(hash, path) | |||
| error_paths_multiple << path_to_s(path) | |||
| end | |||
|
|
|||
| def validate_title_type(hash, path) | |||
| # only apply to title.structuredValue with value | |||
| return unless hash[:value] && path.include?(:title) && path.include?(:structuredValue) | |||
There was a problem hiding this comment.
Wondering if this might match too broadly. Can you check for "title" and "structuredValue" in particular locations in the path?
There was a problem hiding this comment.
Thanks, I made some updates to this, also as part of checking relatedResource titles. There may be some variations of cocina that I'm not thinking of, however.
lwrubel
changed the title
|
@justinlittman if you happen to have a chance before departing and wanted to double-check this work before merging |
lwrubel
force-pushed
the
title-value-type-validator
branch
2 times, most recently
from
32f2602
to
3ff8766
Compare
|
@peetucket OK if I go ahead and merge this since I don't see any further notes from @justinlittman? |
| end | ||
|
|
||
| def related_resource_title?(path) | ||
| path.first == :relatedResource && path.third == :title |
There was a problem hiding this comment.
what is second? structuredValue or parallelValue or something? It's theoretically possible to have structuredValues inside parallelValues ...
There was a problem hiding this comment.
Is there an easy way to model that scenario in another spec and see what happens? Do you think that should block this fix?
There was a problem hiding this comment.
When I talked this through with Justin, we went down the path of keeping this in structuredValue only. Not further nesting or other situations.
The path it's looking for would be [:title, 0, :structuredValue, 0] in order to validate the presence of a type in the hash. (could be a different value than 0 of course)
Is there a way you'd recommend we do this? I don't doubt there's something I'm not understanding here.
There was a problem hiding this comment.
@ndushay and I took a look at this and I'll add in the case to handle title.parallelValue.structuredValue. @arcadiafalcone says: this would likely be coming from a MARC record, so with Google Books it's not that uncommon.
There was a problem hiding this comment.
@ndushay see added specs to cover title.parallelValue.structuredValue and relatedResource.title.parallelValue.structuredValue situations. Because the logic is to check for structuredValue in the path, as a descendant of title but not necessarily an immediate descendant, there is room for a parallelValue to exist in the path. I think this is OK for titles?
I also updated some comments to improve clarity, based on our conversation.
lwrubel
force-pushed
the
title-value-type-validator
branch
from
107b42d
to
637c82f
Compare
lwrubel
force-pushed
the
title-value-type-validator
branch
from
637c82f
to
e3b429a
Compare
Why was this change made? 🤔
Validator to address sul-dlss/argo#4279. This goes down one level of nesting (
title.structuredValue) and also validates one level ofrelatedResource.title.structuredValue.How was this change tested? 🤨
Unit. Ran
bin/validate-cocinaon QA, stage, and prod and there were no existing objects with this problem.⚡ ⚠ If this change has cross service impact, run integration tests and/or test in [stage|qa] environment, in addition to specs. ⚡