Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Return source pointer '/data/attributes' if attribute was not supplied in payload #94

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JoeWoodward
Copy link

This PR fixes the source pointer when the payload is missing a validated attribute.

e.g.

class User
  validates :name, presence: true
end

class UsersController
  deserializable_resource :user

  def create
    user = User.new(user_params)
    if user.save
      render jsonapi: user
    else
      render jsonapi_errors: user.errors
    end 
  end

  private
  
  def user_params
    params.require(:user).permit(:name)
  end
end

if we post

{
  data: {
    attributes: {}
  }
}

We get back an error with an empty pointer

{
  "errors": [
    {
      "title": "Invalid name",
      "detail": "Name can't be blank",
      "source": {}
    }
  ],
  "jsonapi": {
    "version": "1.0"
  }
}

With this change we return the node where the attribute is missing from

{
  "errors": [
    {
      "title": "Invalid name",
      "detail": "Name can't be blank",
      "source": {
        "pointer": "/data/attributes"
      }
    }
  ],
  "jsonapi": {
    "version": "1.0"
  }
}

@JoeWoodward JoeWoodward force-pushed the feature/infer_source_from_errors branch from 94965a5 to 3c197dd Compare September 2, 2018 13:12
@JoeWoodward
Copy link
Author

JoeWoodward commented Sep 2, 2018

Should we override the error description if the pointer is missing? http://jsonapi.org/examples/#error-objects-source-usage the example here returns a description

"detail": "Missing data Member at document's top level."
which maybe makes more sense. Then supply the original error in the meta

i.e.

{
  "errors": [
    {
      "detail": "Missing `name` Member at document's /data/attributes level",
      "source": {
        "pointer": "/data/attributes"
      },
      "meta": {
        "detailed_error_message": {
          "title": "Invalid name",
          "detail": "Name cannot be blank"
        }
      }
    }
  ],
  "jsonapi": {
    "version": "1.0"
  }
}

Maybe the meta is overkill. Probably enough to just override the detail and remote the title

@beauby
Copy link
Member

beauby commented Apr 5, 2019

IIRC, the /data pointer should always be valid, unless the payload itself is invalid (to be double checked), in which case we could add some logic to craft the right pointer/error depending on the payload (/data vs /data/attributes).

@JoeWoodward
Copy link
Author

I think that's the issue though. The following is a valid payload

{
  data: {
    attributes: {}
  }
}

Even if the logic was changed to consider this invalid we can still get null pointers with missing attributes. e.g.

{
  data: {
    type: 'users',
    attributes: {
      email: 'valid@mail.com'
      # name: 'expected but not passed at all, not just null'
    }
  }
}
{
  "errors": [
    {
      "title": "Invalid name",
      "detail": "Name can't be blank",
      "source": {}
    }
  ],
  "jsonapi": {
    "version": "1.0"
  }
}

There is an issue with this PR though; The error isn't necessarily going to be from a member on /data/attributes. I actually looked back at this repo because I was researching sideposting, which I believe isn't through the attributes node so returning source: { pointer: '/data/attributes' } would be invalid.

Maybe there's a way to decipher the pointer from the AM::Errors hash. Need to look into it further

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants