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

Excluding primary key from api view breaks POST #400

Closed
graup opened this Issue Feb 6, 2015 · 3 comments

Comments

Projects
None yet
2 participants
@graup
Contributor

graup commented Feb 6, 2015

I tried excluding the primary key in create_api as mentioned here:

http://flask-restless.readthedocs.org/en/latest/customizing.html#specifying-which-columns-are-provided-in-responses

However, this breaks POSTing to this view, as the post handler tries to get the pk from the database object after inserting.

File "flask_restless/views.py", line 1390, in post
    primary_key = result[pk_name]
KeyError: 'id'

I was going to submit a failing unit test, but upon further thinking it actually makes sense that this doesn't work. After all, how should a client know which instance it created if the response may not contain the id?

So I guess, it would just be useful to add a hint to the docs explaining this. Something like "Make sure the primary key field is included if you intend to allow POST".

@graup graup changed the title from Excluding primary key from view breaks POST to Excluding primary key from api view breaks POST Feb 6, 2015

@jfinkels

This comment has been minimized.

Owner

jfinkels commented Feb 6, 2015

This should definitely be in the documentation, but it's also a bug in the code. The code should raise an error and inform the user that he or she is not allowed to exclude primary keys.

@graup

This comment has been minimized.

Contributor

graup commented Feb 6, 2015

I would add: that he or she is not allowed to exclude primary keys, as long as other methods than GET are allowed. It might make sense to some users to not expose keys on a read only API.

If you don't mind I'd try coming up with a pull request for raising the error (incl. unit test).

graup added a commit to graup/flask-restless that referenced this issue Feb 6, 2015

graup added a commit to graup/flask-restless that referenced this issue Feb 6, 2015

graup added a commit to graup/flask-restless that referenced this issue Feb 6, 2015

graup added a commit to graup/flask-restless that referenced this issue Feb 6, 2015

jfinkels added a commit that referenced this issue Feb 8, 2015

@jfinkels

This comment has been minimized.

Owner

jfinkels commented Feb 8, 2015

I have merged in a fix for this at 3fd6c8a.

@jfinkels jfinkels closed this Feb 8, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment