#5434 Small generator issue for generator in config/initializers/session_store.rb (Rails 3.0.0.rc2) - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#5434 ✓resolved

Small generator issue for generator in config/initializers/session_store.rb (Rails 3.0.0.rc2)

Reported by Willem van Bergen | August 24th, 2010 @ 10:14 AM

When running rake rails:update using Rails 3.0.0.rc2, it will not generate the correct session key. It generates:

MyApp::Application.config.session_store :cookie_store, :key => '_config_session'

Instead of:

MyApp::Application.config.session_store :cookie_store, :key => '_my_app_session'

Comments and changes to this ticket

Rohit Arondekar August 24th, 2010 @ 03:20 PM
- Assigned user set to “José Valim”
- Importance changed from “” to “Low”
This looks very similar to https://rails.lighthouseapp.com/projects/8994/tickets/5207

Assigning to Jose, since he knew how to fix that issue.
Neeraj Singh August 24th, 2010 @ 04:02 PM
- State changed from “new” to “open”
- Tag changed from “rails:update generator session_store.rb” to “rails 3, rails:update generator session_store.rb, patch”
Attached is code patch.
- 5434_fix_for_edge.diff 945 Bytes
José Valim August 24th, 2010 @ 04:13 PM
Nice Neeraj! Can you try to add a test case? You can reproduce the rake test calls inside app_generator_test.rb!
You flagged this item as spam.
Marjan Krekoten' August 24th, 2010 @ 04:56 PM
I attached patch that fixes this issue. And also added test for it.
- fix_app_name_detection.diff 2.4 KB
You flagged this item as spam.
Marjan Krekoten' August 24th, 2010 @ 04:57 PM
Oops... I missed that there is a patch already :)
José Valim August 24th, 2010 @ 05:00 PM
Krekoten, thanks for the patch! Your fix looks good however the test needs to be improved. It relies to heavily on stubs, meaning that minor changes in Thor or Rails can make it fail, even when it should not. Could you please try to provide a test that does not use stubs/mocks?
You flagged this item as spam.
Marjan Krekoten' August 24th, 2010 @ 05:01 PM
Sure, I'll try.
You flagged this item as spam.
Marjan Krekoten' August 24th, 2010 @ 06:32 PM
Well, I didn't find better way. We need that stubs to execute generator with already initialized app or it will not fail.
You flagged this item as spam.
Fred Wu August 25th, 2010 @ 02:38 AM
Hey guys,

Since I was the one who submitted the app_name fix, here's my patch for it. I've improved the tests a bit also.
- app_name_fix.diff 3.5 KB
Repository August 28th, 2010 @ 10:08 PM
- State changed from “open” to “resolved”
(from [83f4507cf153ea6081dd70326f5f3a0331cc167e]) Fixed the session name generated by the app_generator. Also refactored the corresponding test suites to be cleaner. [#5434 state:resolved]

Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/83f4507cf153ea6081dd70326f5f3a...
Repository August 28th, 2010 @ 10:09 PM
(from [3edd3d052efc6d39345aaa3258109ec2fc92baa4]) Fixed the session name generated by the app_generator. Also refactored the corresponding test suites to be cleaner. [#5434 state:resolved]

Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/3edd3d052efc6d39345aaa3258109e...
You flagged this item as spam.
Willem van Bergen August 30th, 2010 @ 06:24 AM
The new generator uses the directory name to create the session key prefix, instead of the application name. So, when I fork "MyApp" in a directory called "their_app", this is what I'll get.
```
MyApp::Application.config.session_store :cookie_store, :key => '_their_app_session'
```
I think it should generate:
```
MyApp::Application.config.session_store :cookie_store, :key => '_my_app_session'
```
Sorry for nitpicking guys! :)
You flagged this item as spam.
Fred Wu August 30th, 2010 @ 12:54 PM
Good catch Willem.

Here's the patch for it.
- app_name_fix.diff 2.2 KB
You flagged this item as spam.
Marjan Krekoten' August 30th, 2010 @ 01:06 PM
:) last patch almost identical to mine just said in other words...
You flagged this item as spam.
Fred Wu September 24th, 2010 @ 07:48 AM
Ah it appears so. :D

@José: Any change to include either the patch in 3.0.1?
José Valim September 24th, 2010 @ 08:06 AM
- State changed from “resolved” to “open”
Repository September 24th, 2010 @ 12:11 PM
- State changed from “open” to “resolved”
(from [51e8435e346c3f2f6356342b661231fcaeaa357a]) Ensures the app generator generates the correct @app_name. [#5434 state:resolved]

Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/51e8435e346c3f2f6356342b661231...
Repository September 24th, 2010 @ 12:12 PM
(from [b0f059136bf9f99095738c5a1f4b57ed927771cd]) Ensures the app generator generates the correct @app_name. [#5434 state:resolved]

Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/b0f059136bf9f99095738c5a1f4b57...