Skip to content
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

Add formatter setting to suppress MARC Relator code in () #67

Open
wants to merge 1 commit into
base: 2.x
Choose a base branch
from

Conversation

kayakr
Copy link
Contributor

@kayakr kayakr commented Apr 29, 2021

GitHub Issue: No issue.

What does this Pull Request do?

Changes Typed Relation Formatter to add display setting to suppress MARC Relator code. For example, changes "Author (aut): Bilbo Baggins" to "Author: Bilbo Baggins". Is hard-coded to look for ( and ) via regex.

What's new?

Introduces new config field suppress_code for fields using "Typed Relation Formatter".

How should this be tested?

  • Apply patch
  • Add Repository Item with Linked Agent values using MARC Relators.
  • When viewing item, Linked Agent values should include MARC Relator codes in parentheses.
  • Edit display for the relevant Repository Item display mode, e.g. PDFjs. Change Linked Agent field format to suppress code (checkbox "Hide code in () after label, if present" is checked). Save display configuration.
  • Clear cache
  • View repository item; Linked Agent values should no longer include MARC Relator codes in parentheses.

Interested parties

@seth-shaw-unlv @Islandora/8-x-committers

@seth-shaw-unlv
Copy link
Contributor

So, I'm split on this. Code-wise, all it need is some coding standard fixes to pass tests, I think.

However, I hesitate to hard-code a solution in this module a convention established in config (which a site could remove if they wish) in a different module, especially when that convention has recently been removed.

On the other hand, some sites might want the ability to include parenthetical statements on their labels for metadata creation/editing, while hiding that information from public display. This assumes they only use this Formatter for display (they would still show up in the dedup formatter unless we updated that one too) and understand that the full label with the parenthetical text may show up in unexpected places. (Although I don't think it would in a standard build of islandora_defaults, presently.)

I would like to hear from MIG folks before proceeding. Tagging @kspurgin, @rtilla1, and @rosiel. I don't appear to have a GitHub handle for Paige...

@kayakr
Copy link
Contributor Author

kayakr commented Apr 29, 2021

@seth-shaw-unlv You're right that this should probably live in islandora_defaults along with the rel_types for Linked Agent, being a specific implementation of Typed Relation. I missed the discussion about removing Relator codes from the labels. Do you know where that took place?

@kspurgin
Copy link
Contributor

@kayakr - The discussion about removing relator code from display originated in the Metadata Interest Group. I sneaked it in this discussion about other changes we were making to the field_linked_agent config: Islandora/documentation#1770 (comment)

@kspurgin
Copy link
Contributor

My position would be that display suppression of anything in parentheses should not be hard-coded in the Typed Relation field type.

Control of the display values desired for a given field instance using this field type should be handled in the "Available Relations" field configuration for the field instance.

The Typed Relation field type is not limited to names and MARC relator codes. Another institution (I forget which now) is using it with work<->work relationship types in the "Available Relations" and the Entity reference part configured to reference other Repository items.

Some "Available Relations" values people want to use may include parenthetical content intended for display.

In fact, MIG is going to discuss whether we want to add "(deprecated -- use {other term})" back to the display value in field_linked_agent config on terms that are deprecated, since this is actually information meant for human viewing/interpretation.

The fact that the MARC relator terms were in parentheses in the config for field_linked_agent was an idiosyncratic formatting choice specific to that use of the Typed Relation field type, and shouldn't drive the code/function of the field type.

@kayakr
Copy link
Contributor Author

kayakr commented Apr 29, 2021

My position would be that display suppression of anything in parentheses should not be hard-coded in the Typed Relation field type.

I agree; it should be closer to the source of the specific data, not in Typed Relation.

Control of the display values desired for a given field instance using this field type should be handled in the "Available Relations" field configuration for the field instance.

My use case is to retain the MARC Relator codes in the labels for use by repository administrators, but to suppress them for public display. Directly altering the available values in the field "Available Relations" removes the codes for all users, that doesn't meet my usecase (I guess I could remove them from the field data and reintroduce them through a formatter...)

The Typed Relation field type is not limited to names and MARC relator codes. Another institution (I forget which now) is using it with work<->work relationship types in the "Available Relations" and the Entity reference part configured to reference other Repository items.

Some "Available Relations" values people want to use may include parenthetical content intended for display.

Note that the patch offers a formatter setting, so it can be enabled or disabled on a per-field basis, as part of entity display or views display.

@rosiel
Copy link
Member

rosiel commented May 3, 2021

I like that it's on the field formatter - I think that makes sense (If I understand correctly, then under Manage Display you can say "suppress code in () after label". That's cool. (However, please teach me how to use this in Views, I thought views formatters were different)
The word "code" might not make sense outside of this use case of relator terms with "text" and "code". If this is suppressed for the public, the () could include all sorts of administrative information. Would "Suppress text in () after label, if present" be acceptable?

@kspurgin
Copy link
Contributor

kspurgin commented May 4, 2021

Earlier, I missed the detail "retain the MARC Relator codes in the labels for use by repository administrators, but to suppress them for public display." Assuming someone authenticated into Islandora to create or edit metadata is included in "repository administrators" here, I like it and so did the Metadata Interest Group when we discussed it yesterday.

One thing briefly mentioned was still having some reservations about hard-coding it as parentheses, since parentheses may ever mean something else in a given field (and be desirable to display), while that same field might have some admin data we wish to hide in some way. The idea was floated of making this configurable using a regex when you set up the field instance. Others shied away from expecting someone setting up the field to enter a correct regex. I wonder if there could be an option like:

Hide from public display any content...

  • in parentheses
  • in square brackets
  • in curly brackets

(where each bullet is a radio selection button)

Or, more flexibly:

Hide from public display any content between:
___ (beginning character) and ___ (ending character)

(where underscores are a little text entry box)

And then the formatter can use regex behind the scenes to make that happen.

Or maybe that's all too complex and we just expect folks to reformat values including parenthetical data that should publicly display if they want to use this option to hide other parts of the values.

But I did want to capture that bit of discussion from the MIG meeting and the two "well, maybe we could..." thoughts I had after.

@kspurgin
Copy link
Contributor

kspurgin commented May 4, 2021

+1 to @rosiel 's suggestion of "Suppress text in () after label, if present" for the wording

@seth-shaw-unlv seth-shaw-unlv changed the base branch from 8.x-1.x to 2.x October 5, 2021 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

4 participants