Skip to content

Commit

Permalink
dm-4338 persistent file links pagebuilder (#742)
Browse files Browse the repository at this point in the history
* remove old js handling async fetching of presigned url for file download links

* remove `after_sign_in` method override

* add shared `unauthorized_response` method to `ApplicationController`

* edit `PracticeResourcesController`

remove `download_and_redirect` action

remove `unauthorized_response` helper method, now lives in `ApplicationController`

remove set_download_session

now returns unauthorized response when trying to access va-user-only resource

* remove `download_and_redirect` view

* add `PageResourceController`

handles requests for downloadable resources found on page builder pages

* edit routs

removes `practice_resources` route

adds route for `PageResourcesController#download`

* edit file links on pagebuilder page views

uses new path helper pointing to `PageResourcesController#download`

* update / add factory_bot factories

* update / add specs for static resource links
  • Loading branch information
PhilipDeFraties authored Dec 8, 2023
1 parent f28dc1f commit 5f363d2
Show file tree
Hide file tree
Showing 15 changed files with 185 additions and 197 deletions.
19 changes: 0 additions & 19 deletions app/assets/javascripts/_page_show.es6
Original file line number Diff line number Diff line change
Expand Up @@ -105,31 +105,12 @@ const COMPONENT_CLASSES = [
}
}

function fetchPageComponentFileResources() {
const pageComponents = document.querySelectorAll('.page-downloadable-file-component, .page-publication-component');
// Replace each 'PageDownloadableFileComponent or PagePublicationComp;onent's 'href' with a new signed URL
pageComponents.forEach(element => {
const filePath = element.getAttribute('data-resource-path');
const fileUrl = element.href;
const fileId = element.getAttribute('data-resource-id');

fetchSignedResource(filePath, fileUrl)
.then(signedUrl => {
const fileElement = document.querySelector(`a[data-resource-id="${fileId}"]`);
if (fileElement) {
fileElement.href = signedUrl;
}
});
});
}

function execPageBuilderFunctions() {
browsePageBuilderPageHappy();
containerizeSubpageHyperlinkCards();
remediateInternalLinksTarget();
identifyExternalLinks();
chromeWorkaroundForAnchorTags();
fetchPageComponentFileResources();
replaceImagePlaceholders();
}

Expand Down
13 changes: 9 additions & 4 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,6 @@ def authenticate_active_admin_user!
end
end

def after_sign_in_path_for(resource)
session[:download_info] ? download_and_redirect_practice_resources_path : super
end

def signed_resource
# In order to circumvent making a request to AWS for tests, we can return the Paperclip attachment's 'url'.
# If there isn't one, the default value is set to an empty string.
Expand All @@ -59,6 +55,15 @@ def signed_resource
render plain: "An error occurred: #{e.message}", status: :internal_server_error
end

def unauthorized_response
respond_to do |format|
warning = 'You are not authorized to view this content.'
flash[:warning] = warning
format.html { redirect_to '/', warning: warning }
format.json { render warning: warning }
end
end

protected

private
Expand Down
46 changes: 46 additions & 0 deletions app/controllers/page_resources_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
class PageResourcesController < ApplicationController
before_action :authenticate_user!, except: [:download]
before_action :set_page_resource

def download
if @page.published
handle_published_page
elsif current_user
handle_user_with_unpublished_practice
else
unauthorized_response
end
end

private

def set_page_resource
@page = Page.find_by(id: resource_params[:page_id])

if @page
@page_resource = @page.page_components.find_by(component_id: resource_params[:id]).component
else
render plain: "Page not found", status: :not_found and return
end
end

def resource_params
params.permit(:page_id, :id)
end

def handle_published_page
if @page.is_public || current_user
redirect_to @page_resource.attachment_s3_presigned_url
else
unauthorized_response
end
end

def handle_user_with_unpublished_practice
if current_user&.has_role?(:admin)
redirect_to @page_resource.attachment_s3_presigned_url
else
unauthorized_response
end
end
end
25 changes: 2 additions & 23 deletions app/controllers/practice_resources_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,13 @@ class PracticeResourcesController < ApplicationController
def download
if practice_published_approved_enabled?
handle_enabled_practice
elsif current_user && !practice_published_approved_enabled?
elsif current_user
handle_user_with_disabled_practice
else
unauthorized_response
end
end

def download_and_redirect
end

private

def set_practice_resource
Expand Down Expand Up @@ -48,8 +45,7 @@ def handle_enabled_practice
if @practice.is_public || current_user
redirect_to @practice_resource.attachment_s3_presigned_url
else
set_download_session
authenticate_user!
unauthorized_response
end
end

Expand All @@ -65,23 +61,6 @@ def check_user_practice_permissions
@practice.user_id == current_user.id || current_user&.roles.any? || is_user_an_editor_for_practice(@practice, current_user)
end

def unauthorized_response
respond_to do |format|
warning = 'You are not authorized to view this content.'
flash[:warning] = warning
format.html { redirect_to '/', warning: warning }
format.json { render warning: warning }
end
end

def set_download_session
session[:download_info] = {
download_url: @practice_resource.attachment_s3_presigned_url,
practice_path: practice_path(@practice_resource.practice),
practice_name: @practice_resource.practice.name
}
end

