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.

Sign up for GitHub

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

Adds items in this collection section. #724

Closed
wants to merge 1 commit into from
Closed

Adds items in this collection section. #724

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions app/models/public_xml.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,8 @@ def relations(predicate)
node.attribute('resource').text.split('/', 2).last.split(':', 2).last
end
end

def object_type
document.root.at_xpath('identityMetadata/objectType').try(:text)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why try here? There shouldn't be any nodes with out an objectType, right? That's pretty fundamental.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because every other call to .text in this class is .try(:text). I have no idea why that would be, but following existing conventions.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think that's a very good reason. A lot of this code was written 12 years ago before the model was clear and before there was a safe navigation operator in Ruby.

end
end
1 change: 1 addition & 0 deletions app/models/purl_resource.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ def collection
end

delegate :rights_metadata, to: :public_xml
delegate :object_type, to: :public_xml

def content_metadata
@content_metadata ||= ContentMetadata.new(public_xml.content_metadata)
Expand Down
3 changes: 3 additions & 0 deletions app/views/purl/_collection_items.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<% if @purl.object_type == 'collection' && @purl.catalog_key %>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you make a method collection? here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would have, except PurlResource already has a .collection method which provides the collection that the item is a part of. It seems that a .collection? method that indicates whether this item is a collection risks confusion.

Copy link
Contributor

Choose a reason for hiding this comment

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

I still feel that we should move as much logic from the views as possible. If we need to rename collection to is_member_of, I think that would be good to and help align with Cocina terminology.

Copy link
Contributor

Choose a reason for hiding this comment

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

This method name has been refactored, so please move this to the collection? method now.

<%= link_to 'View items in this collection in SearchWorks', "#{Settings.searchworks.url}/catalog?f[collection][]=#{@purl.catalog_key}" %>
Copy link
Contributor

@jcoyne jcoyne Oct 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Is it possible the collection is not released to searchworks?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nope, because it has a catalog key.

<% end %>
2 changes: 1 addition & 1 deletion app/views/purl/_find_it.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<% if @purl.catalog_key %>
<%= link_to 'View in SearchWorks', Settings.searchworks.url + @purl.catalog_key %>
<%= link_to 'View in SearchWorks', "#{Settings.searchworks.url}/view/#{@purl.catalog_key}" %>
<% end %>

<% releases = [] %>
Expand Down
13 changes: 13 additions & 0 deletions app/views/purl/_mods_metadata_sections.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
bibliography = render(partial: "mods_bibliographic", locals: {document: document})
contact = render(partial: "mods_contact", locals: {document: document})
find_it = render(partial: "find_it", locals: {document: document})
collection_items = render(partial: "collection_items", locals: {document: document})
%>

<% if description.present? %>
Expand Down Expand Up @@ -85,6 +86,18 @@
</div>
<% end %>

<% if collection_items.present? %>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't you just move all of this into the partial? That is already wrapped in a conditional. Alternatively if you introduce a ViewComponent here, you can make this be the render? condition. I find that much easier to read and test rather than this approach here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with you, but I am following the existing pattern in this file. I'm not trying to refactor in this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've changed the existing pattern in #727 so please follow that pattern here.

<div class="section" id="<%= t('record_side_nav.find_it.id') %>" data-side-nav-class="<%= t('record_side_nav.find_it.id') %>">
<div class="section-heading">
<h2><%= t('record_side_nav.find_it.icon').html_safe %> Items in collection</h2>
</div>
<div class="section-body">
<%= collection_items %>
</div>
</div>
<% end %>


<% if find_it.present? %>
<div class="section" id="<%= t('record_side_nav.find_it.id') %>" data-side-nav-class="<%= t('record_side_nav.find_it.id') %>">
<div class="section-heading">
Expand Down
2 changes: 1 addition & 1 deletion config/settings.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ releases:
url: 'https://earthworks.stanford.edu/catalog/stanford-%{druid}'

searchworks:
url: 'https://searchworks.stanford.edu/view/'
url: 'https://searchworks.stanford.edu'

