Allow for multiple extents

[marlo-longley]

I would love someone to verify that I interpreted the intent written in this ticket correctly:
Closes #989

I would also love feedback on whether we need to repeat this logic for both the _tesim and ssm fields.

Also, since there was no discussion on the ticket, I assume this is non-controversial and desired by the group.

[seanaery]

@marlo-longley This is great progress and it moves this issue closer to fixed. A few thoughts...

Multiple extents

Here's what I'd expect the result to become, i.e., rather than a single-item array, the doc should have multiple extents and only the first should concatenate the values from within the same <physdesc>:

expect(result['extent_ssm']).to equal_array_ignoring_whitespace ['1.25 Linear Feet (1 volume)', '1 document case', '16 DVDRs']

We have a related issue to tackle in #997 where we should be showing a badge for each extent, i.e., multiple extents should be OK as long as we fix that other issue.

Removing and

With multiple values the and is coming from the default separator_options but can be overridden like has been done for several other fields in the catalog_controller.rb template. I.e.,

separator_options: {
      words_connector: '<br/>',
      two_words_connector: '<br/>',
      last_word_connector: '<br/>'
    }

Repeating for extent_tesim

In the traject config you can just copy the value that was captured for extent_ssm into extent_tesim without repeating the logic. I.e.,

to_field "extent_tesim" do |_record, accumulator, context|
  accumulator.concat context.output_hash["extent_ssm"] || []
end

Needed on components and collections alike

Since <extent> can be present in the top-level collection and in individual components, we'll need this tweak to also be present in ead2_component_config.rb; See: https://github.com/projectblacklight/arclight/blob/main/lib/arclight/traject/ead2_component_config.rb#L158-L159

[seanaery]

I definitely welcome additional feedback from @mmmmcode , @gwiedeman and anyone else who can advise on the EAD nuances around extents in #989 :-)

[marlo-longley]

Awesome thanks so much @seanaery !

[marlo-longley]

HI @seanaery -- I believe I addressed all your comments. I also added code to close #997, so now this PR addresses 2 issues. I would love feedback but here is what it's looking like:

The one question I had was about the failing test. I'm not sure if it's a requirement to use the extent field for this test as the size_accessor, and therefore whether it's a concern that this PR alters the data format.

arclight/spec/models/arclight/document_downloads_spec.rb

Line 13 in 0d73b63

extent_ssm: ['42GB'], # This field does not typically hold file size, but for test purposes..

[mmmmcode]

@seanaery @marlo-longley One thing I wonder about is the parentheses in the container summary text (extent altrender="carrier")- are those something we want to supply if they aren't present? Or is that something that should be addressed with local customizations?

[marlo-longley]

@mmmmcode Thanks for taking a look! In these fixtures the words in parentheses are provided directly from the XML, as if a cataloger had entered it that way. So I wasn't thinking we should alter that. I want to make sure I'm understanding you, though!

[mmmmcode]

From my perspective, the () provided by the cataloger looks like style baked into content (like using <emph> to bold series titles), which I'm always a fan of eliminating. But that might just be me ;)

…

On Thu, Dec 7, 2023 at 6:05 PM Marlo Longley ***@***.***> wrote: @mmmmcode <https://github.com/mmmmcode> Thanks for taking a look! In these fixtures the words in parentheses are provided directly from the XML, as if a cataloger had entered it that way. So I wasn't thinking we should alter that. I want to make sure I'm understanding you, though! — Reply to this email directly, view it on GitHub <#1446 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BARNDYMJIWC5HNQD4HL3MZ3YIJDUDAVCNFSM6AAAAABALJ5NLGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBWGI2DCNZRHA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[marlo-longley]

Thanks for explaining @mmmmcode -- I could see this for sure but I'm thinking it is a change that is not directly related to allowing for multiple extents, or properly grouping extents, as the tickets lay out. If we want to change the formatting of text in extents, my take is that we should open a new ticket and discuss there, rather than lump new functionality into this particular PR.

[seanaery]

@marlo-longley That all looks great to me and I agree about not trying to fold in parentheses formatting. As for the failing test, I'm unfamiliar with that downloads feature (we don't use it in our UI) but I wonder if you could just stub in a value there for a made-up accessor like document.finding_aid_size rather than tie it to the actual extent_ssm field and its accessor. https://github.com/projectblacklight/arclight/blob/main/spec/fixtures/config/downloads.yml#L19