beam-community / jsonapi

JSON:API Serializer and Query Handler for Elixir
https://hex.pm/packages/jsonapi
MIT License
490 stars 79 forks source link

Bug with expected format in `included` data #260

Open doomspork opened 2 years ago

doomspork commented 2 years ago

I need to flatten some JSON:API data down in another project so I wanted to look at whether I could leverage the JSONAPI.Utils.DataToParams.process/1 functionality. When I passed in our JSON:API data I got an error:

iex> JSONAPI.Utils.DataToParams.process(file)
** (FunctionClauseError) no function clause matching in anonymous fn/2 in JSONAPI.Utils.DataToParams.process_included/1

    The following arguments were given to anonymous fn/2 in JSONAPI.Utils.DataToParams.process_included/1:

        # 1
        %{
          "attributes" => %{
            "damaged" => nil,
            "expiresAt" => nil,
            "insertedAt" => "2021-11-30T19:15:35Z",
            "integrationMetadata" => %{"customData1" => "__MWJ1071102-MWC1402073"},
            "lotNumber" => nil,
            "quantity" => "300",
            "unit" => "ea",
            "updatedAt" => "2021-11-30T19:15:35Z"
          },
          "id" => "0e4c7852-0ac3-4ee9-8d1d-841ec171976b",
          "relationships" => %{
            "item" => %{
              "data" => %{
                "id" => "8031cf98-89f8-4a1c-a787-b74f6e76a44c",
                "type" => "item"
              },
              "links" => %{"related" => "/items/8031cf98-89f8-4a1c-a787-b74f6e76a44c"}
            }
          },
          "type" => "lineItem"
        }

When I went to the line specified to see what was expected I noticed that we expected a "data" key in each included resources: https://github.com/jeregrine/jsonapi/blob/master/lib/jsonapi/utils/data_to_params.ex#L97 and that's reflected in our tests: https://github.com/jeregrine/jsonapi/blob/master/test/utils/data_to_params_test.exs#L96-L132

 test "processes single includes" do
    incoming = %{
      "data" => %{
        "id" => "1",
        "type" => "user",
        "attributes" => %{
          "name" => "Jerome"
        }
      },
      "included" => [
        %{
          "data" => %{
            "attributes" => %{
              "name" => "Tara"
            },
            "id" => "234",
            "type" => "friend"
          }
        }
      ]
    }

    result = JSONAPI.Utils.DataToParams.process(incoming)

    assert result == %{
             "friend" => [
               %{
                 "name" => "Tara",
                 "id" => "234",
                 "type" => "friend"
               }
             ],
             "id" => "1",
             "type" => "user",
             "name" => "Jerome"
           }
  end

I believe this to be incorrect. If we look at https://jsonapi.org they do not have the "data" key in each of the included resources:

 "included": [{
    "type": "people",
    "id": "9",
    "attributes": {
      "firstName": "Dan",
      "lastName": "Gebhardt",
      "twitter": "dgeb"
    },
    "links": {
      "self": "http://example.com/people/9"
    }
  }, {
    "type": "comments",
    "id": "5",
    "attributes": {
      "body": "First!"
    },
    "relationships": {
      "author": {
        "data": { "type": "people", "id": "2" }
      }
    },
    "links": {
      "self": "http://example.com/comments/5"
    }
  }

If someone else can verify this is in fact a bug I can put together a PR to correct this.

doomspork commented 2 years ago

I found potentially 2 other issues with our parsing:

  1. Parsing relationships also requires "data" but per the spec it is not required: https://jsonapi.org/format/#document-resource-object-relationships. In our case some of our associated data only includes a "link" key.
  2. We don't appear to handle nested relationships which is also supported by the spec: https://jsonapi.org/format/#document-resource-objects.
github-actions[bot] commented 6 days ago

This issue has been automatically marked as "stale:discard". We are sorry that we haven't been able to prioritize it yet. If this issue still relevant, please leave any comment if you have any new additional information that helps to solve this issue. We encourage you to create a pull request, if you can. We are happy to help you with that.