graphiti-api / graphiti

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

Array of strings filters raise an error when only one value is supplied #370

Open mattgibson opened 3 years ago

mattgibson commented 3 years ago

In this URL, the excluded_food_groups are intended to be a comma-separated array, but here, only one value is supplied:

http://www.example.com/recipes/recommendations?filter[customer_id]=3&filter[delivery_date][eq]=2022-12-15&filter[excluded_food_groups][eq]=Beef&filter[meal_plan][eq]=Balanced&filter[portion_count_per_meal][eq]=2

In the resource, the filter is defined like this:

    filter :excluded_food_groups,   :array_of_strings, required: true

The error raised is Graphiti::Errors::TypecastFailed from here

The message is "Beef" violates constraints (type?(Array, "Beef") failed).

If I change this line to the following, then it works:

            params: Dry::Types["coercible.array"].of(map[:params]),
Ben-Fenner commented 2 years ago

This might be of some help to you. We've run into a similar issue and weren't sure if it was our old version of Graphiti (1.3.5) or not so we've not reported it as a bug yet.

In our controller that accepts a filter similar to how you describe, we do this:

def index
  if params[:filter][:excluded_food_groups].class != ActionController::Parameters
    # The user dictated a filter matching style of "case-insensitive equality (:eq)" which Graphiti has already
    # unwrapped from the parameters, or they did not dictate any filter matching style at
    # all (not :gt, nor :gte, etc.) which means we can safely assume they wanted a matching style of "case-insensitive
    # equality (:eq)", so proceed with guaranteeing the excluded_food_groups parameter is an array.

    # Guarantee that if a single excluded food group is given that it is placed into an array to make our desired
    # Graphiti attribute type (:array_of_strings) happy. Multiple excluded food groups don't need to be turned into an array
    # this way, but it doesn't hurt. Hopefully Graphiti 2.0 fixes this issue so we no longer need this work-around.
    # TODO: When that time comes, remove the conditional above and this line of code. Runs tests and assess the
    # situation.
    params[:filter][:excluded_food_groups] = params[:filter][:excluded_food_groups].split(',').flatten
  end

  recommendations = RecommendationResource.all(params)

  respond_to do |format|
    format.json    { render(json:    recommendations) }
    format.jsonapi { render(jsonapi: recommendations) }
    format.xml     { render(xml:     pretty_xml(recommendations)) }
  end
end

(Ignore the pretty_xml part. That's our work-around to a different Graphiti bug.)

bguban commented 3 weeks ago

Hi. I faced the same problem. I would prefer not to update parameters in the controller because it doesn't fix the same issue during sideloading so I just redefined the filter using string value type and then explicitly split it:

  filter :label_ids, :string, only: [:intersects] do
    intersects do |scope, label_ids|
      scope.joins(:item_labels).where(item_labels: { label_id: label_ids.split(',') }).distinct
    end
  end