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

Support blank Rel Types (Issue 1770) #64

Merged
merged 5 commits into from
Apr 21, 2021
Merged

Support blank Rel Types (Issue 1770) #64

merged 5 commits into from
Apr 21, 2021

Conversation

seth-shaw-unlv
Copy link
Contributor

GitHub Issue: Islandora/documentation#1770

What does this Pull Request do?

Updates the LinkedAgent field formatters to omit colons or commas as necessary when the field's rel_type value is blank.

What's new?

  • Changes LinkedAgent field formatters to omit colons or commas as necessary when the field's rel_type value is blank.
  • Requires a cache clearing

How should this be tested?

  • Add a rel_type with a blank label. (Note: @kspurgin has an islandora_defaults PR coming that will add this blank value.)
  • Create a new repository_item with a linked agent that uses the blank Relationship Type.
  • View the item and see the agent has a colon before the agent's link.
  • Change the display mode's linked agent field to use the Dedup formatter and see similar problematic behavior.
  • Apply the PR.
  • Clear Cache.
  • View the item and see the agent has a colon before the agent's link.
  • Play around with adding multiple typed relationships using the same agent, some blank, some not, and verify the new display still looks fine.

Additional Notes

It appears we don't have a Sprint project or label....

Interested parties

@Islandora/8-x-committers

@elizoller
Copy link
Member

I config imported the changes here: https://github.com/Islandora/islandora_defaults/pull/50/files
Created a repository item with a blank relationship type
viewed the item
saw this:
Screen Shot 2021-04-21 at 11 22 10 AM

pulled in this PR
cache clear
and now i have this:

Screen Shot 2021-04-21 at 11 23 54 AM

works fine with others as well
Screen Shot 2021-04-21 at 11 25 12 AM

@elizoller
Copy link
Member

I also tested with the dedup formatter and got this:
Screen Shot 2021-04-21 at 11 27 37 AM
This is slightly confusing because i attached Argelis as both no relation AND annotator. i'm not sure if thats a use case that would happen in real life, but the dedup formatter didn't include any relation for the second instance of the name. which i think makes sense, i just wanted to point it out.

@kspurgin
Copy link
Contributor

Argelis as both no relation AND annotator is certainly a situation that could happen in messy legacy metadata being migrated in.

Whether or not folks would want one of them removed seems like it would differ. But if you're going to only keep one instance of the term, seems to make sense to keep the one with more info on it.

Copy link
Member

@elizoller elizoller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

both formatters work as described

@elizoller
Copy link
Member

@kspurgin are you happy with the above screenshots?

@kspurgin
Copy link
Contributor

They all look good. Is there any need to verify the JSON-LD is getting handled correctly? Seems like it should since we still have the relators:asn before the |

@elizoller
Copy link
Member

{
"@graph": [
{
"@id": "http://localhost:8000/node/7",
"@type": [
"http://pcdm.org/models#Object",
"http://purl.org/coar/resource_type/c_18cc"
],
"http://schema.org/author": [
{
"@id": "http://localhost:8000/user/1"
}
],
"http://purl.org/dc/terms/title": [
{
"@value": "Test Audio",
"@language": "en"
}
],
"http://schema.org/dateCreated": [
{
"@value": "2021-03-25T15:48:31+00:00",
"@type": "http://www.w3.org/2001/XMLSchema#dateTime"
}
],
"http://schema.org/dateModified": [
{
"@value": "2021-04-21T18:27:01+00:00",
"@type": "http://www.w3.org/2001/XMLSchema#dateTime"
}
],
"http://purl.org/dc/terms/extent": [
{
"@value": "1 item",
"@type": "http://www.w3.org/2001/XMLSchema#string"
}
],
"http://id.loc.gov/vocabulary/relators/asn": [
{
"@id": "http://localhost:8000/taxonomy/term/40?_format=jsonld"
},
{
"@id": "http://localhost:8000/taxonomy/term/39?_format=jsonld"
}
],
"http://id.loc.gov/vocabulary/relators/ann": [
{
"@id": "http://localhost:8000/taxonomy/term/40?_format=jsonld"
}
],
"http://id.loc.gov/vocabulary/relators/anl": [
{
"@id": "http://localhost:8000/taxonomy/term/36?_format=jsonld"
}
],
"http://id.loc.gov/vocabulary/relators/anm": [
{
"@id": "http://localhost:8000/taxonomy/term/36?_format=jsonld"
}
],
"http://schema.org/sameAs": [
{
"@id": "http://localhost:8000/node/7"
}
]
},
{
"@id": "http://localhost:8000/user/1",
"@type": [
"http://schema.org/Person"
]
}
]
}

@kspurgin
Copy link
Contributor

Looks great, thanks!

@elizoller
Copy link
Member

elizoller commented Apr 21, 2021

i think that looks right too

@elizoller
Copy link
Member

and made it to fedora correctly as well
Screen Shot 2021-04-21 at 11 55 20 AM

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.

3 participants