Update how imprint data is displayed on PURL

amyehodge commented 9 months ago

See analysis performed for this work in #150

- Do not display the value currently displayed with the label “Imprint” (default) or originInfo displayLabel (if specified) in purl
- MODS display in other systems should not be affected
- Each subelement of MODS originInfo should be displayed as an individual value, grouped under shared display labels
- The following subelements should use the display labels given in https://github.com/sul-dlss/mods_display/blob/b90eb78a1b187c1582e96f493d51931aceb31754/config/locales/en.yml#L23
     - <place> 
     - <publisher> 
     - <dateCreated> 
     - <dateCaptured> 
     - <dateValid> 
     - <dateModified> 
     - <copyrightDate> 
     - <issuance> 
     - <frequency> 
- <dateIssued> should use display label “Publication date” and be formatted as other dates
- <edition> should use display label “Edition”
- Subelements <agent> <displayDate> may be ignored as they are not in current local usage
- Subelement <dateOther> requires more analysis as to feasibility of including more specific info from element attributes
- MODS originInfo is a repeatable field and all values should be displayed

peetucket commented 9 months ago

Note: This ticket will require touching a few other codebases (stanford-mods and purl) Note that other display environments (e.g. Searchworks) should continue to work unaffected, so don't remove stuff from mods_display and stanford-mods to ensure backwards compatibility.

justinlittman commented 9 months ago

Can you please provide some examples?

arcadiafalcone commented 9 months ago

A simple example with place, publisher, dateIssued, and frequency within one originInfo: https://purl.stanford.edu/jn692cx1873

This one has place, publisher, dateIssued, and frequency with repeated values and multiple languages: https://purl.stanford.edu/xk755gc8675

This one also has edition and has multiple originInfo: https://purl.stanford.edu/fv880zr6174

arcadiafalcone commented 9 months ago

If you need comprehensive coverage of the possibilities, I can create some sample records.

justinlittman commented 9 months ago

It would be helpful to have some before and after examples.

arcadiafalcone commented 9 months ago

@justinlittman Examples in MODS or Cocina?

justinlittman commented 9 months ago

