Skip to content

Use 250 additional metadata #308

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

Merged
JPrevost merged 8 commits into from
Dec 15, 2025
Merged

Use 250 additional metadata #308

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6c7869c
Appends ASpace collection to title
JPrevost Dec 10, 2025
3f3ddf6
Add Date ranges to records
JPrevost Dec 10, 2025
da83c3d
Adds summaries to records
JPrevost Dec 10, 2025
5c724aa
Adds subjects to results
JPrevost Dec 12, 2025
289b53e
Adds citation to Primo results
JPrevost Dec 12, 2025
1d8eb2d
TIMDEX Cassette regeneration
JPrevost Dec 10, 2025
2564dce
Ensure date_ranges only select dates with ranges
JPrevost Dec 12, 2025
849a498
Adds test for date range
JPrevost Dec 12, 2025
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
3 changes: 2 additions & 1 deletion app/assets/stylesheets/partials/_results.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@
overflow: hidden;
display: -webkit-box;
-webkit-line-clamp: 3;
line-clamp: 3;
-webkit-box-orient: vertical;
text-overflow: ellipsis;
}
Expand Down Expand Up @@ -404,4 +405,4 @@
@media (max-width: $bp-screen-md) {
flex-direction: column;
}
}
}
2 changes: 1 addition & 1 deletion app/helpers/record_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ def date_parse(date)
date
end

def date_range(range)
def geo_date_range(range)
return unless range.present?

"#{date_parse(range['gte'])} to #{date_parse(range['lte'])}"
Expand Down
12 changes: 2 additions & 10 deletions app/models/normalize_primo_record.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,17 +115,9 @@ def links
end

def citation
return unless @record['pnx']['addata']
return unless @record['pnx']['display']

if @record['pnx']['addata']['volume'].present?
if @record['pnx']['addata']['issue'].present?
"volume #{@record['pnx']['addata']['volume'].join} issue #{@record['pnx']['addata']['issue'].join}"
else
"volume #{@record['pnx']['addata']['volume'].join}"
end
elsif @record['pnx']['addata']['date'].present? && @record['pnx']['addata']['pages'].present?
"#{@record['pnx']['addata']['date'].join}, pp. #{@record['pnx']['addata']['pages'].join}"
end
@record['pnx']['display']['ispartof']&.first
end

def container
Expand Down
45 changes: 42 additions & 3 deletions app/models/normalize_timdex_record.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ def normalize
subjects:,
# TIMDEX-specific fields
content_type:,
date_range:,
dates:,
contributors:,
highlight:,
Expand All @@ -34,7 +35,26 @@ def normalize
private

def title
@record['title'] || 'Unknown title'
title = @record['title'] || 'Unknown title'

# The collection identifier is important for ASpace records so we append it to the title
return title unless source == 'MIT ArchivesSpace'

title += " (#{aspace_collection(@record['identifiers'])})"
end

def aspace_collection(identifiers)
relevant_ids = identifiers.map { |id| id['value'] if id['kind'] == 'Collection Identifier' }.compact

# In the highly unlikely event that there is more than one collection identifier, there's something weird going
# on with the record and we should look into it.
if relevant_ids.count > 1
Sentry.set_tags('mitlib.recordId': identifier || 'empty record id')
Sentry.set_tags('mitlib.collection_ids': relevant_ids.join('; '))
Sentry.capture_message('Multiple Collection IDs found in ASpace record')
end

relevant_ids.first
end

def creators
Expand Down Expand Up @@ -104,7 +124,7 @@ def links
end

def citation
@record['citation'] || nil
@record['citation']
end

def summary
Expand All @@ -130,7 +150,7 @@ def location
def subjects
return [] unless @record['subjects']

@record['subjects'].map { |subject| subject['value'] }
@record['subjects'].flat_map { |subject| subject['value'] }
end

def identifier
Expand All @@ -146,6 +166,25 @@ def dates
@record['dates']
end

def date_range
return unless @record['dates']

# Some records have creation or publication dates that are ranges. Extract those here.
relevant_dates = @record['dates'].select do |date|
%w[creation publication].include?(date['kind']) && date['range'].present?
end

# If the record has no creation or publication date, stop here.
return if relevant_dates.empty?

# If the record *does* have more than one creation/pub date, just take the first one. Note: ASpace records often
# have more than one. Sometimes they are duplicates, sometimes they are different. For now we will just take the
# first.
relevant_date = relevant_dates.first
Copy link
Member

@matt-bernhardt matt-bernhardt Dec 12, 2025

Choose a reason for hiding this comment

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

