Respond to controller name #891

Merged
merged 5 commits into from Dec 25, 2013

Projects

None yet

2 participants

Contributor

When using named controller like controller FoosController do, #controller_name method and routes drawn will refer to this name instead of anonymous one.

About #884.

@JonRowe JonRowe commented on the diff Dec 24, 2013
lib/rspec/rails/example/controller_example_group.rb
@@ -60,15 +60,19 @@ def controller(base_class = nil, &body)
ApplicationController
metadata[:example_group][:described_class] = Class.new(base_class) do
- def self.name; "AnonymousController"; end
+ def self.name
JonRowe
JonRowe Dec 24, 2013 Owner

It'd be better if this was set when you specify the class name to the anonymous controller, incase someone actually sets ApplicationController, as the specified controller, this could then lazily evaluate back to "AnonymousController"

billychan
billychan Dec 24, 2013 Contributor

Add some notes at first or I'll forget :) Yap, quite considerate. I just verified this setting works for both controller do and controller ApplicationController do by adding a spec. The reason would be the class created by Class.new(ApplicationController) is always a sub class of ApplicationController.

JonRowe
JonRowe Dec 25, 2013 Owner

By better, I mean it removes a conditional so is "better" code, tell don't ask and all.

billychan
billychan Dec 25, 2013 Contributor

I'm not able to figure out how to avoid condition here, would like to see an example when you have time :)

@JonRowe JonRowe and 1 other commented on an outdated diff Dec 24, 2013
lib/rspec/rails/example/controller_example_group.rb
end
metadata[:example_group][:described_class].class_eval(&body)
orig_routes = nil
before do
orig_routes = self.routes
+ resource_name = self.instance_variable_get("@controller").controller_name
JonRowe
JonRowe Dec 24, 2013 Owner

You should be able to use @controller here, and if you can't we have a bigger issue.

billychan
billychan Dec 24, 2013 Contributor

👍

@JonRowe JonRowe and 1 other commented on an outdated diff Dec 24, 2013
lib/rspec/rails/example/controller_example_group.rb
self.routes = ActionDispatch::Routing::RouteSet.new.tap { |r|
- r.draw { resources :anonymous }
+ r.draw { resources :"#{resource_name}" }
JonRowe
JonRowe Dec 24, 2013 Owner

This is technically a change in behaviour if you aren't setting the controller class, it should only use resource_name if it's been set.

billychan
billychan Dec 24, 2013 Contributor

I have set it fallback to "anonymous".

@JonRowe JonRowe commented on the diff Dec 24, 2013
lib/rspec/rails/example/controller_example_group.rb
@@ -93,6 +97,7 @@ def routes(&blk)
self.routes = blk.call
end
end
+
JonRowe
JonRowe Dec 24, 2013 Owner

No additional ws please.

JonRowe
JonRowe Dec 25, 2013 Owner

Missed this one ;)

@JonRowe JonRowe commented on an outdated diff Dec 24, 2013
.../rspec/rails/example/controller_example_group_spec.rb
@@ -95,6 +95,26 @@ module RSpec::Rails
controller_class = group.metadata[:example_group][:described_class]
expect(controller_class.superclass).to eq(ApplicationController)
end
+
JonRowe
JonRowe Dec 24, 2013 Owner

No additional ws please

@JonRowe JonRowe and 1 other commented on an outdated diff Dec 24, 2013
.../rspec/rails/example/controller_example_group_spec.rb
@@ -95,6 +95,26 @@ module RSpec::Rails
controller_class = group.metadata[:example_group][:described_class]
expect(controller_class.superclass).to eq(ApplicationController)
end
+
+ end
+
+ describe "controller name" do
+
+ it "sets the name as AnonymousController if it's anonymous" do
+ group.controller { }
+ controller_class = group.metadata[:example_group][:described_class]
+ expect(controller_class.name).to eq "AnonymousController"
+ end
+
+ it "sets the name according to defined controller if it is not anonymous" do
+ class ::FoosController < ::ApplicationController; end;
JonRowe
JonRowe Dec 24, 2013 Owner

consider using stub_const "FoosController", Class.new(ApplicationController) so that it's cleaned up automatically.

billychan
billychan Dec 24, 2013 Contributor

👍

@JonRowe JonRowe commented on an outdated diff Dec 24, 2013
.../rspec/rails/example/controller_example_group_spec.rb
end
+
JonRowe
JonRowe Dec 24, 2013 Owner

Please match the ws style of the rest of the file.

@JonRowe JonRowe commented on an outdated diff Dec 24, 2013
features/controller_specs/anonymous_controller.feature
+
+ before do
+ get :index
+ end
+
+ it "get the class name as described" do
+ expect(assigns[:name]).to eq('FoosController')
+ end
+
+ it "get the controller_name as described" do
+ expect(assigns[:controller_name]).to eq('foos')
+ end
+
+ end
+
+ """
JonRowe
JonRowe Dec 24, 2013 Owner

Thanks for adding cucumber scenarios too, but please make sure your indentation and ws is consistent with the rest of the file.

Owner
JonRowe commented Dec 24, 2013

Thanks for making a start on this :) I've left some feedback on a few things.

Contributor

@JonRowe, thanks a lot for your review and nice suggestions :) I've made the related changes.

@JonRowe JonRowe commented on the diff Dec 25, 2013
features/controller_specs/anonymous_controller.feature
@@ -120,6 +120,39 @@ Feature: anonymous controller
When I run `rspec spec`
Then the examples should all pass
+ Scenario: get name and controller_name from the described class
+ Given a file named "spec/controllers/get_name_and_controller_name_from_described_class_spec.rb" with:
+ """ruby
JonRowe
JonRowe Dec 25, 2013 Owner

I actually meant indent the code inwards under the Given, like the first few scenarios, I now realise some of the later scenarios are already indented incorrectly :)

@JonRowe JonRowe merged commit 3e37de9 into rspec:master Dec 25, 2013

1 check passed

default The Travis CI build passed
Details
@JonRowe JonRowe added a commit that referenced this pull request Dec 25, 2013
@JonRowe JonRowe cleanup after #891 143e3cd
@JonRowe JonRowe added a commit that referenced this pull request Dec 25, 2013
@JonRowe JonRowe Changelog for #891
[skip ci]
2fb9dcf
Owner
JonRowe commented Dec 25, 2013

Thanks for your help

Contributor

Jon, thanks!

@billychan billychan deleted the billychan:respond-to-controller-name branch Dec 25, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment