Skip to content

Commit

Permalink
Rename Attachment#urlname into slug (#1848)
Browse files Browse the repository at this point in the history
And deprecate urlname, because it is a not very well known name.
  • Loading branch information
tvdeyen authored Jul 17, 2020
1 parent 173d3ee commit 96fb431
Show file tree
Hide file tree
Showing 9 changed files with 29 additions and 26 deletions.
2 changes: 1 addition & 1 deletion app/controllers/alchemy/admin/pages_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ def destroy

def link
@attachments = Attachment.all.collect { |f|
[f.name, download_attachment_path(id: f.id, name: f.urlname)]
[f.name, download_attachment_path(id: f.id, name: f.slug)]
}
end

Expand Down
4 changes: 2 additions & 2 deletions app/helpers/alchemy/url_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ def show_page_path_params(page, optional_params = {})

# Returns the path for downloading an alchemy attachment
def download_alchemy_attachment_path(attachment)
alchemy.download_attachment_path(attachment, attachment.urlname)
alchemy.download_attachment_path(attachment, attachment.slug)
end

# Returns the url for downloading an alchemy attachment
def download_alchemy_attachment_url(attachment)
alchemy.download_attachment_url(attachment, attachment.urlname)
alchemy.download_attachment_url(attachment, attachment.slug)
end

# Returns the full url containing host, page and anchor for the given element
Expand Down
5 changes: 4 additions & 1 deletion app/models/alchemy/attachment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,13 @@ def to_jq_upload
end

