-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[HOLD] Fix validation of description values including title #681
base: main
Are you sure you want to change the base?
Conversation
@@ -62,22 +62,30 @@ def validate_values_for_blanks(hash, path) | |||
end | |||
|
|||
def validate_values_for_multiples(hash, path) | |||
return unless hash.count { |key, value| %i[value groupedValue structuredValue parallelValue].include?(key) && value.present? } > 1 | |||
return unless hash.count { |key, value| %w[value groupedValue structuredValue parallelValue].include?(key) && value.present? } > 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
keys are strings, rather than symbols.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we know these keys are strings?
The entrypoint to the DescriptionValuesValidator
class is the Validator
class, which converts the attributes to be validated into a HashWithIndifferentAccess
instance.
The HashWithIndifferentAccess
class maps symbol keys to string keys in its writing interface. Thus:
[y]ou are guaranteed that the key is returned as a string
(see https://api.rubyonrails.org/classes/ActiveSupport/HashWithIndifferentAccess.html)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for explaining/documenting this in detail!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yay!
Why was this change made? 🤔
HOLD because data in prod needs remediation.
Follow-on to #675 (see sul-dlss/argo#4279 and also #424) to make sure that ValidationError is raised in several invalid title and other DescriptiveValue situations.
How was this change tested?
Unit. When running a dor-services-app instance locally in docker with the cocina-models branch, error is raised by DSA when an invalid update sent from Argo spreadsheet upload.
Ran
bin/validate-cocina
. There are 244 invalid druids in prod, none in qa or stage. Gave @arcadiafalcone the list of problem druids for remediation in sul-dlss/argo#4279. They are all for"Multiple value, groupedValue, structuredValue, and parallelValue in description"
errors.