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

[Controller] Adds Route Helper Methods to Controller #362

Merged
merged 4 commits into from
Nov 8, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions spec/amber/controller/base_spec.cr
Expand Up @@ -27,6 +27,11 @@ module Amber::Controller
controller.responds_to?(:client_ip).should eq true
controller.responds_to?(:request).should eq true
controller.responds_to?(:response).should eq true
controller.responds_to?(:route_action).should eq true
controller.responds_to?(:route_resource).should eq true
controller.responds_to?(:route_scope).should eq true
controller.responds_to?(:route_controller).should eq true
Copy link
Contributor

Choose a reason for hiding this comment

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

Do all of these need to be prefixed with route_?

Copy link
Contributor Author

@eliasjpr eliasjpr Nov 5, 2017

Choose a reason for hiding this comment

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

I prefixed to avoid name collisions with methods define by the user. I am open to suggestions

controller.responds_to?(:controller_name).should eq true
end

describe "#redirect_back" do
Expand Down
1 change: 1 addition & 0 deletions src/amber/controller/base.cr
Expand Up @@ -9,6 +9,7 @@ module Amber::Controller
include Helpers::Redirect
include Helpers::Render
include Helpers::Responders
include Helpers::Route

include Callbacks

Expand Down
23 changes: 23 additions & 0 deletions src/amber/controller/helpers/route.cr
@@ -0,0 +1,23 @@
module Amber::Controller::Helpers
module Route
def route_action
context.request_handler.action
end

def route_resource
context.request_handler.resource
end

def route_scope
context.request_handler.scope
end

def route_controller
context.request_handler.controller
end

def controller_name
self.class.name.gsub(/Controller/i, "")
Copy link
Contributor

Choose a reason for hiding this comment

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

For this regex, I would anchor to the end. And does the i make it case-insensitive, is that necessary?

https://play.crystal-lang.org/#/r/31bu

Copy link
Contributor

Choose a reason for hiding this comment

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

Would people want the controller name downcased?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I think we should use /Controller$/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You guys are correct I will also fix the regex. I looked at the rails implementation

https://apidock.com/rails/ActionController/Metal/controller_name/class

Copy link
Member

Choose a reason for hiding this comment

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

Use sub instead of gsub since we only want to replace once.

end
end
end