Skip to content

Commit cfc3554

Browse files
JPrevostJPrevost
authored
Merge pull request #402 from MITLibraries/use-611
Render fulfillment container only when applicable
2 parents 2cbfc1f + 67f1232 commit cfc3554

5 files changed

Lines changed: 134 additions & 11 deletions

File tree

‎app/helpers/results_helper.rb‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,4 +80,33 @@ def article?(format)
8080

8181
format.match?(/\barticle\b/i)
8282
end
83+
84+
# Determines if a result has any fulfillment links to render
85+
#
86+
# @param result [Hash] A normalized Primo result hash
87+
# @return [Boolean] True if the result has links, availability, or ThirdIron/OpenAlex triggers
88+
def result_get?(result)
89+
renderable_links?(result) ||
90+
result[:availability].present? ||
91+
(Feature.enabled?(:oa_always) && article?(result[:format])) ||
92+
thirdiron_content?(result)
93+
end
94+
95+
private
96+
97+
def renderable_links?(result)
98+
return false unless result[:links].present?
99+
return true if Feature.enabled?(:record_link)
100+
101+
# If record_link is disabled, exclude results that have ONLY "full record" links
102+
result[:links].any? { |link| link['kind'].downcase != 'full record' }
103+
end
104+
105+
def thirdiron_content?(result)
106+
ThirdIron.enabled? && (
107+
result[:doi].present? ||
108+
result[:pmid].present? ||
109+
(result[:format]&.downcase == 'journal' && result[:issn].present?)
110+
)
111+
end
83112
end