def practice_published_approved_enabled?
@practice.published && @practice.approved && @practice.enabled
end
Expand Down
12 changes: 5 additions & 7 deletions app/views/page/_publications_list_cards.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,11 @@
<div class="usa-card__header">
<h3 class="publication-title">
<% if publication.attachment? %>
<%= link_to publication.attachment_s3_presigned_url,
<% page_component = publication.page_component %>
<%= link_to page_resource_download_path(id: page_component.component_id, page_id: page_component.page.id.to_s),
class: 'usa-link',
target: '_blank',
download: publication.attachment_file_name,
'data-resource-id': publication.id,
'data-resource-path': publication.attachment.path do %>
<i class="far fa-file margin-right-1"></i><%=publication.title %><span class="sr-only">Download <%= publication.attachment_file_name %></span>
target: '_blank' do %>
<i class="far fa-file margin-right-1"></i><%=publication.title %><span class="usa-sr-only">Download <%= publication.attachment_file_name %></span>
<% end %>
<% elsif publication.url? %>
<%= link_to(publication.title, publication.url, class: set_link_classes(publication.url), target: get_link_target_attribute(publication.url)) %>
Expand All @@ -33,4 +31,4 @@
</div>
</div>
</li>
<% end %>
<% end %>
14 changes: 6 additions & 8 deletions app/views/page/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -328,14 +328,12 @@
<div class="grid-container">
<div class="grid-row <%= page_narrow_classes %> <%= next_component != nil && next_component.component_type == pc.component_type ? 'margin-bottom-1' : 'margin-bottom-3 ' %> font-sans-sm line-height-162<%= ' margin-bottom-0' if last_component === index %>">
<%= content_tag(:span, description, class: 'margin-right-1') if description != '' %>
<%= link_to component.attachment_s3_presigned_url,
class: 'usa-link usa-link--external page-downloadable-file-component',
target: '_blank',
download: display_name != '' ? display_name : component.attachment_file_name,
'data-resource-id': pc.component_id,
'data-resource-path': pc.component.attachment.path do %>
<i class="far fa-file margin-right-1"></i><%= display_name != '' ? display_name : component.attachment_file_name %>
<% end %>
<%= link_to page_resource_download_path(page_id: @page.id.to_s, id: component.id),
class: 'usa-link usa-link--external page-downloadable-file-component',
target: '_blank' do
%>
<i class="far fa-file margin-right-1"></i><%= display_name != '' ? display_name : component.attachment_file_name %>
<%end%>
</div>
</div>

Expand Down
38 changes: 0 additions & 38 deletions app/views/practice_resources/download_and_redirect.html.erb

This file was deleted.

7 changes: 2 additions & 5 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
Rails.application.routes.draw do
require "./lib/constraints/route_constraints"
get ':page_group_friendly_id' => 'page#show', constraints: PageGroupHomeConstraint
get ':page_group_friendly_id/:page_slug' => 'page#show', constraints: PageGroupConstraint
get ':page_group_friendly_id/:page_slug' => 'page#show', as: 'page_show', constraints: PageGroupConstraint
get 'page_resources/:id/download' => 'page_resources#download', as: 'page_resource_download'
ActiveAdmin.routes(self)
devise_for :users, controllers: { registrations: 'registrations' }
devise_scope :user do
Expand Down Expand Up @@ -59,10 +60,6 @@
end
end

resources :practice_resources, only: [] do
get 'download_and_redirect', on: :collection
end

# old practice routes redirects
get '/practices/:id', to: redirect('/innovations/%{id}', status: 302)
get '/practices/:id/edit/metrics', to: redirect('/innovations/%{id}/edit/metrics', status: 302)
Expand Down
15 changes: 7 additions & 8 deletions spec/factories/page_components.rb
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
FactoryBot.define do
factory :page_component do
association :page
position { [1, 2, 3, 4, 5].sample }
component_type { 'PageDownloadableFileComponent' }
component_id { SecureRandom.uuid }

transient do
component_type { nil }
component_attributes { {} }
end

component do
if component_type
association(component_type.underscore.to_sym, **component_attributes)
trait :with_downloadable_file_component do
after(:build) do |page_component|
downloadable_file_component = build(:page_downloadable_file_component)
page_component.component = downloadable_file_component
end
end
end
Expand Down
36 changes: 36 additions & 0 deletions spec/factories/page_downloadable_file_components.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
FactoryBot.define do
factory :page_downloadable_file_component do
display_name { "Sample File" }
description { "This is a sample file for download." }

transient do
file_type { "pdf" }
end

attachment_file_name do
if file_type == "docx"
"dummy.docx"
else
"dummy.pdf"
end
end

attachment_content_type do
if file_type == "pdf"
"application/pdf"
elsif file_type == "docx"
"application/vnd.openxmlformats-officedocument.wordprocessingml.document"
else
"application/pdf"
end
end

attachment_file_size { File.size(Rails.root.join('spec', 'assets', attachment_file_name)) }
attachment_updated_at { Time.current }

after(:build) do |page_downloadable_file_component|
file_name = page_downloadable_file_component.attachment_file_name
page_downloadable_file_component.attachment = File.new(Rails.root.join('spec', 'assets', file_name))
end
end
end
8 changes: 4 additions & 4 deletions spec/factories/pages.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
sequence(:description) { |n| "Page Description #{n}" }
sequence(:slug) { |n| "page-slug-#{n}" }

created_at { Time.now }
updated_at { Time.now }
published { Time.now }
ever_published { false }
created_at { Date.today }
updated_at { Date.today }
published { Date.today }
ever_published { true }
is_visible { true }
template_type { 0 }
is_public { false }
Expand Down
6 changes: 6 additions & 0 deletions spec/factories/users.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,10 @@
confirmed_at { Time.now }
accepted_terms { true }
end

trait :admin do
after(:create) do |user|
user.add_role :admin
end
end
end
Loading

0 comments on commit 5f363d2

Please sign in to comment.