Open jmanian opened 3 years ago
I intentionally did not add the option as a request-level parameter, because it would only work on the first request made by each client instance, given that connection
is reused after the first request.
1 Warning | |
---|---|
:warning: | There're library changes, but not tests. That's OK as long as you're refactoring existing code. |
Generated by :no_entry_sign: Danger
This is good, but I think we can do better. What do you think of being able to write this:
config.connection do |connection, options|
connection.use ::Slack::Web::Faraday::Response::StoreScopes, options
end
This way clients can install any middleware.
Another option could be adding .middleware
:
config.middleware << ::Slack::Web::Faraday::Response::StoreScopes
Not sure which one I prefer, would take either.
@dblock How would your first option work exactly? Would the block be stored in the config as a proc, and then called with the connection instance during the ::Faraday::Connection.new(endpoint, options) do |connection|
block? Or is there a way to use
middleware before the connection is instantiated?
In both cases how would it handle the fact that I'm passing the instance of the client to use
? If I understand how the first option works, then the user would need to know about this and account for it in the config block. In the latter option how would the code know what options to use when it calls use
for each config.middleware
?
Also with these options would the user be able to set this at the client instance level, or only at the config level? In my case, since I only need to use this in a specific place with auth_test
, my plan was to set it on the instance, e.g.:
Slack::Web::Client.new(token: token, store_scopes: true).auth_test
@dblock How would your first option work exactly? Would the block be stored in the config as a proc, and then called with the connection instance during the
::Faraday::Connection.new(endpoint, options) do |connection|
block? Or is there a way touse
middleware before the connection is instantiated?
Right, you would yield
at the right time the stored proc.
In both cases how would it handle the fact that I'm passing the instance of the client to
use
? If I understand how the first option works, then the user would need to know about this and account for it in the config block. In the latter option how would the code know what options to use when it callsuse
for eachconfig.middleware
?
I think you could hide this away. I didn't write the code but I would want the supporting config code to always pass in client: self
when the proc is evaluated.
Also with these options would the user be able to set this at the client instance level, or only at the config level? In my case, since I only need to use this in a specific place with
auth_test
, my plan was to set it on the instance, e.g.:Slack::Web::Client.new(token: token, store_scopes: true).auth_test
It should work in both.
If you are having trouble implementing this LMK and I could give it a shot. I don't know whether it's possible - I am only thinking about what the user would want to write, and leaving the implementation to the professionals ;)
Resolves #378
This adds an optional response middleware to the Web client. When used the middleware pulls the OAuth scopes from the response header
x-oauth-scopes
and puts them on the instance of the client atclient.oauth_scopes
.I wasn't sure the best way to gain access to the client itself from the middleware, so I followed the pattern used by the
Mashify
middleware to pass in an option. Maybe there's a better way?I'm also not sure about the naming of the middleware class (
StoreScopes
) or the config option (store_scopes
).~No tests or documentation yet.~