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

Reorganize GeoData full record based on usability test results #185

Merged
merged 1 commit into from
May 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
26 changes: 8 additions & 18 deletions app/helpers/record_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,32 +98,16 @@ def access_type(metadata)
access_right.first['description']
end

# For GDT records, a 'more information' section includes all fields that are currently mapped in
# [transmogrifier](https://github.com/MITLibraries/transmogrifier/blob/main/transmogrifier/sources/json/aardvark.py).
# Note: the publishers field is not yet available in TIMDEX API, but it should be added here once it is.
def more_info_geo?(metadata)
relevant_fields = %w[alternate_titles contributors dates format identifiers languages locations
links notes provider publishers rights]
metadata.keys.any? { |key| relevant_fields.include? key }
end

def parse_nested_field(field)
# Don't continue if it's not a nested field.
return unless field.is_a?(Array) && field.first.is_a?(Hash)

# We don't care about display subfields with null values, our the contributors 'mitAffiliated' subfield.
# We don't care about display subfields with null values.
field.map do |subfield|
subfield.reject { |key, value| key == 'mitAffiliated' || value.blank? }
subfield.reject { |_, value| value.blank? }
end.compact
end

def render_subfield(subfield)
# Date ranges are handled differently than other subfields
return ("kind: #{subfield['kind']}; range: " + date_range(subfield['range'])) if subfield['range'].present?

subfield.map { |key, value| "#{key}: #{value}" }.join('; ')
end

def source_metadata_available?(links)
links&.any? { |link| link['kind'] == 'Download' && link['text'] == 'Source Metadata' }
end
Expand All @@ -134,6 +118,12 @@ def source_metadata_link(links)
links.select { |link| link['kind'] == 'Download' && link['text'] == 'Source Metadata' }.first['url']
end

def geospatial_coordinates?(locations)
return if locations.blank?

locations.any? { |location| location['geoshape'] }
end

private

def render_kind_value(list)
Expand Down
2 changes: 1 addition & 1 deletion app/models/timdex_record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class TimdexRecord < TimdexBase
}
literaryForm
locations {
geopoint
geoshape
kind
value
}
Expand Down
107 changes: 0 additions & 107 deletions app/views/record/_more_info_geo.html.erb

This file was deleted.

88 changes: 86 additions & 2 deletions app/views/record/_record_geo.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,37 @@
</p>
<% end %>

<% if @record['alternateTitles'].present? %>
<h3 class="section-title">Alternate titles</h3>
<ul class="list-unbulleted">
<% parse_nested_field(@record['alternateTitles']).each do |title| %>
<li><%= title['value'] %></li>
<% end %>
</ul>
<% end %>

<% if @record['dates'].present? %>
<h3 class="section-title">Dates</h3>
<ul class="list-dates">
<% @record['dates'].each do |date| %>
<li>
<%= date['kind'] %>: <%= date_parse(date['value']) %><%= date_range(date['range']) %>
<%= " Note: #{date['note']}" if date['note'].present? %>
</li>
<% end %>
</ul>
<% end %>

<!-- The publishers field also includes location and date subfields, but these are unused in GDT. -->
<% if @record['publishers'].present? %>
<h3 class="section-title">Publishers</h3>
<ul>
<% parse_nested_field(@record['publishers']).each do |publisher| %>
<li><%= publisher['name'] %></li>
<% end %>
</ul>
<% end %>

<% if @record['summary'].present? %>
<h3 class="section-title">Summary</h3>
<% @record['summary'].each do |paragraph| %>
Expand All @@ -37,8 +68,61 @@
</ul>
<% end %>

<% if more_info_geo?(@record) %>
<%= render partial: 'more_info_geo', locals: { metadata: @record } %>
<!-- We only care about geospatial locations for this, as place names are also subjects. -->
<% if geospatial_coordinates?(@record['locations']) %>
<h3 class="section-title">Geospatial coordinates</h3>
<ul>
<% parse_nested_field(@record['locations']).each do |location| %>
JPrevost marked this conversation as resolved.
Show resolved Hide resolved
<% if location['geoshape'].present? %>
<li><%= "#{location['kind']}: #{location['geoshape']}" %></li>
<% end %>
<% end %>
</ul>
<% end %>

<% if @record['notes'].present? %>
<h3 class="section-title">Notes</h3>
<ul>
<% parse_nested_field(@record['notes']).each do |note| %>
<li><%= "#{note['kind']}: " if note['kind'] %><%= note['value'] %></li>
<% end %>
</ul>
<% end %>

<% if @record['provider'].present? %>
<h3 class="section-title">Provider</h3>
<p><%= @record['provider'] %></p>
<% end %>

<% if @record['rights'].present? %>
<h3 class="section-title">Rights</h3>
<ul>
<% parse_nested_field(@record['rights']).each do |right| %>
<!-- Ignore 'access to files' right, which is implied in the access link and the 'geo_data_info partial' -->
<% unless right['kind'] == 'Access to files' %>
<li><%= "#{right['kind']}: " if right['kind'] %><%= right['description'] %></li>
<% end %>
<% end %>
</ul>
<% end %>

<% if @record['citation'].present? %>
<h3 class="section-title">Citation</h3>
<p><%= @record['citation'] %></p>
<% end %>

<% if @record['format'].present? %>
<h3 class="section-title">Format</h3>
<p><%= @record['format'] %></p>
<% end %>

<% if @record['languages'].present? %>
<h3 class="section-title">Languages</h3>
<ul>
<% @record['languages'].each do |language| %>
<li><%= language %></li>
<% end %>
</ul>
<% end %>

<div class="record-access-links">
Expand Down
42 changes: 11 additions & 31 deletions test/helpers/record_helper_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -270,26 +270,6 @@ class RecordHelperTest < ActionView::TestCase
assert_equal 'https://example.org/metadata.zip', source_metadata_link(links)
end

test 'more_info_geo? true if some relevant fields exist' do
date_record = { 'dates' => [{ 'kind' => 'Issued', 'value' => '2001' }] }
assert more_info_geo?(date_record)

locations_record = { 'locations' => [{ 'kind' => 'Place Name', 'value' => 'The Village Vanguard' }] }
assert more_info_geo?(locations_record)

provider_record = { 'provider' => 'MIT' }
assert more_info_geo?(provider_record)

publishers_record = { 'publishers' => [{ 'name' => 'Jagjaguwar', 'location' => 'Bloomington, IN',
'date' => '2011' }] }
assert more_info_geo?(provider_record)
end

test 'more_info_geo? false if no more info available' do
record = { 'title' => 'foo', 'source' => 'bar' }
assert_not more_info_geo?(record)
end

test 'parse_nested_field returns nil for fields that are not nested' do
string_field = 'string'
array_of_strings_field = ['string', 'other_string']
Expand All @@ -302,23 +282,23 @@ class RecordHelperTest < ActionView::TestCase
assert_equal nested_field, parse_nested_field(nested_field)
end

test 'parse_nested_field ignores mitAffiliated subfield' do
contributors = [{ 'kind' => 'bandleader', 'value' => 'Coltrane, John', 'mitAffiliated' => false }]
assert_equal [{ 'kind' => 'bandleader', 'value' => 'Coltrane, John' }], parse_nested_field(contributors)
end

test 'parse_nested_field trims null values' do
contributors = [{ 'kind' => 'bandleader', 'value' => 'Coltrane, John', 'identifier' => nil }]
assert_equal [{ 'kind' => 'bandleader', 'value' => 'Coltrane, John' }], parse_nested_field(contributors)
end

test 'render_subfield treats date ranges accordingly' do
date_range = { 'kind' => 'Coverage', 'value' => '1999', 'range' => { 'gte' => '1999', 'lte' => '2000' } }
assert_equal "kind: Coverage; range: 1999 to 2000", render_subfield(date_range)
test 'geospatial_coordinates? returns true if geoshape is present' do
geoshape = [{ 'kind' => 'a', 'geoshape' => 'b' }]
assert geospatial_coordinates?(geoshape)
end

test 'geospatial_coordinates? returns false if geoshape is absent' do
place = [{ 'kind' => 'Place name', 'value' => 'Pittsburgh' }]
assert_not geospatial_coordinates?(place)
end

test 'render_subfield renders a variety of key/value pairs' do
contributor = { 'kind' => 'bandleader', 'value' => 'Coltrane, John', 'identifier' => 'Trane', 'genre' => 'jazz' }
assert_equal "kind: bandleader; value: Coltrane, John; identifier: Trane; genre: jazz", render_subfield(contributor)
test 'geospatial_coordinates? returns false if locations are not present' do
locations = []
assert_not geospatial_coordinates?(locations)
end
end
Loading
Loading