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

Compatibility with jsonapi-resources 0.10 #64

Closed
valscion opened this issue Apr 15, 2017 · 8 comments
Closed

Compatibility with jsonapi-resources 0.10 #64

valscion opened this issue Apr 15, 2017 · 8 comments

Comments

@valscion
Copy link
Member

valscion commented Apr 15, 2017

Seems like we will have to do some work in the future to be compatible with upcoming JR version, 0.10, where resources being closely coupled to ActiveRecord is being worked on.

@hidde-jan in #48 (comment)

Are there any thoughts on how (or if) to incorporate cerebris/jsonapi-resources#1006 and cerebris/jsonapi-resources#977?

From JA:R 0.10 onward, the idea is to delegate record fetching to a RecordAccessor.

In the future, the logic in PunditScopedResource might need to be moved into such a class.


EDIT:

@valscion
Copy link
Member Author

valscion commented Oct 2, 2019

Ok JR 0.10 has been released now. It seems that the major changes done are things concerning jsonapi-authorization, so it will take quite an effort to get us compatible with it.

https://jsonapi-resources.com/v0.10/guide/whats_new.html

We should update this gem to be compatible with JR 0.10 and release a new major version to support only that version. People on older versions could use older versions of jsonapi-authorization.

Any help is appreciated. I don't have time any time soon to look into this and neither is this a priority on our team any time soon.

@valscion valscion pinned this issue Oct 2, 2019
@valscion valscion changed the title Compatibility with JR 0.10? Compatibility with jsonapi-resources 0.10 Oct 2, 2019
@rpbaltazar
Copy link

rpbaltazar commented Dec 6, 2019

@valscion do you have any sort of tasklist of things that need to be touched ?
I could possibly have some time to try to push this forward.

@valscion
Copy link
Member Author

valscion commented Dec 6, 2019

Not really, sorry.

One thing I'd really like to see here would be a new approach to tests where we would assert on user visible functionality — so going from a full request to an expected authorization error without relying on any stubbing.

Some of the current tests have gotten so complex due to the authorization stubbing going on that I have a lack of trust on any big changes done to the existing tests.

These new types of tests could first start small and test only a subset of the authorization logic. I'd like them to be in a totally different directory than the existing RSpec tests, and as self-contained as possible. Meaning that the resources, controllers and policies needed for the tests would all be in one file, even if it would make for a longer file.

This type of tests could be done with minitest instead of RSpec, too.

@romikoops
Copy link

Hello, any progress with supporting 0.10? 4 months since issue opening.

@valscion
Copy link
Member Author

Does not seem like it. Feel free to offer help if you need JR 0.10 support.

@lgebhardt
Copy link

I'm sorry the JR 0.10 changes have broken this project. I'm interested in helping to get this resolved, though I have a few concerns about efficiency in addition to the compatibility issues with the new way JR v0.10 works. I have spent very little time looking at it so far, but I think most of the issues should be fairly easily solvable.

@valscion
Copy link
Member Author

valscion commented Jan 2, 2020

Thank you for offering to help, @lgebhardt ☺️. I haven't looked into the compatibility differences much either — I merely know that the way we've used to hook into the processor pipeline needs to be changed to accommodate JR 0.10.

This code: https://github.com/venuu/jsonapi-authorization/blob/v3.0.2/lib/jsonapi/authorization/authorizing_processor.rb

The way we've previously tied into JR has been like so (copied from current readme):

Usage

First make sure you have a Pundit policy specified for every backing model that your JR resources use.

Hook up this gem as the default processor for JR, and optionally allow rescuing from Pundit::NotAuthorizedError to output better errors for unauthorized requests:

# config/initializers/jsonapi-resources.rb
JSONAPI.configure do |config|
  config.default_processor_klass = JSONAPI::Authorization::AuthorizingProcessor
  config.exception_class_whitelist = [Pundit::NotAuthorizedError]
end

Make all your JR controllers specify the user in the context and rescue errors thrown by unauthorized requests:

class BaseResourceController < ActionController::Base
  include JSONAPI::ActsAsResourceController
  rescue_from Pundit::NotAuthorizedError, with: :user_not_authorized

  private

  def context
    {user: current_user}
  end

  def user_not_authorized
    head :forbidden
  end
end

Have your JR resources include the JSONAPI::Authorization::PunditScopedResource module.

class BaseResource < JSONAPI::Resource
  include JSONAPI::Authorization::PunditScopedResource
  abstract
end

@valscion
Copy link
Member Author

We never managed to fix the compatibility with jsonapi-resources v0.10. In the end, we decided to no longer support this gem. Discussion here:

@valscion valscion closed this as not planned Won't fix, can't repro, duplicate, stale Jun 26, 2023
@venuu venuu locked as resolved and limited conversation to collaborators Jun 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging a pull request may close this issue.

4 participants