Since we're in access systems, I'm assuming this is about MODS mapping. (Unless I'm misunderstanding something.)

peetucket commented 9 months ago

I think based on the analysis in #150, there will be some mods mapping in stanford-mods, some parsing in mods_display and then updates to purl to take advantage of the new parsed metadata for display.

ndushay commented 9 months ago

originInfo elements retrievable individually from gems, and then displayed on purl pages

Imprint Datum	mods gem	stanford-mods gem	mods_display gem	purl page	exemplar druids
place	origin_info.place.placeTerm (?) origin_info.place (?)	Stanford::Mods::OriginInfo.place (multivalued)	t150-imprint branch	imprint-redux branch	https://purl.stanford.edu/jn692cx1873 https://purl.stanford.edu/xk755gc8675 (mult values)
publisher	origin_info.publisher	Stanford::Mods::Imprint.send(:publisher_vals_str)	"	"	https://purl.stanford.edu/jn692cx1873 https://purl.stanford.edu/xk755gc8675 (mult values)
dateCreated	originInfo.dateCreated	Stanford::Mods::Imprint.dates([:dateCreated])	t150-imprint branch	imprint-redux-branch	https://purl.stanford.edu/bb112zx3193
dateCaptured	originInfo.dateCaptured	Stanford::Mods::Imprint.dates([:dateCaptured])	[ ]	[ ]
dateValid	originInfo.dateValid	Stanford::Mods::Imprint.dates([:dateValid])	[ ]	[ ]
dateModified	originInfo.dateModified	Stanford::Mods::Imprint.dates([:dateModified])	[ ]	[ ]
copyrightDate	originInfo.copyrightDate	Stanford::Mods::Imprint.dates([:CopyrightDate])	[ ]	[ ]
issuance	originInfo.issuance		[ ]	[ ]
frequency	originInfo.frequency		[ ]	[ ]	https://purl.stanford.edu/jn692cx1873 https://purl.stanford.edu/xk755gc8675 (mult values)
dateIssued	originInfo.dateIssued	Stanford::Mods::Imprint.dates([:dateIssued])	[ ]	[ ]	https://purl.stanford.edu/jn692cx1873 https://purl.stanford.edu/xk755gc8675 (mult values)
edition	originInfo.edition	Stanford::Mods::Imprint.send(:edition_vals_str)	t150-imprint branch	imprint-redux branch	http://localhost:3000/fv880zr6174
(dateOther)	originInfo.dateOther		[ ]	[ ]
		pub_year_int, pub_year_sort_str, pub_year_display_str (note: EDTF can be ranges, we pick earliest)	[ ]	[ ]
		Stanford::Mods::Imprint for each originInfo node	[ ]	[ ]

Subelements <agent> <displayDate> may be ignored as they are not in current local usage
Subelement <dateOther> requires more analysis as to feasibility of including more specific info from element attributes

ndushay commented 9 months ago

@thatbudakguy do we want to keep the mods_display approach of delivering html fragments for the new imprint information to go in purl, or do we want to get the field label and field value from mods display and let the app (purl) choose the html tags and so on?

peetucket commented 9 months ago

One intermediate option may be to create one method to just extra data, and leave the existing methods that emit HTML markup too (and that could be refactored to use the extraction methods to avoid code duplication). The consumers of the gem could then pick if they want to deal with HTML markup themselves or not. Would at least allow us to move towards that without having to change all consumers at once (though could of course lead to some inconsistency).

thatbudakguy commented 9 months ago

talking to the access team, it seems like the least interventionist thing to do would be:

write new, dedicated methods for rendering the HTML to display the separate fields listed in this ticket
leave the imprint-related method alone, so that other consuming apps can continue to use it
call the new methods in PURL to render the appropriate <dl>

if we stop writing view components/HTML in mods_display, I think it becomes hard to justify its existence (since mods/stanford-mods are supposed to be "the parsing ones"). whether or not we want to pre-render HTML for apps consuming the gem is a different discussion, but I think if apps don't want/need prerendered HTML, they can just use the other parts of the stack (mods/stanford-mods).

on the other hand, if we need to do some more decoupling of mods_display and stanford-mods as part of this ticket, I think that could be worthwhile. if the boundary between "parsing the metadata" and "rendering the html" isn't very clean, maybe we can do some work to make it better.

ndushay commented 9 months ago

Nick - FYI:

I always intended to leave existing methods in mods_display alone, so I don't inadvertently break other apps.

I can proceed as you suggest, but it feels like determining a direction forward on matters such as these would be useful ... and requires more Access dev opinions.

should these new methods for purl imprint information return info in an outer <dl> tag?
- is this an accessibility thing that should become standard for all the public methods for mods_display?
- should there be a simple wrapper method in mods_display, like outer_dl(:mods_display_method_name) so apps can opt into the <dl> tag for anything?
should there be better documentation on the purpose of mods_display?
- i was really startled when I discovered it returns html fragments - a few sentences of documentation could help.
- How useful is it for html fragments to be served to apps? Does it provide value, or constrain the apps? How similar are the diff access app consumers of MODS?
- if it's about combining or accessing information in MODS for display, without the html tags ... should that be in stanford-mods??

It's unfortunate you've been ill; I've sort of paused here because I think maybe a real-time discussion needs to happen with the appropriate access team players on the next-gen architecture for MODS -> displayed in apps. How should metadata information from SDR turn into accessible html for access apps?

I feel like this is an opportunity to refactor with an eye for the future, and so I've paused briefly to allow that future vision to be discussed. Laura has volunteered to set up a meeting, if that is needed.

Of course I can proceed as you indicate ... but if waiting another day or week will result in some knowledge share, then we won't be ripping out fresh code to move our code towards that future.

ndushay commented 9 months ago

so Action items:

come up with a way to wrap all <dt><dd> pairs in a <dl>, with a way to "opt out" as necessary (default is to include <dl>) -- I think Nick was going to write a separate ticket for this? (sul-dlss/purl#786)
create new methods in mods_display to retrieve each desired imprint info nugget, with the accompanying <dl><dt><dd> tags. Naomi will do this for this ticket.

justinlittman commented 9 months ago

Is this work affected by the new design?

lwrubel commented 9 months ago

@ndushay is continuing to work through the date fields rendering (see table).

ndushay commented 8 months ago

Ready

place PR #160
publisher PR #160
edition PR #160, PR #162
copyrightDate PR #161
dateCaptured PR #161
dateCreated PR #161
dateIssued PR #161
dateModified PR #161
dateValid PR #161
issuance PR #163
frequency PR #163

That's all the new fields except for dateOther which is backburnered.

ndushay commented 8 months ago

@amyehodge @arcadiafalcone if displayLabel is present on an originInfo element, should any of these new elements use the displayLabel value? Or is this so rare that we don't care?

Also, is there any distinction between multiple (dateCreated) in a single originInfo element vs. multiple originInfo elements with (dateCreated) in terms of display? Or is this so rare that we don't care?

ndushay commented 8 months ago

also @arcadiafalcone any progress with dateOther?

ndushay commented 8 months ago

@thatbudakguy I think it will be easier to implement wrapping all the output fields with <dl> tags because that html code is magically supplied for all fields. And everything I did is a field (most things are fields, I believe).

~~So maybe that's a separate (accessibility) ticket for mods_display?~~

I read Chris's comment on #158 ... We already have sul-dlss/purl#786 to add <dl> tags for purl ...

arcadiafalcone commented 8 months ago

Re: originInfo displayLabel - I don't think dropping this will cause a problem, but I'll request a report to double-check.

Re: data in different originInfo elements - The data isn't consistent enough to make it worth trying to group items for display.

arcadiafalcone commented 8 months ago

Re: dateOther Assuming the dateOther type validates against the list of event.date types at https://github.com/sul-dlss/cocina-models/blob/main/description_types.yml

Use label "Other date" and process date for display as for other date fields. Include the value of dateOther[@type] in parentheses after the value. Example

1995

Other date 1995 (acquisition)

This may be updated to include the calendar value in the parentheses as well - I'm checking if the data will support that.

ndushay commented 8 months ago

remaining:

doublecheck what to do with displayLabel for new elements after report is run
dateOther -- to be implemented.

ndushay commented 8 months ago

PR #168 will close this ticket. Any additional changes, e.g.

dateOther algorithm tweaks
displayLabel algorithm tweaks
whatever

should be ticketed separately.

The work to display these values in purl is ticket sul-dlss/purl#845.

sul-dlss / mods_display

Update how imprint data is displayed on PURL #153

Ready