I'm a little concerned that this block will miss valid date ranges that may be in the data. The scenario is:

  • relevant_dates gets populated with multiple creation / publication entries
  • The first of these entries is not a range, but later ones are
  • Because we only grab the first entry in this list, and it is not a range, this method returns empty while a valid range got discarded.

It feels like a more robust approach would be to do all the filtering prior to the first return clause, something like .select { |date| $w[creation publication.include?(date['kind']) && date['range'].present? } ?

To be clear, I haven't looked at whether this condition exists in Timdex records - if dates are all either ranges or not-ranges, then this concern may be irrelevant and what you've written here is fine.

Copy link
Member Author

@JPrevost JPrevost Dec 12, 2025

Choose a reason for hiding this comment

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

Good point. This logic was ported from Bento but Bento only ever dealt with ASpace records so it is likely we need to revisit the logic.

Copy link
Member Author

@JPrevost JPrevost Dec 12, 2025

Choose a reason for hiding this comment

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

Updated using general idea you proposed and added a test to confirm behavior.


"#{relevant_date['range']['gte']}-#{relevant_date['range']['lte']}"
end

def contributors
@record['contributors']
end
Expand Down
13 changes: 13 additions & 0 deletions app/models/timdex_search.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ class TimdexSearch < TimdexBase
hits
records {
timdexRecordId
identifiers {
kind
value
}
title
source
contentType
Expand All @@ -61,6 +65,10 @@ class TimdexSearch < TimdexBase
dates {
kind
value
range {
gte
lte
}
}
links {
kind
Expand All @@ -84,6 +92,11 @@ class TimdexSearch < TimdexBase
}
sourceLink
summary
subjects {
kind
value
}
citation
Copy link
Member

@matt-bernhardt matt-bernhardt Dec 12, 2025

Choose a reason for hiding this comment

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

Non-blocking question about the citation field specifically:

Adding the citation field here, and the update to the citation method in the Timdex normalizer, don't seem to be paired with any relevant change to the timdex result partial - we're only adding a comment there that things aren't ready to display yet? (It feels like the method in the normalizer was never called, if the query never populated the field)

I agree with the comment you've added that citations in timdex need more work to be useful in this way, so I'm treating the changes in this work as part of that, but wanted to state my reading here in case I've missed a change in the UI logic.

Copy link
Member Author

@JPrevost JPrevost Dec 12, 2025

Choose a reason for hiding this comment

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

You are correct. I initially implemented the citation in the timdex partial but the data is so wonky in the citation I took it out of the view. I left it in the query intact so it will be easier to show people the weird data in the near future without having to rebuild cassettes.

}
aggregations {
accessToFiles {
Expand Down
2 changes: 1 addition & 1 deletion app/views/record/_record.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@
<ul class="list-dates">
<% @record['dates'].each do |date| %>
<li>
<%= date['kind'] %>: <%= date_parse(date['value']) %><%= date_range(date['range']) %>
<%= date['kind'] %>: <%= date_parse(date['value']) %>
<%= " Note: #{date['note']}" if date['note'].present? %>
</li>
<% end %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/record/_record_geo.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
<ul class="list-dates">
<% @record['dates'].each do |date| %>
<li>
<%= date['kind'] %>: <%= date_parse(date['value']) %><%= date_range(date['range']) %>
<%= date['kind'] %>: <%= date_parse(date['value']) %><%= geo_date_range(date['range']) %>
<%= " Note: #{date['note']}" if date['note'].present? %>
</li>
<% end %>
Expand Down
35 changes: 30 additions & 5 deletions app/views/search/_result.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,44 @@

<p class="pub-info">
<span><%= result[:content_type]&.each { |type| type['value'] }&.join(' ; ') %></span>
<span>
<% result[:dates]&.each do |date| %>
<%= date['value'] if date['kind'] == 'Publication date' %>
<% end %>
</span>

<% if result[:date_range].present? %>
<span><%= result[:date_range] %></span>
<% else %>
<span>
<% result[:dates]&.each do |date| %>
<%= date['value'] if date['kind'] == 'Publication date' %>
<% end %>
</span>
<% end %>
</p>

<%# unclear why TIMDEX is using contributors and Primo is using creators. Should we normalize this? %>
<% if result[:contributors].present? %>
<span class="sr">Contributors: </span>
<ul class="list-inline truncate-list contributors">
<%= render partial: 'shared/contributors', locals: { contributors: result[:contributors] } %>
</ul>
<% end %>

<%# Primo displays citation here. TIMDEX citations probably need work to be useful here %>

<% if result[:subjects].present? %>
<div class="result-subjects truncate-list">
<span class="sr">Subjects: </span>
<ul>
<% result[:subjects].each do |subject| %>
<li><%= subject %></li>
<% end %>
</ul>
</div>
<% end %>

<% if result[:summary].present? %>
<div class="result-summary truncate-list">
Copy link
Member

@matt-bernhardt matt-bernhardt Dec 12, 2025

Choose a reason for hiding this comment

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

Non-blocking concern: I see that we're using a div element here, while the geodata partial uses a paragraph element for the summary. I don't know if that's meaningful, and is ultimately a question for Dave I think, but there's a part of me that worries about unintended divergence in the templates that provide each instance.

Copy link
Member Author

@JPrevost JPrevost Dec 12, 2025

Choose a reason for hiding this comment

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

GeoData needs to be completely reworked once we stabilize USE. I think consistency within the USE partials is the main focus right now, and yeah I'm letting Dave determine ultimate markup.

<span class="sr">Summary: </span><%= result[:summary] %>
</div>
<% end %>
</div>

<div class="result-highlights use">
Expand Down
23 changes: 23 additions & 0 deletions app/views/search/_result_primo.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
<span><%= result[:year] %></span>
</p>

<%# unclear why TIMDEX is using contributors and Primo is using creators. Should we normalize this? %>
<% if result[:creators].present? %>
<span class="sr">Contributors: </span>
<ul class="list-inline truncate-list contributors">
Expand All @@ -32,6 +33,28 @@
</ul>
<% end %>

<% if result[:citation].present? %>
<div class="result-container">
<span>In: </span><%= result[:citation] %>
</div>
<% end %>

<% if result[:subjects].present? %>
<div class="result-subjects truncate-list">
<span class="sr">Subjects: </span>
<ul>
<% result[:subjects].each do |subject| %>
<li><%= subject %></li>
<% end %>
</ul>
</div>
<% end %>

<% if result[:summary].present? %>
<div class="result-summary truncate-list">
Copy link
Member

@matt-bernhardt matt-bernhardt Dec 12, 2025

Choose a reason for hiding this comment

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

Same non-blocking concern here as for the timdex result partial above, about whether the markup should be uniform between timdex / primo / geo partials.

<span class="sr">Summary: </span><%= result[:summary] %>
</div>
<% end %>
</div>

<div class="result-get">
Expand Down
100 changes: 76 additions & 24 deletions test/fixtures/primo/full_record.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,35 +1,83 @@
{
"pnx": {
"display": {
"title": ["Testing the Limits of Knowledge"],
"creator": ["Smith, John A.", "Jones, Mary B."],
"contributor": ["Brown, Robert C."],
"creationdate": ["2023"],
"type": ["book"],
"description": ["A comprehensive study of testing methodologies"],
"subject": ["Computer Science", "Software Testing"]
"title": [
"Testing the Limits of Knowledge"
],
"creator": [
"Smith, John A.",
"Jones, Mary B."
],
"contributor": [
"Brown, Robert C."
],
"creationdate": [
"2023"
],
"ispartof": [
"Journal of Testing, Vol. 2, Issue 3"
],
"type": [
"book"
],
"description": [
"A comprehensive study of testing methodologies"
],
"subject": [
"Computer Science",
"Software Testing"
]
},
"addata": {
"btitle": ["Complete Guide to Testing"],
"date": ["2023"],
"volume": ["2"],
"issue": ["3"],
"pages": ["123-145"],
"jtitle": ["Journal of Testing"],
"isbn": ["9781234567890", "1234567890"],
"pub": ["MIT Press"],
"doi": ["10.1038/s41567-023-02305-y"],
"pmid": ["22110403"]
"btitle": [
"Complete Guide to Testing"
],
"date": [
"2023"
],
"volume": [
"2"
],
"issue": [
"3"
],
"pages": [
"123-145"
],
"jtitle": [
"Journal of Testing"
],
"isbn": [
"9781234567890",
"1234567890"
],
"pub": [
"MIT Press"
],
"doi": [
"10.1038/s41567-023-02305-y"
],
"pmid": [
"22110403"
]
},
"facets": {
"frbrtype": ["5"],
"frbrgroupid": ["12345"]
"frbrtype": [
"5"
],
"frbrgroupid": [
"12345"
]
},
"search": {
"creationdate": ["2023"]
"creationdate": [
"2023"
]
},
"control": {
"recordid": ["alma991000000001234567"]
"recordid": [
"alma991000000001234567"
]
}
},
"context": "contextual",
Expand All @@ -41,10 +89,14 @@
"availabilityStatus": "available"
},
"holding": [
{"location": "Main Library"},
{"location": "Branch Library"}
{
"location": "Main Library"
},
{
"location": "Branch Library"
}
],
"link": [],
"almaOpenurl": "https://na06.alma.exlibrisgroup.com/view/uresolver/01MIT_INST/openurl?param=value"
}
}
}
Loading