Skip to content

fix: restore value_map for empty-element preservation across metadata classes - #56

Merged
ronaldtse merged 1 commit into
mainfrom
fix/metadata-empty-element-preservation
Jul 27, 2026
Merged

fix: restore value_map for empty-element preservation across metadata classes#56
ronaldtse merged 1 commit into
mainfrom
fix/metadata-empty-element-preservation

Conversation

@ronaldtse

Copy link
Copy Markdown
Contributor

Summary

Validating BUGREPORT.metadata-iso-collection-types.md against current main (sts 0.6.10) revealed a regression from PR #55: the value_map: { to: { empty: :empty } } workaround that preserves empty <comm-ref/> and <secretariat/> elements was applied inconsistently across the four NisoSts metadata classes.

Validation results (before this PR):

Class comm-ref secretariat
MetadataIso preserved dropped
MetadataStd preserved preserved
RegMeta dropped dropped
NatMeta dropped dropped

Fix

Restored value_map: { to: { empty: :empty } } uniformly:

  • lib/sts/niso_sts/metadata_iso.rb: secretariat (added)
  • lib/sts/niso_sts/reg_meta.rb: comm-ref, secretariat (both added)
  • lib/sts/niso_sts/nat_meta.rb: comm-ref, secretariat (both added)

Regression spec

spec/niso_sts/metadata_cardinality_spec.rb — 4 new specs (one per metadata class) asserting both <comm-ref/> and <secretariat/> survive a parse → serialise round-trip. The specs would have failed the original PR #55 — caught during this validation.

BUGREPORT validation verdict

The original BUGREPORT claims are fully addressed by PR #55 + this follow-up:

  • 11 repeatable attributes are now collections (or singular-by-design for the empty-element trade-off on comm_ref/secretariat)
  • 5 schema-singular attributes (doc_ident, doc_ref, page_count, pub_date, std_ident) stay singular
  • Both reproduction cases from the BUGREPORT (meta.std_ref = [sr1, sr2] and meta.title_wrap = [tw1, tw2]) now serialise distinct entries
  • Empty <comm-ref/> and <secretariat/> round-trip cleanly through all four metadata classes

Test plan

  • bundle exec rspec — 5202 examples, 0 failures (+4 cross-class regression)
  • bundle exec rubocop — clean (537 files)
  • Manual verification: <iso-meta><comm-ref/><secretariat/></iso-meta> round-trips for all 4 metadata classes
  • CI green

… classes

PR #55 fixed the cardinality bug in NisoSts metadata classes (MetadataIso,
MetadataStd, RegMeta, NatMeta) but applied the value_map workaround
inconsistently:

- MetadataIso: comm-ref had value_map, secretariat did NOT (regression)
- MetadataStd: both had value_map (correct)
- RegMeta: neither had value_map (regression)
- NatMeta: neither had value_map (regression)

The cardinality work in PR #55 kept comm_ref and secretariat as singular
attributes (sacrificing schema-conformance) to preserve empty elements
in real fixtures via value_map: { to: { empty: :empty } }. The trade-off
was only fully applied to MetadataStd; three of four metadata classes
silently dropped empty <comm-ref/> and <secretariat/> elements on
round-trip.

This restores the value_map on all four classes uniformly:

- lib/sts/niso_sts/metadata_iso.rb: secretariat (added)
- lib/sts/niso_sts/reg_meta.rb: comm-ref, secretariat (both added)
- lib/sts/niso_sts/nat_meta.rb: comm-ref, secretariat (both added)

Adds a regression spec covering all four classes — discovered by the
round-trip audit when validating BUGREPORT.metadata-iso-collection-types.md.

Verification: with all four classes preserving both empty elements, the
following round-trip works:

  <iso-meta><comm-ref/><secretariat/></iso-meta>
  -> MetadataIso.from_xml(...).to_xml
  -> <iso-meta><comm-ref/><secretariat/></iso-meta>

5202 examples, 0 failures (was 5198; +4 new cross-class regression
specs). Rubocop clean.
@ronaldtse
ronaldtse merged commit ea027e0 into main Jul 27, 2026
14 checks passed
@ronaldtse
ronaldtse deleted the fix/metadata-empty-element-preservation branch July 27, 2026 23:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant