Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

add source.pointer support #6

Open
Roanmh opened this issue Oct 24, 2024 · 0 comments
Open

add source.pointer support #6

Roanmh opened this issue Oct 24, 2024 · 0 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@Roanmh
Copy link

Roanmh commented Oct 24, 2024

from the atomic operations spec

The error object SHOULD include a source member with a pointer to the invalid operation

While not a requirement, this is effects the usability of the API greatly. Currently a validation error looks like this:

{
	"errors": [
		{
			"detail": "This field is required.",
			"status": "400",
			"code": "required"
		}
	]
}

The missing pointer makes this error very unhelpful.

Developer Notes

I think there is a simpler way to maintain the interface of get_serializer(): If AtomicOperationView stores the current index (and related arguments) in an instance variable, then get_serializer() can be called without arguments. get_serializer() can reference the instance variables instead of the arguments to determine the answer. We implemented this by overwriting get_serializer() in our subclass.

However, the pointer values will be incorrect. The Serializer for the given operation will generate a pointer without the atomic operation context in mind. e.g. /data/attributes/name instead of /atomic:operations/0/data/attributes/name. For our project we worked around this by implementing a custom exception handler. This callback can access the AtomicOperationsView instance, so we could get the correct index. Perhaps this library can write a thin wrapper around the json-api exception handler to make this correction? I don't know how custom exceptions handlers work with libraries like this.

The matter seems non-trivial for two reasons, 1) as shown below django-rest-framework-json-api does not support any arguments for ExampleView.get_serializer() and 2) the pointer must be adapted to the schema of the atomic operations extension e.g. /{ATOMIC_OPERATIONS}/{idx}/data/<path/to/error>

Field information is captured in the serializer.ValidationError at first, but format_drf_errors looses that information. format_drf_errors calls context["view"].get_serializer() without any arguments, which throws this error (captured via pdb):

(Pdb) context["view"].get_serializer()
*** TypeError: AtomicOperationView.get_serializer() missing 3 required positional arguments: 'idx', 'operation_code', and 'resource_type'

It then falls back to not providing a pointer.

My first guess is that this library needs its own exception_handler, but this would duplicate a lot of logic. I'm not sure how else to approach this without an upstream change to support per-error/per-field arguments on get_serializer(). Perhaps that is a possibility? I'm unfamiliar with the space.

Our use-case doesn't require this improvement yet, so I have to leave it here.

@jokiefer jokiefer added enhancement New feature or request help wanted Extra attention is needed labels Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants