Skip to content

Commit

Permalink
Download links target _blank and append UI marker
Browse files Browse the repository at this point in the history 
Why are these changes being introduced:

* We want to display a message in our restricted CDN auth flow that
  instructs a user to close the window/tab when the download is complete
  to return to their search if they came from TIMDEX UI.

Relevant ticket(s):

* https://mitlibraries.atlassian.net/browse/GDT-317
* https://mitlibraries.atlassian.net/browse/GDT-304

How does this address that need:

* Adds `target=_blank` to download link tags
* appends `?timdex-ui=true` to download link URLs

Document any side effects to this change:

* A bunch of tests reformatted automatically on save. The only relevant
  changes to the tests were on lines 246-247
  • Loading branch information
@JPrevost
JPrevost committed May 20, 2024
1 parent e616f95 commit a11558c
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 121 deletions.
3 changes: 2 additions & 1 deletion app/helpers/record_helper.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ def gis_access_link(metadata)
if access_type(metadata) == 'unknown: check with owning institution'
links.select { |link| link['kind'] == 'Website' }.first['url']
else
links.select { |link| link['kind'] == 'Download' && link['text'] == 'Data' }.first['url']
url = links.select { |link| link['kind'] == 'Download' && link['text'] == 'Data' }.first['url']
"#{url}?timdexui=true"
end
end

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
238 changes: 119 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,148 @@ 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 'source_metadata_available? returns true if source metadata link exists' do
Expand Down Expand Up @@ -272,7 +272,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 +304,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 +315,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 a11558c

Please sign in to comment.