‎app/javascript/controllers/content_loader_controller.js‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,26 @@ export default class extends Controller {
1111
fetch(this.urlValue)
1212
.then(response => response.text())
1313
.then(html => {
14-
const parentElement = this.element.parentElement;
15-
// Replace the entire element with the fetched HTML
16-
this.element.outerHTML = html;
17-
// Hide primo links if libkey link is present
18-
if (parentElement.querySelector('.libkey-link')) {
19-
const resultGet = parentElement.closest('.result-get');
20-
if (resultGet) {
21-
const primoLinks = resultGet.querySelectorAll('.primo-link');
22-
// removing instead of hiding to avoid layout issues when selecting which link to highlight
23-
primoLinks.forEach(link => link.remove());
14+
const parentElement = this.element.parentElement
15+
// Replace the entire element with the fetched HTML, or remove if empty
16+
if (html.trim()) {
17+
this.element.outerHTML = html
18+
// Hide primo links if libkey link is present
19+
if (parentElement.querySelector('.libkey-link')) {
20+
const resultGet = parentElement.closest('.result-get')
21+
if (resultGet) {
22+
const primoLinks = resultGet.querySelectorAll('.primo-link')
23+
// removing instead of hiding to avoid layout issues when selecting which link to highlight
24+
primoLinks.forEach(link => link.remove())
25+
}
26+
}
27+
} else {
28+
// Remove empty loader
29+
this.element.remove()
30+
31+
// Remove parent empty container, confirming first that it's empty
32+
if (!parentElement.textContent.trim()) {
33+
parentElement.remove()
2434
}
2535
}
2636
})

‎app/views/search/_result_primo.html.erb‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,8 @@
7575
<% end %>
7676
</div>
7777

78+
79+
<% if result_get?(result) %>
7880
<div class="result-get">
7981
<% if result[:links].present? %>
8082
<% result[:links].each do |link| %>
@@ -120,6 +122,7 @@
120122
<%= render(partial: 'trigger_browzine', locals: { issn: result[:issn] }) %>
121123
<% end %>
122124
</div>
125+
<% end %>
123126
<% end %>
124127
</div>
125128
</li>

‎config/environments/development.rb‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@
6868
# config.i18n.raise_on_missing_translations = true
6969

7070
# Annotate rendered view with file names.
71-
config.action_view.annotate_rendered_view_with_filenames = true
71+
# Disabled because annotation comments pollute dynamically-loaded HTML fragments (e.g. ThirdIron responses)
72+
# config.action_view.annotate_rendered_view_with_filenames = true
7273

7374
# Uncomment if you wish to allow Action Cable access from any origin.
7475
# config.action_cable.disable_request_forgery_protection = true

‎test/helpers/results_helper_test.rb‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,4 +111,84 @@ class ResultsHelperTest < ActionView::TestCase
111111
assert_not article?(nil)
112112
assert_not article?('')
113113
end
114+
115+
test 'result_get? returns true when result has links' do
116+
assert result_get?({ links: [{ 'kind' => 'PDF', 'url' => 'https://example.com' }] })
117+
end
118+
119+
test 'result_get? returns true when result has availability' do
120+
assert result_get?({ availability: 'Available' })
121+
end
122+
123+
test 'result_get? returns true when ThirdIron enabled and result has doi' do
124+
ClimateControl.modify(THIRDIRON_ID: '123', THIRDIRON_KEY: 'abc') do
125+
assert result_get?({ doi: '10.1234/test' })
126+
end
127+
end
128+
129+
test 'result_get? returns true when ThirdIron enabled and result has pmid' do
130+
ClimateControl.modify(THIRDIRON_ID: '123', THIRDIRON_KEY: 'abc') do
131+
assert result_get?({ pmid: '12345678' })
132+
end
133+
end
134+
135+
test 'result_get? returns true when ThirdIron enabled and result is a journal with issn' do
136+
ClimateControl.modify(THIRDIRON_ID: '123', THIRDIRON_KEY: 'abc') do
137+
assert result_get?({ format: 'Journal', issn: '1234-5678' })
138+
end
139+
end
140+
141+
test 'result_get? returns true when oa_always enabled and result is an article' do
142+
ClimateControl.modify(FEATURE_OA_ALWAYS: 'true') do
143+
assert result_get?({ format: 'Article' })
144+
end
145+
end
146+
147+
test 'result_get? returns false when result has no fulfillment content' do
148+
ClimateControl.modify(THIRDIRON_ID: nil, THIRDIRON_KEY: nil, FEATURE_OA_ALWAYS: nil) do
149+
assert_not result_get?({})
150+
assert_not result_get?({ format: 'Book' })
151+
end
152+
end
153+
154+
test 'result_get? returns false when ThirdIron disabled even with doi' do
155+
ClimateControl.modify(THIRDIRON_ID: nil, THIRDIRON_KEY: nil) do
156+
assert_not result_get?({ doi: '10.1234/test' })
157+
end
158+
end
159+
160+
test 'result_get? returns false when oa_always disabled and result has no links' do
161+
ClimateControl.modify(FEATURE_OA_ALWAYS: nil, THIRDIRON_ID: nil, THIRDIRON_KEY: nil) do
162+
assert_not result_get?({ format: 'Article' })
163+
end
164+
end
165+
166+
test 'result_get? returns false when result has only full record link and feature is disabled' do
167+
ClimateControl.modify(FEATURE_RECORD_LINK: nil, THIRDIRON_ID: nil, THIRDIRON_KEY: nil, FEATURE_OA_ALWAYS: nil) do
168+
assert_not result_get?({ links: [{ 'kind' => 'Full Record', 'url' => 'https://example.com' }] })
169+
end
170+
end
171+
172+
test 'result_get? returns true when result has only full record link and feature is enabled' do
173+
ClimateControl.modify(FEATURE_RECORD_LINK: 'true', THIRDIRON_ID: nil, THIRDIRON_KEY: nil, FEATURE_OA_ALWAYS: nil) do
174+
assert result_get?({ links: [{ 'kind' => 'Full Record', 'url' => 'https://example.com' }] })
175+
end
176+
end
177+
178+
test 'result_get? returns true when result has PDF link even if record_link feature is disabled' do
179+
ClimateControl.modify(FEATURE_RECORD_LINK: nil, THIRDIRON_ID: nil, THIRDIRON_KEY: nil, FEATURE_OA_ALWAYS: nil) do
180+
assert result_get?({ links: [{ 'kind' => 'PDF', 'url' => 'https://example.com' }] })
181+
end
182+
end
183+
184+
test 'result_get? returns true when result has both full record and PDF links even if record_link feature is disabled' do
185+
ClimateControl.modify(FEATURE_RECORD_LINK: nil, THIRDIRON_ID: nil, THIRDIRON_KEY: nil, FEATURE_OA_ALWAYS: nil) do
186+
assert result_get?({
187+
links: [
188+
{ 'kind' => 'Full Record', 'url' => 'https://example.com' },
189+
{ 'kind' => 'PDF', 'url' => 'https://pdf.example.com' }
190+
]
191+
})
192+
end
193+
end
114194
end

0 commit comments

Comments
 (0)