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 how we handle Accepts and Content-Type #822

Closed
paulcsmith opened this issue Jun 20, 2019 · 0 comments · Fixed by #869
Closed

Change how we handle Accepts and Content-Type #822

paulcsmith opened this issue Jun 20, 2019 · 0 comments · Fixed by #869
Assignees
Milestone

Comments

@paulcsmith
Copy link
Member

paulcsmith commented Jun 20, 2019

Right now the param parser correctly parses the Content-Type

lucky/src/lucky/params.cr

Lines 384 to 394 in b5d6056

private def json?
content_type.try(&.match(/^application\/json/))
end
private def multipart?
content_type.try(&.match(/^multipart\/form-data/))
end
private def content_type : String?
request.headers["Content-Type"]?
end

But when check what the request wants in return we incorrectly use Content-Type but we should instead use Accept.

I think we can keep the same html?|json?|etc methods, but they should all use the Accept header instead of Content-Type

Prior art

@paulcsmith paulcsmith added this to the Lucky next milestone Jul 11, 2019
@paulcsmith paulcsmith self-assigned this Aug 8, 2019
paulcsmith added a commit that referenced this issue Aug 9, 2019
paulcsmith added a commit that referenced this issue Aug 10, 2019
paulcsmith added a commit that referenced this issue Aug 10, 2019
paulcsmith added a commit that referenced this issue Aug 10, 2019
paulcsmith added a commit that referenced this issue Aug 11, 2019
paulcsmith added a commit that referenced this issue Aug 12, 2019
paulcsmith added a commit that referenced this issue Aug 12, 2019
paulcsmith added a commit that referenced this issue Aug 12, 2019
paulcsmith added a commit that referenced this issue Aug 12, 2019
paulcsmith added a commit that referenced this issue Aug 12, 2019
paulcsmith added a commit that referenced this issue Aug 12, 2019
paulcsmith added a commit that referenced this issue Aug 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant