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

Ensure cookies from incoming request are passed to Grover via Middleware #63

Merged
merged 5 commits into from
Jun 22, 2020

Conversation

braindeaf
Copy link
Contributor

I came across this situation this week, where I was loading a page that had javascript based calendar which loaded additional resources. While the page loaded the JSON that populated the calendar did not because the AJAX calls to fetch data did not have the cookies it needed.

This just reuses the example of re-sending cookies, but it puts it into the middleware.

Copy link
Contributor

@abrom abrom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a reasonable idea @braindeaf however I'm not super keen on rolling a custom cookie parser. Suggest look at using something like Rack::Utils.parse_cookies_header as used by Rack::Request. ie

cookies = Rack::Utils.parse_cookies_header(env[Rack::HTTP_COOKIE]).map do |key, value|
  { name: key, value: value, domain: env[Rack::HTTP_HOST] }
end

@braindeaf
Copy link
Contributor Author

Noted, will make appropriate changes :)

Copy link
Contributor

@abrom abrom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change looks fine, although the build is failing because of the middleware file length being too long.

As a temporary measure, update https://github.com/Studiosity/grover/blob/main/.rubocop.yml#L26 to 140

Also, can you please add an entry in the unreleased section of CHANGELOG.md

syntax changes
Add to Unreleased CHANGELOG
@braindeaf
Copy link
Contributor Author

Done. I'm getting some rubocop errors with bundled Rubocop 0.81. I didn't want to go and upgrade it in this pull, but happy to if need be.

Error: unrecognized cop Layout/SpaceAroundMethodCallOperator found in .rubocop.yml, unrecognized cop Style/ExponentialNotation found in .rubocop.yml

@abrom
Copy link
Contributor

abrom commented Jun 22, 2020

No worries, I'll look at those in a separate PR

@abrom abrom merged commit 94390ad into Studiosity:main Jun 22, 2020
@abrom
Copy link
Contributor

abrom commented Jun 23, 2020

FYI I've released this in 0.12.2. Thanks for the PR 👍

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 this pull request may close these issues.

2 participants