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

BUGFIX: Added authorization check for MongoEngineListResource. #86

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

Conversation

yjaaidi
Copy link
Contributor

@yjaaidi yjaaidi commented Nov 17, 2014

Hi,

Authorization is not working with MongoEngineListResource.

I added calls to authorized_... in obj_create, obj_delete and obj_update.

@ghost
Copy link

ghost commented Nov 17, 2014

This would resolve #70. Not really sure why you're bumping the version number in this pull request though.

@yjaaidi
Copy link
Contributor Author

yjaaidi commented Nov 17, 2014

Oh yes ! I think it should solve the issue.

Concerning the bump, this is just a mistake because I'm used to bump my hotfixes using git flow.

@danstovall
Copy link
Contributor

I noticed that there were no modifications to the unit tests. I know it is a small change, but I would feel better merging things in if we had a unit test that specifically covered this change. If you add in some tests and sign off on the changes I will merge them in.

@yjaaidi
Copy link
Contributor Author

yjaaidi commented Nov 18, 2014

Oh ok I understand. I've just been lazy as we have unit tests for this on
our app wishtack.com but didn't take time yet to add same tests on
django-tastypie-mongoengine.

Anyway, I'm taking care of this.

On Tue Nov 18 2014 at 4:42:54 PM Dan Stovall [email protected]
wrote:

I noticed that there were no modifications to the unit tests. I know it is
a small change, but I would feel better merging things in if we had a unit
test that specifically covered this change. If you add in some tests and
sign off on the changes I will merge them in.


Reply to this email directly or view it on GitHub
#86 (comment)
.

@mitar
Copy link
Member

mitar commented Nov 30, 2014

Great! Thanks.

Also please sure you try to keep the code style. So compare it with the rest of the code. Like where we have empty lines and not and so on. :-)

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

Successfully merging this pull request may close these issues.

3 participants