`derive_filter? false` does not appear to do anything at the resource level

[sevenseacat]

The docs say this should be supported at the resource level, but after adding it to my json_api block:

  json_api do
    type "artist"
    includes [:albums]

    derive_filter? false
  end

I still have filter appearing in Swagger UI:

Adding it to an individual route (ie. index :search) does work as expected.

In open_api.ex there is one reference to route.derive_filter? that looks a bit sketchy to me:

    defp has_index_route?(domains, resource) do
      resource
      |> AshJsonApi.Resource.Info.routes(domains)
      |> Enum.any?(fn route ->
        route.type == :index && read_action?(resource, route) && route.derive_filter?
      end)
    end

Does this need to be route.derive_filter? || AshJsonApi.Resource.Info.derive_filter?(resource) ?

[zachdaniel]

Yep, it does :)

[zachdaniel]

Actually now I'm not sure...that probably does need to change, but it wouldn't solve the issue I don't think.

[zachdaniel]

oh, duh. Its && not || is all :)

[sevenseacat]

I don't think is it. Should we have tests for this stuff?

  json_api do
    type "artist"
    includes [:albums]

    derive_filter? false
  end

After updating, I still have the filter option in swagger UI, it's just not pre-populated with content anymore. Redoc still shows the full set of options.

Putting it at the route level still works.

[zachdaniel]

We should have tests, yes. And there are tests, but it's not 100% coverage, as you're discovering. but for example, in /open_api_test.exs

      %OpenApiSpex.Operation{} = operation = api_spec.paths["/authors/no_filter"].get
      refute Enum.any?(operation.parameters, &(&1.name == :filter))

[zachdaniel]

The way that derive_filter? is meant to work (at least the way I intended it to work) is that derive_filter? on the resource would disable all filter derivation everywhere. So route.derive_filter? is only useful for turning it off. So it should be derive_filter?(resource) && route.derive_filter?. There may be another place that I'm not checking both places.

[zachdaniel]

Hm... as far as I can tell, we are always checking in both places...

okay, so has_index_route? didn't need to be && derive_filter?(resource) because it was already inside a function that was checking that.

[zachdaniel]

Got it reproduced in a test. We were only testing the route specific option.

[zachdaniel]

alas, twas a typo.

[zachdaniel]

Typo fix here: https://github.com/ash-project/ash_json_api/commit/b1aae808a3038f7e7e1facf4b366c979c03e606a#diff-87214ffb41924051bbbbfb1242254e5ef95b0d9d6d1b4bb6cf270d866eb4120bL11

[sevenseacat]

This works as expected now, thank you :D