graphiti-api / graphiti

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

Filter of extra attribute fails when generating the schema #378

Closed PChambino closed 3 years ago

PChambino commented 3 years ago

After adding a filter for an extra attribute, for example:

extra_attribute :my_attr, :string, filterable: true
filter :my_attr, only: [:eq]

The GraphitiSpecHelpers::RSpec.schema! fails with:

     NoMethodError:
       undefined method `[]' for nil:NilClass
     # /usr/local/bundle/gems/graphiti-1.2.41/lib/graphiti/schema.rb:200:in `block (2 levels) in filters'

Looking at the code (schema.rb#L200) we can see that the schema generation doesn't consider extra attributes when generating filters. Also, the attributes in resource.extra_attributes don't have any :schema key by default. Feels like an oversight, but also not exactly sure what a good solution is for this specific case.

I guess I would have expected the filter of extra attributes to still be included in the generated schema. The alternative would be to not include these filters in the schema for now so at least it doesn't fail with an error.

richmolj commented 3 years ago

Basically we expect extra attributes to be used purely for reads. Maybe that's short-sighted, but it's how things are set up today.

Looking at the code (schema.rb#L200) we can see that the schema generation doesn't consider extra attributes when generating filters.

Makes sense based on the status quo reasoning above, but I'd be happy to accept a PR.

Also, the attributes in resource.extra_attributes don't have any :schema key by default.

This seems like a bug regardless of anything else

PChambino commented 3 years ago

Ah, I see. In our use case, we define a short list of attributes to avoid having large payloads by default, and then define extra attributes for most of the other data, so clients can explicitly request any extra data they need.

richmolj commented 3 years ago

Makes sense, just not the typical use case. Would be happy to review a PR that considers extra attrs in the filtering/sorting schema ❤️