Add support for directly mounted Grape::API::Instance #22
Conversation
| gem 'grape', '1.2.3' | ||
| gem 'rails-controller-testing' | ||
| gem 'sqlite3', '~> 1.4' | ||
| when '2.5.3', '2.6.3' then |
There was a problem hiding this comment.
I am only seeing this gem used in ops_app and property, which both use ruby 2.6.3, so support for ruby 2.5.3 can probably be dropped as well at this point. (Unless it is used somewhere else that I am missing)
There was a problem hiding this comment.
Additionally, the .ruby-version file still lists ruby-2.3.3 so that can probably be updated to ruby-2.6.3
There was a problem hiding this comment.
Just realized that this is a public gem, so I guess it should support more versions than one's used at Appfolio.
There was a problem hiding this comment.
Yeah, we should support more than just what we use internally.
There was a problem hiding this comment.
Yeah, as a public repo we need to support more versions. I can update the .ruby-version though.
| [ | ||
| options[:for], | ||
| options[:for].base | ||
| ].detect { |api| api.respond_to?(:decl_auth_context) } |
There was a problem hiding this comment.
TIL detect exists and is basically find.
| # Grape >= 1.2.0 endpoint | ||
| # Authorization::Controller::Grape can be included into either Grape::API | ||
| # or Grape::API::Instance, so we need to check both. | ||
| [ |
There was a problem hiding this comment.
Is the order significant? It seems we used to check options[:for].base first when it used to be in ControllerPermission.
There was a problem hiding this comment.
Its only ever going to be one or the other.
|
This CI run tests the changes against Grape 1.2: |
This PR adds support for Grape APIs that are mounted directly as
Grape::API::Instanceinstead ofGrape::API.Appraisals are also added for Grape 1.3.0.