# An url save filename without format suffix
def urlname
def slug
CGI.escape(file_name.gsub(/\.#{extension}$/, "").tr(".", " "))
end

alias_method :urlname, :slug
deprecate urlname: :slug, deprecator: Alchemy::Deprecation

# Checks if the attachment is restricted, because it is attached on restricted pages only
def restricted?
pages.any? && pages.not_restricted.blank?
Expand Down
2 changes: 1 addition & 1 deletion app/models/alchemy/essence_file.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def attachment_url

routes.download_attachment_path(
id: attachment.id,
name: attachment.urlname,
name: attachment.slug,
format: attachment.suffix,
)
end
Expand Down
2 changes: 1 addition & 1 deletion app/views/alchemy/essences/_essence_file_view.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
attachment.name,
alchemy.download_attachment_path(
attachment,
name: attachment.urlname,
name: attachment.slug,
format: attachment.suffix
),
{
Expand Down
18 changes: 9 additions & 9 deletions spec/helpers/alchemy/url_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ module Alchemy

context "with addiitonal parameters" do
subject(:show_page_path_params) do
helper.show_page_path_params(page, {query: "test"})
helper.show_page_path_params(page, { query: "test" })
end

it "returns a Hash with urlname, no locale and query parameter" do
Expand All @@ -52,7 +52,7 @@ module Alchemy

context "with additional parameters" do
subject(:show_page_path_params) do
helper.show_page_path_params(page, {query: "test"})
helper.show_page_path_params(page, { query: "test" })
end

it "returns a Hash with urlname, locale and query parameter" do
Expand All @@ -74,7 +74,7 @@ module Alchemy
end

it "should return the correct relative path string with additional parameters" do
expect(helper.show_alchemy_page_path(page, {query: "test"})).to \
expect(helper.show_alchemy_page_path(page, { query: "test" })).to \
eq("/#{page.language_code}/testpage?query=test")
end
end
Expand All @@ -89,7 +89,7 @@ module Alchemy
end

it "should return the correct relative path string with additional parameter" do
expect(helper.show_alchemy_page_path(page, {query: "test"})).to \
expect(helper.show_alchemy_page_path(page, { query: "test" })).to \
eq("/testpage?query=test")
end
end
Expand All @@ -107,7 +107,7 @@ module Alchemy
end

it "should return the correct url string with additional parameters" do
expect(helper.show_alchemy_page_url(page, {query: "test"})).to \
expect(helper.show_alchemy_page_url(page, { query: "test" })).to \
eq("http://#{helper.request.host}/#{page.language_code}/testpage?query=test")
end
end
Expand All @@ -123,24 +123,24 @@ module Alchemy
end

it "should return the correct url string with additional parameter" do
expect(helper.show_alchemy_page_url(page, {query: "test"})).to \
expect(helper.show_alchemy_page_url(page, { query: "test" })).to \
eq("http://#{helper.request.host}/testpage?query=test")
end
end
end
end

context "attachment path helpers" do
let(:attachment) { mock_model(Attachment, urlname: "test-attachment.pdf") }
let(:attachment) { mock_model(Attachment, slug: "test-attachment.pdf") }

it "should return the correct relative path to download an attachment" do
expect(helper.download_alchemy_attachment_path(attachment)).to \
eq("/attachment/#{attachment.id}/download/#{attachment.urlname}")
eq("/attachment/#{attachment.id}/download/#{attachment.slug}")
end

it "should return the correct url to download an attachment" do
expect(helper.download_alchemy_attachment_url(attachment)).to \
eq("http://#{helper.request.host}/attachment/#{attachment.id}/download/#{attachment.urlname}")
eq("http://#{helper.request.host}/attachment/#{attachment.id}/download/#{attachment.slug}")
end
end

Expand Down
8 changes: 4 additions & 4 deletions spec/models/alchemy/attachment_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,22 +54,22 @@ module Alchemy
describe "urlname sanitizing" do
it "escapes unsafe url characters" do
attachment.file_name = "f#%&cking cute kitten pic.png"
expect(attachment.urlname).to eq("f%23%25%26cking+cute+kitten+pic")
expect(attachment.slug).to eq("f%23%25%26cking+cute+kitten+pic")
end

it "removes format suffix from end of file name" do
attachment.file_name = "pic.png.png"
expect(attachment.urlname).to eq("pic+png")
expect(attachment.slug).to eq("pic+png")
end

it "converts dots into escaped spaces" do
attachment.file_name = "cute.kitten.pic.png"
expect(attachment.urlname).to eq("cute+kitten+pic")
expect(attachment.slug).to eq("cute+kitten+pic")
end

it "escapes umlauts in the name" do
attachment.file_name = "süßes katzenbild.png"
expect(attachment.urlname).to eq("s%C3%BC%C3%9Fes+katzenbild")
expect(attachment.slug).to eq("s%C3%BC%C3%9Fes+katzenbild")
end
end

Expand Down
2 changes: 1 addition & 1 deletion spec/models/alchemy/essence_file_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ module Alchemy
subject { essence.attachment_url }

it "returns the download attachment url." do
is_expected.to match(/\/attachment\/#{attachment.id}\/download\/#{attachment.urlname}\.#{attachment.suffix}/)
is_expected.to match(/\/attachment\/#{attachment.id}\/download\/#{attachment.slug}\.#{attachment.suffix}/)
end

context "without attachment assigned" do
Expand Down
12 changes: 6 additions & 6 deletions spec/views/essences/essence_file_view_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
build_stubbed(:alchemy_attachment, file: file, name: "an image", file_name: "image with spaces.png")
end

let(:essence) { Alchemy::EssenceFile.new(attachment: attachment) }
let(:content) { Alchemy::Content.new(essence: essence) }
let(:essence) { Alchemy::EssenceFile.new(attachment: attachment) }
let(:content) { Alchemy::Content.new(essence: essence) }

context "without attachment" do
let(:essence) { Alchemy::EssenceFile.new(attachment: nil) }
Expand All @@ -27,7 +27,7 @@
it "renders a link to download the attachment" do
render content, content: content
expect(rendered).to have_selector(
"a[href='/attachment/#{attachment.id}/download/#{attachment.urlname}.#{attachment.suffix}']",
"a[href='/attachment/#{attachment.id}/download/#{attachment.slug}.#{attachment.suffix}']"
)
end

Expand All @@ -40,14 +40,14 @@

context "with link_text set in the local options" do
it "has this value as link text" do
render content, content: content, options: {link_text: "Download this file"}
render content, content: content, options: { link_text: "Download this file" }
expect(rendered).to have_selector("a:contains('Download this file')")
end
end

context "with link_text set in the content settings" do
before do
allow(content).to receive(:settings) { {link_text: "Download this file"} }
allow(content).to receive(:settings) { { link_text: "Download this file" } }
end

it "has this value as link text" do
Expand All @@ -69,7 +69,7 @@

context "with html_options given" do
it "renders the linked ingredient with these options" do
render content, content: content, html_options: {title: "Bar", class: "blue"}
render content, content: content, html_options: { title: "Bar", class: "blue" }
expect(rendered).to have_selector('a.blue[title="Bar"]')
end
end
Expand Down

0 comments on commit 96fb431

Please sign in to comment.