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.

Sign up for GitHub

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

Jump to bottom

Fix serialisation validation #84

Closed
duk3luk3 opened this issue Jan 2, 2018 · 4 comments
Closed

Fix serialisation validation #84

duk3luk3 opened this issue Jan 2, 2018 · 4 comments
Labels
bug discussion

Comments

@duk3luk3
Copy link
Contributor

duk3luk3 commented Jan 2, 2018

In the course of #81 it came up that serialise doesn't validate data correctly: #81 (comment)

Do we need different validation logic for different methods (POST/PATCH/PUT?) or is it the same for all?
@wopian
Copy link
Owner

wopian commented Jan 3, 2018 •
edited
Loading

PATCH and DELETE are the only ones that require an id
PATCH, POST and DELETE all require a type (and will be guessed if missing)

So other than the id, the validation should be identical as they all output the same structure.

@duk3luk3
Copy link
Contributor Author

duk3luk3 commented Jan 3, 2018

The validation here:

https://github.com/wopian/kitsu/blob/master/src/util/serialise/index.js#L31-L42

is only relevant for PATCH and PUT, since those are the only methods where you can attach relationships, right?

@wopian
Copy link
Owner

wopian commented Jan 3, 2018

POST can also have relationships (the PUT method is not used by JSON:API though)

http://jsonapi.org/format/#crud-creating

@duk3luk3
Copy link
Contributor Author

duk3luk3 commented Jan 3, 2018

OK, that tells me everything I needed to know I think :-)

I will make a new PR as soon as I find time.

duk3luk3 added a commit to duk3luk3/kitsu that referenced this issue Jan 4, 2018
@duk3luk3
Fix validation of relationship serialisation
a590be8 
Removes alternative check for type attribute on relationship elements.

The check is superfluous, ID check is necessary.

Fixes wopian#84.
@duk3luk3 duk3luk3 mentioned this issue Jan 4, 2018
Merged
duk3luk3 added a commit to duk3luk3/kitsu that referenced this issue Jan 4, 2018
@duk3luk3
Fix validation of relationship serialisation
a6dd34c 
Removes alternative check for type attribute on relationship elements.

The check is superfluous, ID check is necessary.

Fixes wopian#84.
@wopian wopian closed this as completed in #85 Jan 5, 2018
wopian pushed a commit that referenced this issue Jan 5, 2018
@duk3luk3 @wopian
refactor: cleanup serialisation validation (#85)
f99286e 
* Fix validation of relationship serialisation

Removes alternative check for type attribute on relationship elements.

The check is superfluous, ID check is necessary.

Fixes #84.

* Reformat and turn for-loop into map
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug discussion
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@duk3luk3 @wopian