landing_page_druids:
- bb112zx3193
Expand Down
32 changes: 32 additions & 0 deletions spec/fixtures/document_cache/bb/631/ry/3167/mods
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?xml version="1.0" encoding="UTF-8"?>
<mods xmlns="http://www.loc.gov/mods/v3" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#" xmlns:xlink="http://www.w3.org/1999/xlink" version="3.7" xsi:schemaLocation="http://www.loc.gov/mods/v3 http://www.loc.gov/standards/mods/v3/mods-3-7.xsd">
<titleInfo usage="primary">
<title>Native American Alumni Oral Histories</title>
</titleInfo>
<typeOfResource manuscript="yes" collection="yes">mixed material</typeOfResource>
<language>
<languageTerm authority="iso639-2b" type="code">eng</languageTerm>
</language>
<originInfo>
<place>
<placeTerm authority="marccountry" type="code">cau</placeTerm>
</place>
<issuance>monographic</issuance>
</originInfo>
<location>
<url usage="primary display">https://purl.stanford.edu/bb631ry3167</url>
</location>
<location>
<physicalLocation>Dept. of Special Collections and University Archives Stanford University Libraries Stanford, CA 94305</physicalLocation>
</location>
<recordInfo>
<recordContentSource authority="marcorg">CSt</recordContentSource>
<descriptionStandard>aacr</descriptionStandard>
<descriptionStandard>dacs</descriptionStandard>
<recordOrigin>Converted from MARCXML to MODS version 3.7 using MARC21slim2MODS3-7_SDR_v2-5.xsl (SUL 3.7 version 2.5 20210421; LC Revision 1.140 20200717)</recordOrigin>
<recordCreationDate encoding="marc">210811</recordCreationDate>
<recordIdentifier source="SIRSI">a13965062</recordIdentifier>
</recordInfo>
<accessCondition type="useAndReproduction">The materials are open for research use and may be used freely for non-commercial purposes with an attribution. For commercial permission requests, please contact the Stanford University Archives ([email protected]).</accessCondition>
<accessCondition type="copyright">Copyright (c) The Board of Trustees of the Leland Stanford Junior University. All rights reserved.</accessCondition>
</mods>
73 changes: 73 additions & 0 deletions spec/fixtures/document_cache/bb/631/ry/3167/public
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
<?xml version="1.0" encoding="UTF-8"?>
<publicObject id="druid:bb631ry3167" published="2023-01-05T18:31:17Z" publishVersion="cocina-models/0.86.0">
<identityMetadata>
<objectType>collection</objectType>
<objectLabel>Native American Alumni Oral Histories</objectLabel>
<otherId name="catkey">13965062</otherId>
</identityMetadata>
<rightsMetadata>
<access type="discover">
<machine>
<world/>
</machine>
</access>
<access type="read">
<machine>
<world/>
</machine>
</access>
<use>
<human type="useAndReproduction">The materials are open for research use and may be used freely for non-commercial purposes with an attribution. For commercial permission requests, please contact the Stanford University Archives ([email protected]).</human>
<license/>
</use>
<copyright>
<human>Copyright (c) The Board of Trustees of the Leland Stanford Junior University. All rights reserved.</human>
</copyright>
</rightsMetadata>
<rdf:RDF xmlns:fedora="info:fedora/fedora-system:def/relations-external#" xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#">
<rdf:Description rdf:about="info:fedora/druid:bb631ry3167">


