From 664d26d1da03f731baec1ef19ab3e0d83393e676 Mon Sep 17 00:00:00 2001 From: Wil Clouser Date: Tue, 1 Mar 2011 21:40:24 -0800 Subject: [PATCH] Rename poorly named has_perm to mysteriously unused but well named method --- apps/access/acl.py | 24 ++++------------ apps/access/tests.py | 28 +++++++++---------- apps/api/handlers.py | 2 +- apps/api/tests/test_oauth.py | 2 +- apps/devhub/helpers.py | 2 +- .../devhub/addons/listing/item_actions.html | 6 ++-- .../devhub/templates/devhub/addons/owner.html | 2 +- .../templates/devhub/addons/payments.html | 2 +- apps/devhub/templates/devhub/base.html | 2 +- .../templates/devhub/versions/edit.html | 2 +- .../templates/devhub/versions/list.html | 4 +-- apps/devhub/views.py | 10 +++---- apps/reviews/views.py | 4 +-- apps/versions/views.py | 2 +- 14 files changed, 40 insertions(+), 52 deletions(-) diff --git a/apps/access/acl.py b/apps/access/acl.py index 87ff6b7248..f889fa55c0 100644 --- a/apps/access/acl.py +++ b/apps/access/acl.py @@ -30,9 +30,12 @@ def action_allowed(request, app, action): def check_ownership(request, obj, require_owner=False): - """Check if request.user has permissions for the object.""" + """ + A convenience function. Check if request.user has permissions + for the object. + """ if isinstance(obj, Addon): - return has_perm(request, obj, viewer=not require_owner) + return check_addon_ownership(request, obj, viewer=not require_owner) elif isinstance(obj, Collection): return check_collection_ownership(request, obj, require_owner) else: @@ -53,22 +56,7 @@ def check_collection_ownership(request, collection, require_owner=False): return False -def check_addon_ownership(request, addon, require_owner=False): - """Check if request.user has owner permissions for the add-on.""" - if not request.user.is_authenticated(): - return False - if action_allowed(request, 'Admin', 'EditAnyAddon'): - return True - - roles = (amo.AUTHOR_ROLE_OWNER, amo.AUTHOR_ROLE_DEV) - if not require_owner: - roles += (amo.AUTHOR_ROLE_VIEWER,) - - return bool(addon.authors.filter(addonuser__role__in=roles, - user=request.amo_user)) - - -def has_perm(request, addon, viewer=False, dev=False, ignore_disabled=False): +def check_addon_ownership(request, addon, viewer=False, dev=False, ignore_disabled=False): """ Check request.amo_user's permissions for the addon. diff --git a/apps/access/tests.py b/apps/access/tests.py index a1544e3910..e2a2a553ff 100644 --- a/apps/access/tests.py +++ b/apps/access/tests.py @@ -10,7 +10,7 @@ from addons.models import Addon, AddonUser from cake.models import Session from users.models import UserProfile -from .acl import match_rules, action_allowed, has_perm +from .acl import match_rules, action_allowed, check_addon_ownership def test_match_rules(): @@ -111,51 +111,51 @@ class TestHasPerm(TestCase): def test_anonymous(self): self.request.user.is_authenticated.return_value = False self.client.logout() - assert not has_perm(self.request, self.addon) + assert not check_addon_ownership(self.request, self.addon) def test_admin(self): self.request.amo_user = self.login_admin() self.request.groups = self.request.amo_user.groups.all() - assert has_perm(self.request, self.addon) + assert check_addon_ownership(self.request, self.addon) def test_disabled(self): self.addon.update(status=amo.STATUS_DISABLED) - assert not has_perm(self.request, self.addon) + assert not check_addon_ownership(self.request, self.addon) self.test_admin() def test_ignore_disabled(self): self.addon.update(status=amo.STATUS_DISABLED) - assert has_perm(self.request, self.addon, ignore_disabled=True) + assert check_addon_ownership(self.request, self.addon, ignore_disabled=True) def test_owner(self): - assert has_perm(self.request, self.addon) + assert check_addon_ownership(self.request, self.addon) self.au.role = amo.AUTHOR_ROLE_DEV self.au.save() - assert not has_perm(self.request, self.addon) + assert not check_addon_ownership(self.request, self.addon) self.au.role = amo.AUTHOR_ROLE_VIEWER self.au.save() - assert not has_perm(self.request, self.addon) + assert not check_addon_ownership(self.request, self.addon) def test_dev(self): - assert has_perm(self.request, self.addon, dev=True) + assert check_addon_ownership(self.request, self.addon, dev=True) self.au.role = amo.AUTHOR_ROLE_DEV self.au.save() - assert has_perm(self.request, self.addon, dev=True) + assert check_addon_ownership(self.request, self.addon, dev=True) self.au.role = amo.AUTHOR_ROLE_VIEWER self.au.save() - assert not has_perm(self.request, self.addon, dev=True) + assert not check_addon_ownership(self.request, self.addon, dev=True) def test_viewer(self): - assert has_perm(self.request, self.addon, viewer=True) + assert check_addon_ownership(self.request, self.addon, viewer=True) self.au.role = amo.AUTHOR_ROLE_DEV self.au.save() - assert has_perm(self.request, self.addon, viewer=True) + assert check_addon_ownership(self.request, self.addon, viewer=True) self.au.role = amo.AUTHOR_ROLE_VIEWER self.au.save() - assert has_perm(self.request, self.addon, viewer=True) + assert check_addon_ownership(self.request, self.addon, viewer=True) diff --git a/apps/api/handlers.py b/apps/api/handlers.py index 6f3f50f01b..660cd21ae0 100644 --- a/apps/api/handlers.py +++ b/apps/api/handlers.py @@ -31,7 +31,7 @@ def check_addon_and_version(f): addon = Addon.objects.id_or_slug(addon_id).get() except: return rc.NOT_HERE - if not acl.has_perm(request, addon, viewer=True): + if not acl.check_addon_ownership(request, addon, viewer=True): return rc.FORBIDDEN if 'version_id' in kw: diff --git a/apps/api/tests/test_oauth.py b/apps/api/tests/test_oauth.py index deb813b5a8..4fc5cc10af 100644 --- a/apps/api/tests/test_oauth.py +++ b/apps/api/tests/test_oauth.py @@ -571,7 +571,7 @@ class TestAddon(BaseOauth): self.token, data={}, content_type=MULTIPART_CONTENT) eq_(r.status_code, 410, r.content) - @patch('access.acl.has_perm') + @patch('access.acl.check_addon_ownership') def test_not_my_addon(self, acl): data = self.create_addon() id = data['id'] diff --git a/apps/devhub/helpers.py b/apps/devhub/helpers.py index ac38b5f8cc..5d23de9dca 100644 --- a/apps/devhub/helpers.py +++ b/apps/devhub/helpers.py @@ -14,7 +14,7 @@ from access import acl from addons.helpers import new_context -register.function(acl.has_perm) +register.function(acl.check_addon_ownership) @register.inclusion_tag('devhub/addons/listing/items.html') diff --git a/apps/devhub/templates/devhub/addons/listing/item_actions.html b/apps/devhub/templates/devhub/addons/listing/item_actions.html index 6788cae6a4..436769a067 100644 --- a/apps/devhub/templates/devhub/addons/listing/item_actions.html +++ b/apps/devhub/templates/devhub/addons/listing/item_actions.html @@ -1,14 +1,14 @@
{{ _('Actions') }}