-
-
Notifications
You must be signed in to change notification settings - Fork 111
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
Added default headers option #88
Conversation
@@ -26,6 +26,8 @@ def finish | |||
|
|||
if _requires_no_body? | |||
@_body = nil | |||
else | |||
@headers.merge!(configuration.default_headers) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this else for merging the default headers if the method isn't HEAD or the status code isn't HTTP_STATUSES_WITHOUT_BODY defined above
end | ||
end | ||
end | ||
|
||
describe 'duplicate' do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add this to duplicate
and reset!
tests? TY!
@AlfonsoUceda This looks really good. I left one comment for extra tests for Quoting #87 :
This is still missing and it's really important for resource based headers. class MyAction
include Lotus::Action
# Default header in configuration is "Content-Security-Policy" => "script-src 'self'"
def call(params)
# ...
headers['Content-Security-Policy'] = "script-src 'self' https://api.example.org"
end
end The header specified in the action should be the output. |
@@ -26,6 +45,7 @@ def finish | |||
|
|||
if _requires_no_body? | |||
@_body = nil | |||
@headers.reject! {|header,_| !ENTITY_HEADERS.include?(header) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick, add a space after {
except above comments, 👍 to go |
@AlfonsoUceda Thank you, merged 😉 |
…ers with nil values, safely duplicate this setting. Ref #88
closes #87