Fix options headers

[karlfreeman]

Having been digging through a couple issues I realised I'm not the only one struggling to get Grape to work correctly with CORS.

I've taken a stab at fixing up the add_head_not_allowed_methods logic which seems to be passing the test suite still. However looking at some of the discussions surrounding the logic behind this method I'm not sold on it being the 'best' way to do this.

WIP 👍 Feedback appreciated

[karlfreeman]

cc #414, #422 and #406

[dblock]

This is very good, I don't know whether it's the most elegant fix, but it solves at least some of the issues. Do you think you could go through those issues, pull out any specs/bugs that are discussed and add them here? Notably #406 has one. Also please update CHANGELOG. I would merge anything that lets us close those issues and we can iterate on the implementation later.

[karlfreeman]

I couldn't agree more about the elegance of this one. However I didn't want to bite off too much with this change as its a sweeping change.

I'll clean up the commit with a CHANGELOG entry, the previous commit already had #406's test so should be good to go.

[dblock]

I mean you should merge that test in #406, that was PRed but not merged. This pull request is also conflicting, please merge from upstream.

[karlfreeman]

There shouldn't be any conflicts anymore.

I'm not sure how #406's test is relevant? I might be missing something but surely setting a header within a GET method should not effect a totally different method (eg the OPTIONS) request's headers.

In the tests I wrote, I add a header before all requests which you would then assume would be passed through the OPTIONS request's headers.

Is #406's test the desired behaviour?

[dblock]

Check https://travis-ci.org/intridea/grape/builds/13515348, the Travis build failed with a few style issues (we run Rubocop).

You're probably right about #406, I'll have to think about it. As is this PR is perfectly mergeable, thanks.

[karlfreeman]

Sorry, didn't notice the failed build as the the first two timed out.

Formatting fixed. 👍 on test suite failing on formatting, wish more libs did this.

[dblock]

Merged.