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

Delete_relationship does not raise BadRequestError when data is not array of dictionaries #53

Open
kaitj91 opened this issue Mar 27, 2017 · 0 comments

Comments

@kaitj91
Copy link
Contributor

kaitj91 commented Mar 27, 2017

It is very important that the library is able to return some sort of response that let's the user know they submitted an invalid request by raising a BadRequestError and returning a 400 and some sort of description explaining why.

Through testing delete_relationship, I found that we are not raising a BadRequestError when we send a payload where data is not an array of dictionaries.

For example:

    def test_delete_one_to_many_relationship_when_data_has_invalid_format(self):
        """Delete a relationship when the payload data is not an array of dictionaries returns 400.

        A BadRequestError is raised.
        """
        user = models.User(
            first='Sally', last='Smith',
            password='password', username='SallySmith1')
        self.session.add(user)
        blog_post = models.Post(
            title='This Is A Title', content='This is the content',
            author_id=user.id, author=user)
        self.session.add(blog_post)
        comment = models.Comment(
            content='This is a comment', author_id=user.id,
            post_id=blog_post.id, author=user, post=blog_post)
        self.session.add(comment)
        self.session.commit()
        payload = {
            'data': ['test']
        }

        with self.assertRaises(errors.BadRequestError) as error:
            models.serializer.delete_relationship(
                self.session, payload, 'posts', blog_post.id, 'comments')

       
        self.assertEqual(error.exception.detail, 'data must be an array of objects')
        self.assertEqual(error.exception.status_code, 400)

This test fails because there is no check for this in place in delete_relationship. Thus when I try to run this test a TypeError occurs. Specifically the TypeError is occuring because we are failing to check

if not isinstance(item, dict):
    raise BadRequestError('data must be an array of objects')

at the beginning of the for loop below:

        for item in data['data']:
            item = self._fetch_resource(session, item['type'], item['id'],
                                        Permissions.EDIT)

Because we do not have this check a TypeError occurs because we are trying to look up item['type'] and because it is a list we would need to look up by integer indices and not strings.
This is an easy fix that would provide an informative error message to users who send badly formatted payloads.

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

No branches or pull requests

1 participant