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

before_action not calling method #50

Open
drujensen opened this issue Aug 13, 2015 · 17 comments
Open

before_action not calling method #50

drujensen opened this issue Aug 13, 2015 · 17 comments

Comments

@drujensen
Copy link
Contributor

So, I'm probably doing something wrong, but I have a before_action :authorize in my controller and it doesn't seem to call the def authorize method.

I see the macros defined in the controllers for before_action. Any idea's?

@bararchy
Copy link
Contributor

it works for me , make sure authorize is the last def in the class , its something to do with how the macro is working , if you still have issues I'll add a code sample from a working project

@drujensen
Copy link
Contributor Author

Thanks for you quick response!

Here is my controller: https://github.com/drujensen/crystal-blog/blob/master/app/controllers/post_controller.cr

When I inspect the @callback_methods, it doesn't have any of the @actions I list. It has the @types instead. Not sure how this works but it seems like you would want to register the actions instead?

@drujensen
Copy link
Contributor Author

I have a fix but I'm not happy with it. Basically, to register the action in the macro, you need the list of actions that the before filter applies too. Here is the fix in the Base::Controller.

  macro before_action(callback, *actions)
    {% for action in actions %}
      def _before_{{action.id}}_{{callback.id}}
      @before_callbacks["{{action.id}}"] = [] of (-> Bool) unless @before_callbacks["{{action.id}}"]?
      @before_callbacks["{{action.id}}"] << ->{{callback.id}}
      end
    {% end %}
  end

Before, this method was creating callbacks for the @types instead of the @actions.

Let me know if you have a better way of getting the list of actions. Can you grab them from the other macro called action?

@bararchy
Copy link
Contributor

Well, macros aren't my strong side, I'll leave that to @sdogruyol or @Codcore .

But, I know you have two options regarding before_action

# before_action :method_to_execute_before_actions, only: [:methods, :to, :use, :the, :before, :action]
before_action :authorized?, only: [:execute, :new, :notifications, :dash_board] 

Or

# This will apply "authorized?" to every action 
before_action :authorized?

@drujensen
Copy link
Contributor Author

I verified that the only: option works. The before_action :authorized? doesn't work.

Specifically:

  macro before_action(callback, only=[] of Symbol)
    {% if only.empty? %}
      {% only = @type.methods.map(&.name.stringify) %}
    {% end %}

If you don't specify the only:, you can see that it tries to get the list for you. The problem is that it is creating the list from @types instead of @actions.

So the callbacks are indexed on any types that are defined in the controller, instead of the actions defined in the other macro called actions. Not sure how to get that list of actions from the other macro.

Is there a way to do this in Crystal?

@sdogruyol
Copy link
Contributor

@drujensen
Copy link
Contributor Author

Thanks @sdogruyol . How do I get the list of actions in the other macro called before_action? Can one macro use the results of another?

@sdogruyol
Copy link
Contributor

Well first you have to register it via actions macro and then i think you can access the actions via @actions

@drujensen
Copy link
Contributor Author

unfortunately, @actions is not available in the scope of the macro.

@sdogruyol
Copy link
Contributor

I see we need to tinker about macro loading order :(

@sdogruyol
Copy link
Contributor

Also we need specs for before_action callbacks

@drujensen
Copy link
Contributor Author

@sdogruyol are you saying that it should be available in the macros? Let me try some things if thats true.

@sdogruyol
Copy link
Contributor

Yeah.If you register it as an action first then it should be available.

@drujensen
Copy link
Contributor Author

EDIT: Nope, false alarm. Its still not available even when I register the action before the before action

@sdogruyol
Copy link
Contributor

We have to rethink about before_callback implementation and have some proper specs 👍

@drujensen
Copy link
Contributor Author

@sdogruyol Ok. Thanks for your help. For now, I will use the only: option and pass in all the actions.

@sdogruyol
Copy link
Contributor

Your welcome @drujensen i'll tackle this issue

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

No branches or pull requests

3 participants