ash-project / ash_json_api

The JSON:API extension for the Ash Framework
https://hexdocs.pm/ash_json_api
MIT License
55 stars 41 forks source link

`default_fields` at the resource level not loading required aggregates and erroring #195

Closed sevenseacat closed 1 month ago

sevenseacat commented 1 month ago

Describe the bug

When including an aggregate into the list of defined fields at the resource level, eg.

  json_api do
    type "artist"
    includes [:albums]

    # album_count is an aggregate
    default_fields [:name, :album_count]
  end

Attempting to access an API endpoint for the resource generates an error because the aggregate is not loaded:

[error] ** (Protocol.UndefinedError) protocol Jason.Encoder not implemented for #Ash.NotLoaded<:aggregate, field: :album_count> of type Ash.NotLoaded (a struct), Jason.Encoder protocol must always be explicitly implemented.

If you own the struct, you can derive the implementation specifying which fields should be encoded to JSON:

    @derive {Jason.Encoder, only: [....]}
    defstruct ...

It is also possible to encode all fields, although this should be used carefully to avoid accidentally leaking private information when new fields are added:

    @derive Jason.Encoder
    defstruct ...

Finally, if you don't own the struct you want to encode to JSON, you may use Protocol.derive/3 placed outside of any module:

    Protocol.derive(Jason.Encoder, NameOfTheStruct, only: [...])
    Protocol.derive(Jason.Encoder, NameOfTheStruct)
. This protocol is implemented for the following type(s): Any, Ash.CiString, Ash.Union, Atom, BitString, Date, DateTime, Decimal, Ecto.Association.NotLoaded, Ecto.Schema.Metadata, Float, Integer, Jason.Fragment, Jason.OrderedObject, List, Map, NaiveDateTime, OpenApiSpex.JsonErrorResponse, OpenApiSpex.OpenApi, Time
    (jason 1.4.3) lib/jason.ex:164: Jason.encode!/2
    (ash_json_api 1.3.0) lib/ash_json_api/controllers/response.ex:61: AshJsonApi.Controllers.Response.render_one/5
    ...

This works if the default_fields is specified at the route level, just not at the resource level.

Expected behavior

The API response should not error and should include the album_count aggregate in the response.

Runtime

zachdaniel commented 1 month ago

I just pushed up a fix for this in response to https://github.com/ash-project/ash_json_api/issues/194 can you see if that fixes this for you as well please?

zachdaniel commented 1 month ago

I've got a test reproducing that other issue, but it doesn't behave the same way as this one does, let me try with an aggregate instead of a calc, perhaps its a difference in our serialization.

zachdaniel commented 1 month ago

Okay, yeah so without that previous fix, it looks like we get the error you're describing. So that last fix fixed both, will ensure both are covered in the test I'm writing :)

sevenseacat commented 1 month ago

Yep, works 👍 I thought it would be pretty similar to that issue but different enough to warrant its own mention.