graphiti-api / graphiti

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

allow_nil: true not working for String columns #375

Open jelaniwoods opened 3 years ago

jelaniwoods commented 3 years ago

I edited the example employee directory app to allow nil values for each employee field—

 class EmployeeResource < ApplicationResource
   self.filters_accept_nil_by_default = true
   attribute :first_name, :string
   attribute :last_name, :string
   attribute :age, :integer
  ...

When I try to enter null in Vandal I get an error

graphiti-employee-resource-null-bug

Started GET "/api/v1/employees?filter[first_name][eq]=null" for ::1 at 2021-08-13 16:52:50 -0500
Processing by EmployeesController#index as JSONAPI
  Parameters: {"filter"=>{"first_name"=>{"eq"=>"null"}}}
Completed 500 Internal Server Error in 3ms (ActiveRecord: 0.0ms)

NoMethodError (undefined method `downcase' for nil:NilClass):

app/controllers/employees_controller.rb:3:in `index'

Entering null in Integer columns like age and team_id work as expected.

The problem seems to stem from line 43 in this method

https://github.com/graphiti-api/graphiti/blob/7991740b40f9ec32f017cd90d298388f42a6c94a/lib/graphiti/adapters/active_record.rb#L41-L45

I also noticed there was a test for null values in a String field this that is passing. I assume this is still passing because the test adapter doesn't use the same filter methods that are in the active_record adapter.

smai-f commented 2 years ago

Hey @jelaniwoods I put up a fix for this here 🤞 : https://github.com/graphiti-api/graphiti/pull/410