Add view decorators for checking membership, authorship and admin rights

[ana-balica]

The view decorators are used to check if a user is a member of a community, admin of the community and/or author of such models as Resource, CommunityPage, etc.
For instance, to make a change to a model instance, the user should be a member of the community and have the specific permission, so the following decorators will be used in combination with permission_required.

Dependent on #9 (models) and #15 (mock dependency should be moved to requirements-dev.txt).

[econchick]

Just FYI - a few of these import statements will already be there once #9 is merged.

[econchick]

So, stupid question. How can I actually test the GUI/end-user part of this? :)

I like the docstrings. May I suggest you also add :retval: for expected return value/type, and :raises: if/when an error is raised where applicable.

Also, only if you find you have time and are interested, look into doctests. It helps reviewers (like me!) understand more quickly what the expected behavior is, as well as it can be incorporated into testing, and help future developers working on your code.

[econchick]

BTW the decorator code looks good to me, just a couple of minor things to clean up that I commented on.

[econchick]

For line 34 and 36, it's not exactly "pythonic" to end a list with a comma, as well as an extra space. It doesn't break flake, but it looks quite odd.

[ana-balica]

You are right, the comma and a space aren't the right way to do it.
According to PEP8 (scroll a bit down to the list examples) the closing bracket in a multi-line construct should be placed on a new line.
Which one will work for us?

mockup_tests = [
    {"user": self.auth_user_foo, "success": True},
    {"user": self.auth_user_bar, "success": False},
]

Or maybe leave it this way?

mockup_tests = [{"user": self.auth_user_foo, "success": True},
                {"user": self.auth_user_bar, "success": False}]

Usually the comma is left out there, in case a developer will append to the list and it is a good practice to prevent future accidental syntax errors. But if you consider the list will never be updated, I will remove the comma :)

[ana-balica]

The end-user part testing is tricky. I myself have added a trivial view and a URL to it, by changing the decorators around that view and manipulating database data, I was able to check the output for different use-cases (made a matrix to test all possibilities manually).
Here is an example:

@login_required(login_url='/accounts/login/')
@membership_required(Community, 'id__exact', 'id')
def sample_view(request, id=1):
    return HttpResponse("foo")

and in urls.py

url(r'^sample_view/(?P<id>\d+)/$', 'dashboard.views.sample_view'),

And so I get foo or Permission Denied.

doctests are a great idea. I will search on decorator testing with doctests.
For :returns: can I simply say decorator function or a inner function?

[ana-balica]

@econchick -- updated.
With doctests I have added very trivial examples, just to show the usage of the decorator. It doesn't test much, merely shows that the decorator accepts a model param, a lookup field and a string variable name, which should be the same as the one passed to the view.

I was also thinking maybe I should add something like that? But in that case there won't be a return value to test.

@membership_required(Community, "id__exact", "foo")
def foo_view(request, foo):
    return HttpResponse(foo)

I think we could add extra information about that in the Sphinx documentation. What is your opinion on that?

[econchick]

Definitely +1 on adding documentation. Always better to be more clear.

The doc tests look good, but since I am on mobile I can't run them myself. Did doc tests run okay? Also, is the Travis failure of any importance? I can't see what's failing either.

[econchick]

Ah just saw your first reply. Have :returns: show exactly the object that is expected - eg or something, it looks like your updated commit is just fine.

[econchick]

Re trailing comma, i wouldn't anticipate much addition wrt tests, but we can leave it in there to be safe. Also I think ending the brackets on a new line is better/more pythonic.

[ana-balica]

@econchick Doctests run ok. To enable doctests to be discovered by the nose runner, it is necessary merge #18

[ana-balica]

@econchick rebased against current master.

[ana-balica]

Renamed theDashboardTestCase class to a more specific name. I assume we will have DashboardFormsTestCase, DashboardViewsTestCase and all of them will require different setup methods.

If the idea is bad and we want to keep all the tests in one class, I will revert this change.