Skip to content

Commit

Permalink
Merge pull request #192 from MITLibraries/gdt-317-return-to-search
Browse files Browse the repository at this point in the history 
Download links target _blank and append UI marker
  • Loading branch information
@JPrevost
JPrevost committed May 24, 2024
2 parents e616f95 + 54a2e46 commit a771df4
Show file tree
Hide file tree
Showing 4 changed files with 150 additions and 122 deletions.
1 change: 1 addition & 0 deletions Gemfile.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,7 @@ GEM

PLATFORMS
arm64-darwin-22
arm64-darwin-23
x86_64-darwin-19
x86_64-darwin-20
x86_64-darwin-21
Expand Down
19 changes: 17 additions & 2 deletions app/helpers/record_helper.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,27 @@ def gis_access_link(metadata)
# At this point, we don't show download links for non-MIT records. For MIT records, the download link is stored
# consistently as a download link. We are confirming that the link text is 'Data' for added confirmation.
if access_type(metadata) == 'unknown: check with owning institution'
links.select { |link| link['kind'] == 'Website' }.first['url']
website_url(links)
else
links.select { |link| link['kind'] == 'Download' && link['text'] == 'Data' }.first['url']
url = download_url(links)
append_timdexui(url)
end
end

def website_url(links)
links.select { |link| link['kind'] == 'Website' }.first['url']
end

def download_url(links)
links.select { |link| link['kind'] == 'Download' && link['text'] == 'Data' }.first['url']
end

def append_timdexui(url)
uri = Addressable::URI.parse(url)
uri.query_values = (uri.query_values || {}).merge(timdexui: true)
uri.to_s
end

def access_type(metadata)
access_right = metadata['rights']&.select { |right| right['kind'] == 'Access to files' }
return if access_right.blank?
Expand Down
2 changes: 1 addition & 1 deletion app/views/record/_access_buttons.html.erb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<% if access_type(@record) == 'no authentication required' %>
<a class="btn button-primary access-button" href="<%= gis_access_link(@record) %>">Download geodata files</a>
<% elsif access_type(@record) == 'MIT authentication required' %>
<a class="btn button-primary access-button" href="<%= gis_access_link(@record) %>">
<a class="btn button-primary access-button" href="<%= gis_access_link(@record) %>" target="_blank">
Download geodata files <span class="auth-notice">MIT authentication</span>
</a>
<% else %>
Expand Down
250 changes: 131 additions & 119 deletions test/helpers/record_helper_test.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,148 +103,160 @@ class RecordHelperTest < ActionView::TestCase
end

test 'access_type returns nil if corresponding right is blank' do
rights = {
'rights' => [
{
'description' => 'foo',
'kind' => 'bar'
}
]
}
rights = {
'rights' => [
{
'description' => 'foo',
'kind' => 'bar'
}
]
}
assert_nil access_type(rights)
end

test 'access_type returns the description of the corresponding right' do
rights = {
'rights' => [
{
'description' => 'foo',
'kind' => 'bar'
},
{
'description' => 'no authentication required',
'kind' => 'Access to files'
}
]
}
'rights' => [
{
'description' => 'foo',
'kind' => 'bar'
},
{
'description' => 'no authentication required',
'kind' => 'Access to files'
}
]
}
assert_equal 'no authentication required', access_type(rights)
end

test 'gis_access_link returns nil if access type is blank' do
links_no_rights = {
'links' => [
{
'kind' => 'Download',
'text' => 'Data',
'url' => 'https://example.org/dz_f7regions_2016.zip'
},
{
'kind' => 'Website',
'text' => 'Website',
'url' => 'https://example.org/gismit:dz_f7regions_2016'
}
]
}
'links' => [
{
'kind' => 'Download',
'text' => 'Data',
'url' => 'https://example.org/dz_f7regions_2016.zip'
},
{
'kind' => 'Website',
'text' => 'Website',
'url' => 'https://example.org/gismit:dz_f7regions_2016'
}
]
}
assert_nil access_type(links_no_rights)
assert_nil gis_access_link(links_no_rights)
end

test 'gis_access_link returns nil if links are blank' do
rights_no_links = {
'rights' => [
{
'description' => 'foo',
'kind' => 'bar'
},
{
'description' => 'no authentication required',
'kind' => 'Access to files'
}
]
}
'rights' => [
{
'description' => 'foo',
'kind' => 'bar'
},
{
'description' => 'no authentication required',
'kind' => 'Access to files'
}
]
}
assert_nil gis_access_link(rights_no_links)
end

test 'gis_access_link is website URL for non-MIT records' do
access_elsewhere = {
'rights' => [
{
'description' => 'foo',
'kind' => 'bar'
},
{
'description' => 'unknown: check with owning institution',
'kind' => 'Access to files'
}
],
'links' => [
{
'kind' => 'Download',
'text' => 'Data',
'url' => 'https://example.org/dz_f7regions_2016.zip'
},
{
'kind' => 'Website',
'text' => 'Website',
'url' => 'https://example.org/gismit:dz_f7regions_2016'
}
],
'provider' => 'Spelman'
}
'rights' => [
{
'description' => 'foo',
'kind' => 'bar'
},
{
'description' => 'unknown: check with owning institution',
'kind' => 'Access to files'
}
],
'links' => [
{
'kind' => 'Download',
'text' => 'Data',
'url' => 'https://example.org/dz_f7regions_2016.zip'
},
{
'kind' => 'Website',
'text' => 'Website',
'url' => 'https://example.org/gismit:dz_f7regions_2016'
}
],
'provider' => 'Spelman'
}
assert_equal 'https://example.org/gismit:dz_f7regions_2016', gis_access_link(access_elsewhere)
end

test 'gis_access_link is data download URL for MIT records' do
access_free = {
'rights' => [
{
'description' => 'foo',
'kind' => 'bar'
},
{
'description' => 'no authentication required',
'kind' => 'Access to files'
}
],
'links' => [
{
'kind' => 'Download',
'text' => 'Data',
'url' => 'https://example.org/dz_f7regions_2016.zip'
},
{
'kind' => 'Website',
'text' => 'Website',
'url' => 'https://example.org/gismit:dz_f7regions_2016'
}
]
}
'rights' => [
{
'description' => 'foo',
'kind' => 'bar'
},
{
'description' => 'no authentication required',
'kind' => 'Access to files'
}
],
'links' => [
{
'kind' => 'Download',
'text' => 'Data',
'url' => 'https://example.org/dz_f7regions_2016.zip'
},
{
'kind' => 'Website',
'text' => 'Website',
'url' => 'https://example.org/gismit:dz_f7regions_2016'
}
]
}
access_auth = {
'rights' => [
{
'description' => 'foo',
'kind' => 'bar'
},
{
'description' => 'MIT authentication required',
'kind' => 'Access to files'
}
],
'links' => [
{
'kind' => 'Download',
'text' => 'Data',
'url' => 'https://example.org/dz_f7regions_2016.zip'
},
{
'kind' => 'Website',
'text' => 'Website',
'url' => 'https://example.org/gismit:dz_f7regions_2016'
}
]
}
assert_equal 'https://example.org/dz_f7regions_2016.zip', gis_access_link(access_free)
assert_equal 'https://example.org/dz_f7regions_2016.zip', gis_access_link(access_auth)
'rights' => [
{
'description' => 'foo',
'kind' => 'bar'
},
{
'description' => 'MIT authentication required',
'kind' => 'Access to files'
}
],
'links' => [
{
'kind' => 'Download',
'text' => 'Data',
'url' => 'https://example.org/dz_f7regions_2016.zip'
},
{
'kind' => 'Website',
'text' => 'Website',
'url' => 'https://example.org/gismit:dz_f7regions_2016'
}
]
}
assert_equal 'https://example.org/dz_f7regions_2016.zip?timdexui=true', gis_access_link(access_free)
assert_equal 'https://example.org/dz_f7regions_2016.zip?timdexui=true', gis_access_link(access_auth)
end

test 'append_timdexui_with_no_existing_query_values' do
url = 'https://example.org/dz_f7regions_2016.zip'
assert_equal('https://example.org/dz_f7regions_2016.zip?timdexui=true',
append_timdexui(url))
end

test 'append_timdexui_with_existing_query_values' do
url = 'https://example.org/dz_f7regions_2016.zip?hallo=goodbye'
assert_equal('https://example.org/dz_f7regions_2016.zip?hallo=goodbye&timdexui=true',
append_timdexui(url))
end

test 'source_metadata_available? returns true if source metadata link exists' do
Expand Down Expand Up @@ -272,7 +284,7 @@ class RecordHelperTest < ActionView::TestCase

test 'parse_nested_field returns nil for fields that are not nested' do
string_field = 'string'
array_of_strings_field = ['string', 'other_string']
array_of_strings_field = %w[string other_string]
assert_nil parse_nested_field(string_field)
assert_nil parse_nested_field(array_of_strings_field)
end
Expand Down Expand Up @@ -304,8 +316,8 @@ class RecordHelperTest < ActionView::TestCase

test 'deduplicate_subjects returns only unique subjects' do
# within the same subject
duplicative_subject = [{ 'kind' => 'foo', 'value' => ['bar', 'bar', 'baz']}]
assert_equal [['bar', 'baz']], deduplicate_subjects(duplicative_subject)
duplicative_subject = [{ 'kind' => 'foo', 'value' => %w[bar bar baz] }]
assert_equal [%w[bar baz]], deduplicate_subjects(duplicative_subject)

# across multiple subjects
multiple_duplicative_subjects = [{ 'kind' => 'foo', 'value' => ['bar'] },
Expand All @@ -315,7 +327,7 @@ class RecordHelperTest < ActionView::TestCase

test 'deduplicate_subjects ignores case' do
# within the same subject
duplicative_subject = [{ 'kind' => 'foo', 'value' => ['Bar', 'BAR', 'bar']}]
duplicative_subject = [{ 'kind' => 'foo', 'value' => %w[Bar BAR bar] }]
assert_equal [['Bar']], deduplicate_subjects(duplicative_subject)

# across multiple subjects
Expand Down

0 comments on commit a771df4

Please sign in to comment.