</rdf:Description>
</rdf:RDF>
<oai_dc:dc xmlns:dc="http://purl.org/dc/elements/1.1/" xmlns:srw_dc="info:srw/schema/1/dc-schema" xmlns:oai_dc="http://www.openarchives.org/OAI/2.0/oai_dc/" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.openarchives.org/OAI/2.0/oai_dc/ http://www.openarchives.org/OAI/2.0/oai_dc.xsd">
<dc:title>Native American Alumni Oral Histories</dc:title>
<dc:type>Collection</dc:type>
<dc:language>eng</dc:language>
<dc:identifier>https://purl.stanford.edu/bb631ry3167</dc:identifier>
</oai_dc:dc>
<mods xmlns="http://www.loc.gov/mods/v3" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#" xmlns:xlink="http://www.w3.org/1999/xlink" version="3.7" xsi:schemaLocation="http://www.loc.gov/mods/v3 http://www.loc.gov/standards/mods/v3/mods-3-7.xsd">
<titleInfo usage="primary">
<title>Native American Alumni Oral Histories</title>
</titleInfo>
<typeOfResource manuscript="yes" collection="yes">mixed material</typeOfResource>
<language>
<languageTerm authority="iso639-2b" type="code">eng</languageTerm>
</language>
<originInfo>
<place>
<placeTerm authority="marccountry" type="code">cau</placeTerm>
</place>
<issuance>monographic</issuance>
</originInfo>
<location>
<url usage="primary display">https://purl.stanford.edu/bb631ry3167</url>
</location>
<location>
<physicalLocation>Dept. of Special Collections and University Archives Stanford University Libraries Stanford, CA 94305</physicalLocation>
</location>
<recordInfo>
<recordContentSource authority="marcorg">CSt</recordContentSource>
<descriptionStandard>aacr</descriptionStandard>
<descriptionStandard>dacs</descriptionStandard>
<recordOrigin>Converted from MARCXML to MODS version 3.7 using MARC21slim2MODS3-7_SDR_v2-5.xsl (SUL 3.7 version 2.5 20210421; LC Revision 1.140 20200717)</recordOrigin>
<recordCreationDate encoding="marc">210811</recordCreationDate>
<recordIdentifier source="SIRSI">a13965062</recordIdentifier>
</recordInfo>
<accessCondition type="useAndReproduction">The materials are open for research use and may be used freely for non-commercial purposes with an attribution. For commercial permission requests, please contact the Stanford University Archives ([email protected]).</accessCondition>
<accessCondition type="copyright">Copyright (c) The Board of Trustees of the Leland Stanford Junior University. All rights reserved.</accessCondition>
</mods>
<releaseData>
<release to="Searchworks">true</release>
</releaseData>
</publicObject>
52 changes: 52 additions & 0 deletions spec/fixtures/document_cache/gk/894/yk/3598/public
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
<?xml version="1.0" encoding="UTF-8"?>
<publicObject id="druid:gk894yk3598" published="2023-06-30T23:36:01Z" publishVersion="cocina-models/0.90.0">
<identityMetadata>
<objectType>collection</objectType>
<objectLabel>Acquisitions Serials</objectLabel>
<sourceId source="sul">Hydrus:collection-amanheim-2017-05-12T20:11:22.791Z</sourceId>
</identityMetadata>
<rightsMetadata>
<access type="discover">
<machine>
<world/>
</machine>
</access>
<access type="read">
<machine>
<world/>
</machine>
</access>
<use>
<human type="useAndReproduction">User agrees that, where applicable, content will not be used to identify or to otherwise infringe the privacy or confidentiality rights of individuals. Content distributed via the Stanford Digital Repository may be subject to additional license and use restrictions applied by the depositor.</human>
<license/>
</use>
</rightsMetadata>
<rdf:RDF xmlns:fedora="info:fedora/fedora-system:def/relations-external#" xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#">
<rdf:Description rdf:about="info:fedora/druid:gk894yk3598">


