Download links target _blank and append UI marker

[JPrevost]

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

Developer

Accessibility

• ANDI or WAVE has been run in accordance to our guide.
• This PR contains no changes to the view layer.
• New issues flagged by ANDI or WAVE have been resolved.
• New issues flagged by ANDI or WAVE have been ticketed (link in the Pull Request details above).
• No new accessibility issues have been flagged.

New ENV

• All new ENV is documented in README.
• All new ENV has been added to Heroku Pipeline, Staging and Prod.
• ENV has not changed.

Approval beyond code review

• UXWS/stakeholder approval has been confirmed.
• UXWS/stakeholder review will be completed retroactively.
• UXWS/stakeholder review is not needed.

Additional context needed to review

The download links for restricted MIT downloads should now pop into a new window/tab. The messaging for the CDN auth app has not been updated to inform the user to close the window to return to the search, but that will be added soon.

All download links (restricted and not) should get ?timdexui=true appended.

Links to metadata or external sites should not be affected.

Code Reviewer

Code

• I have confirmed that the code works as intended.
• Any CodeClimate issues have been fixed or confirmed as
  added technical debt.

Documentation

• The commit message is clear and follows our guidelines
  (not just this pull request message).
• The documentation has been updated or is unnecessary.
• New dependencies are appropriate or there were no changes.

Testing

• There are appropriate tests covering any new functionality.
• No additional test coverage is required.

[coveralls]

Pull Request Test Coverage Report for Build 9224571738

Details

• 11 of 11 (100.0%) changed or added relevant lines in 1 file are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage decreased (-0.1%) to 98.485%

---

Files with Coverage Reduction	New Missed Lines	%
app/controllers/search_controller.rb	1	98.11%

Totals
Change from base Build 9085692261:	-0.1%
Covered Lines:	585
Relevant Lines:	594

---

💛 - Coveralls

[matt-bernhardt]

This all looks good to me, and I've confirmed that the one relevant test makes sense with this change to the helper and the template. I do have a question about whether any of the URLs passing through this system already have querystrings - in which case I think more complex logic might be needed in the helper.

If that's not in our data (and isn't likely to be in the future), then I'm fine with all of this and will switch this to an approval.

[matt-bernhardt]

My only question is whether there's going to be any of these links that already have a querystring in them, which would result in something like

https://cdn.dev1.mitlibrary.net/geo/public/US_MA_P3STATIONS_2004.zip?foo=bar?timdexui=true

[JPrevost]

Great question, and I did think about this and decided that since these links should only be for resources in our buckets which should never have query strings in them this would be okay.

However, we could probably parse the URL and append the query string more safely without a ton of work. Let me look at what that would entail before moving forward.

[matt-bernhardt]

That looks good - I appreciate having those additional helper methods in place here, because that would have been a lot to parse without them.