csv upload: do not import contributor roles without names or form source or type without a form value #4075
Conversation
ndushay
changed the title
ndushay
force-pushed
the
ignore-meaningless-metadata-imports
branch
3 times, most recently
from
faf379f
to
7dddfe4
Compare
ndushay
changed the title
ndushay
changed the title
| }] | ||
| }, | ||
| "purl": "https://sul-purl-stage.stanford.edu/bb041bm1345" | ||
| "title": [{ |
There was a problem hiding this comment.
this part is just formatting (how did we end up with so smay spaces??)
| end | ||
| end | ||
|
|
||
| context "with form property" do |
There was a problem hiding this comment.
the form tests start here.
| def remove_nested_attributes_without_value(compacted_params_hash) | ||
| # event can have contributors, geographic can have form | ||
| [:relatedResource, :event, :geographic].each do |parent_property| | ||
| next if compacted_params_hash[parent_property].blank? |
There was a problem hiding this comment.
Are we protecting against nils or empty arrays or both or something else here?
There was a problem hiding this comment.
we only care to process if there is a (parent_property) present in the hash. It could also be worded:
next unless compacted_params_hash[parent_property].present?
and I had it that way, but it seemed more confusing.
Yes - we are protecting against nil and also empty arrays. It all depends on what columns are in the spreadsheet, at this point.
… source or type without form value
ndushay
force-pushed
the
ignore-meaningless-metadata-imports
branch
from
7dddfe4
to
1497c52
Compare
|
tagging @justinlittman because I just noticed there is a DescriptionValidator service in Argo. I think, per the tickets, I was not supposed to reject the csv, but just ignore these oddities ... but would welcome your review and wonder if this now working-code-wins solution is contrary to a design I didn't see. |
|
I defer to @andrewjbtw on desired behavior. |
HOLD for PO approval (Andrew)Why was this change made? 🤔
Fixes #4071
Fixes #4072
Contributors with roles but not name are basically invalid and require complex remediation (error when publishing).
Form data without a value (e.g. it just has source or type, but no value) also require complex remediation (prevents indexing)
I've created sul-dlss/cocina-models/issues/610 for cocina validation of contributors
How was this change tested? 🤨
Andrew reviewed it in stage; also specs.
⚡ ⚠ If this change has cross service impact (including writing to shared file systems or interacting with other SDR APIs), run integration tests and/or test in [stage|qa] environment, in addition to specs. ⚡
⚡ ⚠ If this change updates the Argo UI, run all integration tests that use Argo and create a PR on the integration test repo to fix anything this change breaks. ⚡
Does your change introduce accessibility violations? 🩺
Nope!
⚡ ⚠ Please ensure this change does not introduce accessibility violations (at the WCAG A or AA conformance levels); if it does, include a rationale. See the Infrastructure accessibility guide for more detail. ⚡