</rdf:Description>
</rdf:RDF>
<oai_dc:dc xmlns:dc="http://purl.org/dc/elements/1.1/" xmlns:srw_dc="info:srw/schema/1/dc-schema" xmlns:oai_dc="http://www.openarchives.org/OAI/2.0/oai_dc/" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.openarchives.org/OAI/2.0/oai_dc/ http://www.openarchives.org/OAI/2.0/oai_dc.xsd">
<dc:title>Acquisitions Serials</dc:title>
<dc:type>Collection</dc:type>
<dc:description>Collection to load serial titles in digital format into the SDR.</dc:description>
<dc:identifier>https://purl.stanford.edu/gk894yk3598</dc:identifier>
<dc:description type="contact" displayLabel="Contact">[email protected]</dc:description>
</oai_dc:dc>
<mods xmlns="http://www.loc.gov/mods/v3" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#" xmlns:xlink="http://www.w3.org/1999/xlink" version="3.7" xsi:schemaLocation="http://www.loc.gov/mods/v3 http://www.loc.gov/standards/mods/v3/mods-3-7.xsd">
<titleInfo>
<title>Acquisitions Serials</title>
</titleInfo>
<typeOfResource collection="yes">mixed material</typeOfResource>
<abstract>Collection to load serial titles in digital format into the SDR.</abstract>
<location>
<url usage="primary display">https://purl.stanford.edu/gk894yk3598</url>
</location>
<note type="contact" displayLabel="Contact">[email protected]</note>
<accessCondition type="useAndReproduction">User agrees that, where applicable, content will not be used to identify or to otherwise infringe the privacy or confidentiality rights of individuals. Content distributed via the Stanford Digital Repository may be subject to additional license and use restrictions applied by the depositor.</accessCondition>
</mods>
<releaseData>
<release to="Searchworks">false</release>
</releaseData>
</publicObject>
9 changes: 9 additions & 0 deletions spec/integration/purl_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
@unpublished_object = 'ab123cd4567'
@legacy_object = 'ir:rs276tc2764'
@nested_resources_object = 'dm907qj6498'
@collection = 'bb631ry3167'
end

describe 'manifest' do
Expand Down Expand Up @@ -206,6 +207,14 @@
end
end

describe 'items in collection' do
it 'included in purl page' do
visit "/#{@collection}"
expect(page).to have_content 'Items in collection'
expect(page).to have_content 'View items in this collection in SearchWorks'
end
end

describe '/status (app monitoring)' do
it 'has response code 200' do
visit '/status'
Expand Down
16 changes: 16 additions & 0 deletions spec/model/purl_resource_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,4 +291,20 @@
end
end
end

describe '#object_type' do
it 'pulls the value from the identity metadata' do
allow(subject).to receive(:public_xml_body).and_return <<-EOF
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move this to a before as it's part of setting up the scenario and has no assertions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm just following the pattern in this file.

Copy link
Contributor

Choose a reason for hiding this comment

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

This codebase is full of bad patterns and needs to be over hauled. Please use practices like https://www.betterspecs.org/#mock and not the existing patterns here.

<?xml version="1.0" encoding="UTF-8"?>
<publicObject>
<identityMetadata>
<objectType>collection</objectType>
<objectLabel>Acquisitions Serials</objectLabel>
</identityMetadata>
</publicObject>
EOF

expect(subject.object_type).to eq 'collection'
end
end
end
35 changes: 35 additions & 0 deletions spec/views/purl/_collection_items.html.erb_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
require 'rails_helper'

RSpec.describe 'purl/_collection_items' do
before do
assign(:purl, purl)
end

context 'when collection has a catalog record id' do
let(:purl) { PurlResource.new(id: 'bb631ry3167') }

it 'displays a View items in this collection link' do
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this same scenario tested in the integration test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but I wanted the test cases in this spec to be complete so that a dev could understand the expected behavior from the view from this alone.

The integration test is necessary because it tests that the view is actually called.

render
expect(rendered).to have_css 'a[href="https://searchworks.stanford.edu/catalog?f[collection][]=13965062"]',
text: 'View items in this collection in SearchWorks'
end
end

context 'when collection does not have a catalog record id' do
let(:purl) { PurlResource.new(id: 'gk894yk3598') }

it 'does not display View items in this collection link' do
render
expect(rendered).not_to have_css 'a'
end
end

context 'when not a collection' do
let(:purl) { PurlResource.new(id: 'cg357zz0321') }

it 'does not display View items in this collection link' do
render
expect(rendered).not_to have_css 'a'
end
end
end