Navigation Menu

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

Make cookie key environment dependent #15804

Closed
wants to merge 1 commit into from

Conversation

ryana
Copy link
Contributor

@ryana ryana commented Jun 18, 2014

More than once I've been bitten by cookie issues when testing between staging and production environment. This change will prevent a lot of hair-pulling on new applications.

More than once I've been bitten by cookie issues when testing between staging and production environment. This change will prevent a lot of hair-pulling.
@rafaelfranca
Copy link
Member

Yes. I had this problem too. I just not sure if would not better to set this configuration in the environment files instead of a initializer now.

@jeremy @matthewd WDYT?

@eileencodes
Copy link
Member

@ryana looks like you have some test failures related to this change.

  1) Failure:
AppGeneratorTest#test_application_name_is_detected_if_it_exists_and_app_folder_renamed [test/generators/app_generator_test.rb:125]:
Expected /_myapp_session/ to match "# Be sure to restart your server when you modify this file.\n\nRails.application.config.session_store :cookie_store, key: \"_myapp_\#{Rails.env}_session\"\n".

@ryana
Copy link
Contributor Author

ryana commented Jun 18, 2014

@rafaelfranca I had that thought too, but didn't want to rock the boat too much ;)

@eileencodes whoops sorry. I'll push a fix this afternoon!

@repinel
Copy link
Member

repinel commented Aug 18, 2015

@rafaelfranca Any thoughts if this should be moved to the environments files? It seems more reasonable then having the key generated on the fly based on the environment.

@rafaelfranca
Copy link
Member

I think this is fine as is right now. @ryana do you still have interest on fixing the tests and rebasing the branch?

@rails-bot
Copy link

rails-bot bot commented Dec 18, 2019

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

@rails-bot rails-bot bot unassigned sgrif Dec 18, 2019
@rails-bot rails-bot bot added the stale label Dec 18, 2019
@rails-bot rails-bot bot closed this Dec 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants