diff --git a/wagtail/wagtailcore/models.py b/wagtail/wagtailcore/models.py index 133886aeb9..96fccb3e2a 100644 --- a/wagtail/wagtailcore/models.py +++ b/wagtail/wagtailcore/models.py @@ -1760,6 +1760,10 @@ class CollectionMember(models.Model): on_delete=models.CASCADE ) + search_fields = ( + index.FilterField('collection'), + ) + class Meta: abstract = True diff --git a/wagtail/wagtailcore/permission_policies/collections.py b/wagtail/wagtailcore/permission_policies/collections.py index ded790e428..3e99b6a73f 100644 --- a/wagtail/wagtailcore/permission_policies/collections.py +++ b/wagtail/wagtailcore/permission_policies/collections.py @@ -44,13 +44,10 @@ class CollectionPermissionLookupMixin(object): return collection_permissions.exists() - def _collection_permission_filter( - self, user, actions, path_filter_param='collection__path__startswith' - ): + def _collections_with_perm(self, user, actions): """ - Return a filter object that can be applied to a queryset of this model - so that it only includes instances in collections for which the user has - any of the given permissions + Return a queryset of collections on which this user has a GroupCollectionPermission + record for any of the given actions, either on the collection itself or an ancestor """ # Get the permission objects corresponding to these actions permissions = self._get_permission_objects_for_actions(actions) @@ -66,17 +63,14 @@ class CollectionPermissionLookupMixin(object): if collection_root_paths: # build a filter expression that will filter our model to just those # instances in collections with a path that starts with one of the above - collection_path_filter = Q(**{ - path_filter_param: collection_root_paths[0] - }) + collection_path_filter = Q(path__startswith=collection_root_paths[0]) for path in collection_root_paths[1:]: - collection_path_filter = collection_path_filter | Q(**{ - path_filter_param: path - }) - return collection_path_filter + collection_path_filter = collection_path_filter | Q(path__startswith=path) + + return Collection.objects.filter(collection_path_filter) else: # no matching collections - return Q(pk=-999) + return Collection.objects.none() def _users_with_perm_filter(self, actions, collection=None): """ @@ -175,7 +169,7 @@ class CollectionPermissionPolicy(CollectionPermissionLookupMixin, BaseDjangoAuth else: # filter to just the collections with this permission return self.model.objects.filter( - self._collection_permission_filter(user, actions) + collection__in=list(self._collections_with_perm(user, actions)) ) def users_with_any_permission_for_instance(self, actions, instance): @@ -199,11 +193,7 @@ class CollectionPermissionPolicy(CollectionPermissionLookupMixin, BaseDjangoAuth return Collection.objects.none() else: - return Collection.objects.filter( - self._collection_permission_filter( - user, actions, path_filter_param='path__startswith' - ) - ) + return self._collections_with_perm(user, actions) class CollectionOwnershipPermissionPolicy( @@ -290,11 +280,14 @@ class CollectionOwnershipPermissionPolicy( # - OR in (a descendant of) a collection for which they have 'add' permission, # and are owned by them - change_perm_filter = self._collection_permission_filter(user, ['change']) - add_perm_filter = ( - self._collection_permission_filter(user, ['add']) & - Q(**{self.owner_field_name: user}) + change_perm_filter = Q( + collection__in=list(self._collections_with_perm(user, ['change'])) ) + + add_perm_filter = Q( + collection__in=list(self._collections_with_perm(user, ['add'])) + ) & Q(**{self.owner_field_name: user}) + return self.model.objects.filter(change_perm_filter | add_perm_filter) else: # action is either not recognised, or is the 'add' action which is @@ -344,19 +337,10 @@ class CollectionOwnershipPermissionPolicy( # return collections which are covered by either 'add' or 'change' permissions # (since collections with 'add' permissions can *potentially* contain instances # they own and can therefore edit) - - return Collection.objects.filter( - self._collection_permission_filter( - user, ['add', 'change'], path_filter_param='path__startswith' - ) - ) + return self._collections_with_perm(user, ['add', 'change']) elif 'add' in actions: - return Collection.objects.filter( - self._collection_permission_filter( - user, ['add'], path_filter_param='path__startswith' - ) - ) + return self._collections_with_perm(user, ['add']) else: # action is not recognised, and so non-superusers diff --git a/wagtail/wagtaildocs/models.py b/wagtail/wagtaildocs/models.py index ab42fb1601..739a55f79c 100644 --- a/wagtail/wagtaildocs/models.py +++ b/wagtail/wagtaildocs/models.py @@ -43,7 +43,7 @@ class AbstractDocument(CollectionMember, TagSearchable): objects = DocumentQuerySet.as_manager() - search_fields = TagSearchable.search_fields + ( + search_fields = TagSearchable.search_fields + CollectionMember.search_fields + ( index.FilterField('uploaded_by_user'), ) diff --git a/wagtail/wagtaildocs/tests.py b/wagtail/wagtaildocs/tests.py index a5d3f64676..65db7808f6 100644 --- a/wagtail/wagtaildocs/tests.py +++ b/wagtail/wagtaildocs/tests.py @@ -1161,6 +1161,11 @@ class TestEditOnlyPermissions(TestCase, WagtailTestUtils): # user should be able to see documents not owned by them self.assertContains(response, "Test document") + def test_search(self): + response = self.client.get(reverse('wagtaildocs:index'), {'q': "Hello"}) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.context['query_string'], "Hello") + def test_get_add(self): response = self.client.get(reverse('wagtaildocs:add')) # permission should be denied diff --git a/wagtail/wagtailimages/models.py b/wagtail/wagtailimages/models.py index db6108acc5..5df6cb5aa1 100644 --- a/wagtail/wagtailimages/models.py +++ b/wagtail/wagtailimages/models.py @@ -123,7 +123,7 @@ class AbstractImage(CollectionMember, TagSearchable): return reverse('wagtailimages:image_usage', args=(self.id,)) - search_fields = TagSearchable.search_fields + ( + search_fields = TagSearchable.search_fields + CollectionMember.search_fields + ( index.FilterField('uploaded_by_user'), ) diff --git a/wagtail/wagtailimages/tests/test_admin_views.py b/wagtail/wagtailimages/tests/test_admin_views.py index c7fc364007..3990047564 100644 --- a/wagtail/wagtailimages/tests/test_admin_views.py +++ b/wagtail/wagtailimages/tests/test_admin_views.py @@ -928,6 +928,11 @@ class TestEditOnlyPermissions(TestCase, WagtailTestUtils): # user should be able to see images not owned by them self.assertContains(response, "Test image") + def test_search(self): + response = self.client.get(reverse('wagtailimages:index'), {'q': "Hello"}) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.context['query_string'], "Hello") + def test_get_add(self): response = self.client.get(reverse('wagtailimages:add')) # permission should be denied