Knock: Update documentation for testing with Rails 5

[denkristoffer]

Rails 5 has now been released and ActionController::TestCase is deprecated so the documentation on testing is outdated. As far as I can tell the request.env object is no longer accessible. Instead, what I've been doing is something like this:

test_helper.rb

def authentication_token
  Knock::AuthToken.new(payload: { sub: users(:one).id }).token
end

def authenticated_headers
  {
    'Authorization': "Bearer #{authentication_token}"
  }
end

controller_test.rb

test "should respond successfully" do
    get model_url(@model), headers: authenticated_headers
end

Maybe there's a better way to do this? Once a format has been decided on I can make a pull request, if you'd like.

[budi]

👍 for this, and also got depreciation warning from Knock itself using the above method:

[DEPRECATION]: Knock.current_user_from_token is deprecated. Please implement User.from_token_payload instead.

[nsarno]

@sachse Thanks! PR is always welcome 👍

The main goal of this part of the README is to show how to create the token and pass it through the header. So let's keep it as simple as possible.

Also as you said, replacing ActionController::TestCase with ActionDispatch::IntegrationTest.

[nsarno]

@budi This warning is not related. It tells you that you rely on Knock.current_user_from_token which is deprecated.

[budi]

@nsarno yeah, I figured.. but that's what I got from following the readme upon testing by invoking

token = Knock::AuthToken.new(payload: { sub: users(:one).id }).token

[denkristoffer]

Actually I get the same error in my tests, even though I don't call Knock.current_user_from_token manually.

[nsarno]

@budi I didn't mean to be obvious 🙂

@budi @sachse This actually happens because the default value for Knock.current_user_from_token is still set. I'll remove it as it shouldn't be and it is disturbing although it has no consequences.

[Donaldo]

Hi @nsarno

Thanks for this gem, it's very useful.
Are you planning a new release without the default value set for Knock.current_user_from_token ?

[nsarno]

@Donaldo Yes, this should happen soon!

[joaosoares]

Also getting the deprecation message using the authenticated_headers method above. Is there any workaround while we wait for the new release?

[Petesta]

Hi, I'm also running into the deprecated issue like @sachse.

I went through the CHANGELOG.md to address the warnings but it doesn't seem to resolve them.

I have no calls or definitions of Knock.current_user_from_handle in my code yet that deprecation warning is still not resolved.

I'm receiving two deprecation warnings.

[DEPRECATION]: Routing to `AuthTokenController` directly is deprecated. Please use `<Entity Name>TokenController` inheriting from it instead. E.g. `UserTokenController`
[DEPRECATION]: Relying on `Knock.current_user_from_handle` is deprecated. Please implement `User#from_token_request` instead.

I went through the README.md to run the generator to create the controller template.

bundle exec rails generate knock:token_controller user

But even after running this command and removing mount Knock::Engine => "/knock" from config/routes.rb I still have both of these deprecation warnings.

If the warning says to not use Knock::AuthTokenController for routing I don't think there's a way for me to avoid this since the generator has this in the code.

https://github.com/nsarno/knock/blob/master/lib/generators/templates/entity_token_controller.rb.erb

What's the workaround for this?

Petesta :: λ -> ~ (develop) ack "current_user_from_token"
config/initializers/knock.rb
49:  # config.current_user_from_token = -> (claims) { User.find claims['sub'] }
Petesta :: λ -> ~ (develop) ack "AuthToken"
app/controllers/user_token_controller.rb
1:class UserTokenController < Knock::AuthTokenController

config/initializers/knock.rb
24:  ## AuthTokenController parameters. It also uses the same variable to enforce

[denkristoffer]

@Petesta As @nsarno mentioned, the next release of knock will fix it. Until then you can just ignore the warning.

[Petesta]

@sachse Do we have any idea when that release may happen? That comment was posted back in the end of July. Even if the warning is benign I'd still like to resolve it. What is needed to help push this along?

[denkristoffer]

Sorry @nsarno, but this issue was originally about the README, not the deprecation warning 😅

[nsarno]

@sachse Ha, you're absolutely right! I merged the related PR. Sorry it took me so long to merge something so simple.

Thank you so much for contributing and being so patient ❤️