Skip to content

feature: add open attachment without download #3681

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

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
+84 −2
Open

feature: add open attachment without download #3681

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
f688aa0
add open attachment without download
Nevelito Feb 21, 2025
227ba54
fix standardrb spec
Nevelito Feb 21, 2025
04add95
fix specs
Nevelito Feb 21, 2025
3a5238d
fix standardrb spec fails
Nevelito Feb 21, 2025
ff44672
delete changes in yarn lock
Nevelito Feb 21, 2025
88fc94c
request changes
Nevelito Mar 6, 2025
a759251
Merge branch 'main' into open_field_attachments
Paul-Bob Mar 13, 2025
ecc62c3
request changes
Nevelito Mar 13, 2025
7597bbe
fix bug
Nevelito Mar 13, 2025
b99dbde
fix spec
Nevelito Mar 13, 2025
c22904d
fix spec
Nevelito Mar 13, 2025
e5f1ab6
fix spec
Nevelito Mar 13, 2025
898f26b
tweak
Paul-Bob Mar 14, 2025
f77cfe3
Merge branch 'main' into open_field_attachments
Paul-Bob Mar 31, 2025
a94cc41
Merge branch 'main' into open_field_attachments
Paul-Bob Apr 16, 2025
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
7 changes: 5 additions & 2 deletions app/components/avo/fields/common/files/view_type/grid_item_component.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,14 @@
<% elsif is_video? %>
<%= video_tag(helpers.main_app.url_for(file), controls: true, preload: false, class: 'w-full') %>
<% else %>
<div class="relative flex flex-col justify-evenly items-center px-2 rounded-lg border bg-white border-gray-500 min-h-24">
<a
<% if file.representable? %>href="<%= helpers.main_app.url_for(file) %>" target="_blank" rel="noopener noreferrer"<% end %>
class="relative flex flex-col justify-evenly items-center px-2 rounded-lg border bg-white border-gray-500 min-h-24 <%= 'hover:bg-gray-100 transition' if file.representable? %>"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's important to check can_download_file? to enforce authorization properly. Otherwise, users without download permissions could still open and inspect the PDF.

>
<div class="flex flex-col justify-center items-center w-full">
<%= helpers.svg "heroicons/outline/document-text", class: 'h-10 text-gray-600 mb-2' %>
</div>
</div>
</a>
<% end %>
<% if @field.display_filename %>
<span class="text-gray-500 mt-1 text-sm truncate" title="<%= file.filename %>"><%= file.filename %></span>
Expand Down
2 changes: 2 additions & 0 deletions app/components/avo/fields/common/files/view_type/grid_item_component.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# frozen_string_literal: true

class Avo::Fields::Common::Files::ViewType::GridItemComponent < Avo::BaseComponent
include Avo::Fields::Concerns::FileAuthorization

prop :field
prop :resource
prop :file
Expand Down
Empty file added spec/dummy/app/assets/csvs/sample.csv
View file
Open in desktop
Empty file.
Empty file added spec/dummy/app/assets/pdfs/cv_sample.pdf
View file
Open in desktop
Empty file.
52 changes: 52 additions & 0 deletions spec/system/avo/open_field_attachment_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
require "rails_helper"

RSpec.describe "OpenFieldAttachment", type: :system do
let!(:user) { User.first }
let!(:cv_file) { Rails.root.join("app", "assets", "pdfs", "cv_sample.pdf") }
let!(:csv_file) { Rails.root.join("app", "assets", "csvs", "sample.csv") }
let(:path) { "/admin/resources/field_discovery_users/#{user.slug}" }

context "with PDF attachment" do
before do
user.cv.attach(io: File.open(cv_file), filename: "cv_sample.pdf", content_type: "application/pdf")
end

it "opens attachment in new window without download" do
test_open_PDF_attachment(path)
end
end

context "with CSV attachment" do
before do
user.cv.attach(io: File.open(csv_file), filename: "sample.csv", content_type: "application/csv")
end

it "can not open or download attachment in new window" do
test_open_CSV_attachment(path)
end
end

def test_open_PDF_attachment(path)
visit path

link = find('a[rel="noopener noreferrer"][target="_blank"]', visible: :all)
expect(link).to be_present
link.click

expect(page.driver.browser.current_url).not_to include("download")
expect(page.driver.browser.window_handles.length).to eq 2
end

def test_open_CSV_attachment(path)
visit path

link = first('a.relative', visible: :all)

Check failure on line 43 in spec/system/avo/open_field_attachment_spec.rb

View workflow job for this annotation

GitHub Actions / lint / runner / standardrb 

[rubocop] reported by reviewdog 🐶
[Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

Raw Output:
spec/system/avo/open_field_attachment_spec.rb:43:18: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
    link = first('a.relative', visible: :all)
                 ^^^^^^^^^^^^

expect(link).to be_present
expect(link[:target]).to eq("")
expect(link[:rel]).to eq("")

expect(page.driver.browser.current_url).not_to include("download")
expect(page.driver.browser.window_handles.length).to eq 1
end
end
Loading