json_api_client icon indicating copy to clipboard operation
json_api_client copied to clipboard

`belongs_to` combined with `shallow_path` breaks request url

Open wpliao1989 opened this issue 5 years ago • 4 comments

Steps to reproduce:

require 'bundler/inline'

gemfile(true) do
  source 'https://rubygems.org'

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem 'json_api_client', '1.8.0'
end

class Resource < JsonApiClient::Resource
  self.site = 'http://example.com/api'
end

class Author < Resource
end

class Store < Resource
end

class Book < Resource
  belongs_to :author, shallow_path: true
  belongs_to :store, shallow_path: true
end

require 'minitest/autorun'

class PathTest < MiniTest::Test
  def test_path
    assert_equal 'books', Book.path({}) # This breaks because the result is `/books` instead of `books`
  end

  def test_get
    Book.all # Sends requests to http://example.com/books instead of http://example.com/api/books
  end
end

What's wrong:

This part of code: https://github.com/JsonApiClient/json_api_client/blob/db890adf3d7175e91829d359336ab7c98ed90a34/lib/json_api_client/resource.rb#L324

a.set_prefix_path(attrs, route_formatter) can return nil if using shallow_path. If there are 2 or more belongs_to association in the resource, paths.join("/") would return extra slashes ([nil, nil].join('/') => '/').

Possible fix:

Remove nils:

def _set_prefix_path(attrs)
  paths = _belongs_to_associations.map do |a|
    a.set_prefix_path(attrs, route_formatter)
  end

  paths.compact.join("/")
end

Also, I'm not sure what _prefix_path is meant to do so maybe we need to fix that method too.

wpliao1989 avatar Jul 15 '20 19:07 wpliao1989

Just opened a PR which addresses this issue. Multiple belongs_to :resource, shallow_path: true will now be possible.

ollizeyen avatar Feb 16 '21 05:02 ollizeyen

Thanks! Did you look into _prefix_path method on line 311 as well? Its implementation is very similar to _set_prefix_path.

wpliao1989 avatar Feb 16 '21 15:02 wpliao1989

Just checked again. Compacted the array in the path getter as well!

ollizeyen avatar Feb 16 '21 17:02 ollizeyen

Will add tests early tomorrow. At least the current suite doesn't break locally. Is there any difference on travis? On travis the runner fails.

ollizeyen avatar Feb 16 '21 18:02 ollizeyen