Skip to content
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.

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

Change ajax? helper to check only XMLHttpRequest presence #1134

Merged
merged 1 commit into from
May 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 3 additions & 8 deletions spec/lucky/mime_type_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ include ContextHelper

describe Lucky::MimeType do
describe "determine_clients_desired_format" do
it "returns the format for the 'Accept' header if it is an AJAX request" do
it "returns the format for the 'Accept' header" do
format = determine_format("accept": "application/json", "X-Requested-With": "XmlHttpRequest")
format.should eq(:json)
end
Expand All @@ -24,12 +24,7 @@ describe Lucky::MimeType do
format.should eq(:csv)
end

it "returns :ajax if it is an AJAX request and no 'Accept' header is given" do
format = determine_format("X-Requested-With": "XmlHttpRequest")
format.should eq(:ajax)
end

describe "when the 'Accept' header is the default browser header, and it is not an AJAX request" do
describe "when the 'Accept' header is the default browser header" do
it "returns :html if :html is an accepted format" do
default_browser_header = "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8"
format = determine_format(default_format: :csv, headers: {"accept": default_browser_header}, accepted_formats: [:html, :csv])
Expand All @@ -43,7 +38,7 @@ describe Lucky::MimeType do
end
end

it "falls back to 'default_format' if no accept header and not AJAX" do
it "falls back to 'default_format' if no accept header" do
format = determine_format(default_format: :csv)
format.should eq(:csv)
end
Expand Down
13 changes: 8 additions & 5 deletions spec/lucky/request_type_helper_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,6 @@ describe Lucky::RequestTypeHelpers do
override_format :foo, &.html?.should(be_false)
end

it "checks if client accepts AJAX" do
override_format :ajax, &.ajax?.should(be_true)
override_format :foo, &.ajax?.should(be_false)
end

it "checks if client accepts XML" do
override_format :xml, &.xml?.should(be_true)
override_format :foo, &.xml?.should(be_false)
Expand All @@ -63,6 +58,14 @@ describe Lucky::RequestTypeHelpers do
override_format :plain_text, &.plain_text?.should(be_true)
override_format :foo, &.plain_text?.should(be_false)
end

it "checks if request send via AJAX" do
action = FakeAction.new
action.ajax?.should(be_false)

action.context.request.headers["X-Requested-With"] = "XMLHttpRequest"
action.ajax?.should(be_true)
end
end

private def override_format(format : Symbol?)
Expand Down
10 changes: 1 addition & 9 deletions src/lucky/mime_type.cr
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,6 @@ class Lucky::MimeType

if usable_accept_header? && accept
from_accept_header(accept)
elsif ajax?
:ajax
elsif accepts_html? && default_accept_header_that_browsers_send?
:html
else
Expand All @@ -79,9 +77,7 @@ class Lucky::MimeType
end

private def usable_accept_header? : Bool
!!(accept_header && !default_accept_header_that_browsers_send?) ||
# If an Ajax request, it should still be possible to accept a different format
!!(accept_header && ajax?)
!!(accept_header && !default_accept_header_that_browsers_send?)
end

private def accept_header : String?
Expand All @@ -101,9 +97,5 @@ class Lucky::MimeType

!!accept && !!(accept =~ /,\s*\*\/\*|\*\/\*\s*,/)
end

private def ajax? : Bool
request.headers["X-Requested-With"]?.try(&.downcase) == "xmlhttprequest"
end
end
end
2 changes: 1 addition & 1 deletion src/lucky/redirectable.cr
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ module Lucky::Redirectable
# ```
# Note: It's recommended to use the method above that accepts a human friendly version of the status
def redirect(to path : String, status : Int32 = 302) : Lucky::TextResponse
if request.headers["X-Requested-With"]?.try(&.downcase) == "xmlhttprequest" && request.method != "GET"
if ajax? && request.method != "GET"
context.response.headers.add "Location", path

# do not enable form disabled elements for XHR redirects, see https://github.com/rails/rails/pull/31441
Expand Down
14 changes: 7 additions & 7 deletions src/lucky/request_type_helpers.cr
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,6 @@ module Lucky::RequestTypeHelpers
accepts?(:json)
end

# Check if the request is AJAX
#
# This tests if the `X-Requested-With` header is `XMLHttpRequest`
def ajax? : Bool
accepts?(:ajax)
end

# Check if the request is HTML
#
# Browsers typically send vague Accept headers. Because of that this will return `true` when:
Expand All @@ -93,6 +86,13 @@ module Lucky::RequestTypeHelpers
accepts?(:plain_text)
end

# Check if the request is AJAX
#
# This tests if the `X-Requested-With` header is `XMLHttpRequest`
def ajax? : Bool
request.headers["X-Requested-With"]?.try(&.downcase) == "xmlhttprequest"
end

# :nodoc:
def plain?
{% raise "This method has been renamed to 'plain_text?'" %}
Expand Down