cerebris / jsonapi-resources

A resource-focused Rails library for developing JSON:API compliant servers.
http://jsonapi-resources.com
MIT License
2.32k stars 532 forks source link

related_resource(s) doesn't include the context nor uses the records scope #561

Closed xdmx closed 8 years ago

xdmx commented 8 years ago

I have set a context method in the posts controller. When I load /api/posts I can see that the context is correctly set, but when I load a related resource like /api/users/1/posts the context is not set, breaking everything.

I also have a records method in the post resource, which returns only published posts, but when I access the posts of the related user it doesn't use that scope and it returns all posts

lgebhardt commented 8 years ago

@xdmx I'm not immediately seeing a problem, though I needed to add a pair of tests to ensure this was working. I've added these tests in the test_context_flows branch (https://github.com/cerebris/jsonapi-resources/tree/test_context_flow). Which controller method is being called where the context isn't being set for you? If it's get_related_resources that will be on the related resource's controller (posts in your case) so context must be set there as well.

xdmx commented 8 years ago

It's get_related_resources... I've added the following in the post_resource:

def self.records(options = {})
  puts 'records is called'
  super
end

def self.apply_filter(records, filter, value, _options={})
  puts 'filter is called'
  super
end

def self.apply_sort(records, order_options, _options={})
  puts 'sort is called'
  puts _options.inspect
  super
end

Then from the browser I go to http://localhost:3000/api/posts and I get

Started GET "/api/posts" for 127.0.0.1 at 2015-12-04 16:05:43 +0000
Processing by Api::PostsController#index as HTML

[...]

records is called
sort is called
{:context=>{:action_name=>"index", :current_user=>#<User id: 1427, username: "test">, :country=>nil}, :include_directives=>nil, :sort_criteria=>[{:field=>"id", :direction=>:asc}], :paginator=>#<PagedPaginator:0x0055a1a9d686c0 @number=1, @size=10>}

But http://localhost:3000/api/users/1/posts gives me the following:

Started GET "/api/users/1/posts" for 127.0.0.1 at 2015-12-04 16:06:57 +0000
Processing by Api::PostController#get_related_resources as HTML
  Parameters: {"relationship"=>"posts", "source"=>"api/users", "user_id"=>"1"}

[...]

sort is called
{}

I'm using the v0.6.2 with the #549 change in order to have the context in the apply_sort method

lgebhardt commented 8 years ago

I assume you are passing in the options to sort_records as part of #549? Is the context available in apply_filter or records?

xdmx commented 8 years ago

Yes, the options in the sort_records is from that pull request.

The context is defined in the controller, which I expect it should be used for index, show and get_related_resources requests:

class PostsController < JSONAPI::ResourceController
      def context
        { action_name: action_name, current_user: current_user, country: params[:country] }
      end
end

I'd also expect that the get_related_resources would call the resource's records, but it doesn't seems like it's calling it...

Meanwhile I've also tried with the current master and it has the same behavior

lgebhardt commented 8 years ago

So the get_related_resources method uses a dynamically created method on the resource named after the relationship to get the related resources. This then calls records_for with the relationship name. Because this records_for is an instance method you have access to the @context on the instance. records_for by default calls the underlying model's method named for the relationship. You may override this to change the behavior, such as adding a context based where clause. It's a bit mucky with the override, but that's left over from the earlier days of the project where we did most things as overrides. I think a better way would be to add a callable option to the relationship if you want to change the default behavior.

In line 864 (https://github.com/cerebris/jsonapi-resources/blob/master/lib/jsonapi/resource.rb#L864) the context should be passed in to apply_sort as an option. I made a note on #549.

xdmx commented 8 years ago

Gotcha, I thought it'd user the Post.records scope, but I now see that it calls User.records_for('posts') which returns user.posts.. I've fixed it with the following in the UserResource:

      def records_for(relation_name)
        case relation_name
          when :posts
            PostResource.records(context: @context).where(user: _model)
          else
            super
        end

I've also fetched the current version of the pull request and the context is correctly working now :smile: