graphiti-api / graphiti

Stylish Graph APIs
https://www.graphiti.dev/
MIT License
985 stars 141 forks source link

Polymorphic nested include broken in 0.6 #8

Closed bastianlb closed 7 years ago

bastianlb commented 7 years ago

Hey Lee,

I got around to upgrading jsonapi_suite in 0.6 and implementing resource etc. I'm having trouble with something that seemed to work previously, which is a nested include on a polymorphic object.

class WatchedAt < ApplicationRecord
  belongs_to :watchable, :polymorphic => true
end

class Movie < ApplicationRecord
  has_many :watched_at, :as => :watchable
end

class Episode < ApplicationRecord
  belongs_to :show
  has_many :watched_at, :as => :watchable
end

class Show < ApplicationRecord
  has_many :episodes
end

I'm trying to issue the following query: /watched_ats?include=watchable,watchable.show

My WatchedAt Resource / Controller:

class WatchedAtResource < JsonapiCompliable::Resource
  type :watched_at
  use_adapter JsonapiCompliable::Adapters::ActiveRecord
  allow_filter :watchable_type

  polymorphic_belongs_to :watchable,
    group_by: proc { |watchable| watchable.watchable_type },                                                                                                                             groups: {
      'Episode' => {
        foreign_key: :watchable_id,
        resource: EpisodeResource,
        scope: -> { Episode.all }
      },                                                                                                                                                                                   'Movie' => {
        foreign_key: :watchable_id,
        resource: MovieResource,
        scope: -> { Movie.all }
      }
    }
end

class WatchedAtsController < ApplicationController
  jsonapi resource: WatchedAtResource do
     sideload_whitelist({
        index: [{ watchable: :show }, :watchable],
        show: [{ watchable: :show }, :watchable]
      })
  end
  def index
    query = WatchedAt.where(user_id: current_user.id).includes(:watchable)
    render_jsonapi(query)
  end
end
class EpisodeResource < JsonapiCompliable::Resource
  type :episode
  use_adapter JsonapiCompliable::Adapters::ActiveRecord
  allow_filter :title
  belongs_to :show,
    foreign_key: :show_id,
    scope: -> { Show.all },
    resource: ShowResource
end

The watchables from each "WatchedAt" object are being included (movies and episodes), however, the show objects on the episodes are not being included. I've also tried things like [{episode: :show}] in my controller sideload_whitelist to no avail. Off the top of your head, do you know why this would be failing?

Thanks for your help.

richmolj commented 7 years ago

@bastianl thanks for the detailed information. This is a bug with polymorphic sideloads, I'll have it fixed today.

richmolj commented 7 years ago

@bastianl fix in v0.6.2, please confirm

bastianlb commented 7 years ago

Hmm, it doesn't seem like jsonapi-suite made it to 0.6.2, I can't seem to install it via bundle..

Could not find gem 'jsonapi_suite (= 0.6.2)' in any of the gem sources listed in your Gemfile or available on this machine.

When I update just jsonapi_compliable to 0.6.2 i'm getting an error in render_jsonapi in almost all my tests..

      Failure/Error: render_jsonapi(query)

      TypeError:
        no implicit conversion of nil into Hash
      # ./app/controllers/watched_ats_controller.rb:12:in `index'
richmolj commented 7 years ago

@bastianl sorry for the confusion, I did mean jsonapi_compliable 0.6.2.

I created a sample application replicating this code that's working. Run rake db:create && rake db:migrate && rake db:seed, start the server and hit /watched_ats?include=watchable.show. If there's an issue, please see if you can recreate it in that repo.

One issue I see is in your code snippet the hash keys are off. Should read

polymorphic_belongs_to :watchable,
    group_by: proc { ... },                                                                                                                           
    groups: {
      'Episode' => { ... }                                                                                                                                                                                
      'Movie' => { ... }
    }

The original comment is missing :groups and Movie keys - not sure if that was a copy/paste issue?

While I'm here, to minor things to help you out. One, the URL /watched_ats?include=watchable,watchable.show can just be /watched_ats?include=watchable.show, no need to repeat watchable. The other is there is no need to do .includes in the controller if you are already sideloading, the libraries will handle eager loading for you 🙂

Hope that helps!

richmolj commented 7 years ago

@bastianl is this still outstanding?