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

Jump to bottom

Download links target _blank and append UI marker #192

Merged
merged 2 commits into from
May 24, 2024
Merged

Download links target _blank and append UI marker #192

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a11558c
Download links target _blank and append UI marker
JPrevost May 20, 2024
54a2e46
Handle URLs that may already have query params
JPrevost May 24, 2024
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
1 change: 1 addition & 0 deletions Gemfile.lock
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading