From c09a07be753e01f9914b48a76a989f3a553f1fec Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Tue, 21 Jun 2022 10:12:02 -0700 Subject: [PATCH] Clarify code and add comments. --- kolibri/core/api.py | 6 ++---- kolibri/core/auth/api.py | 2 +- kolibri/core/auth/permissions/auth.py | 2 +- kolibri/core/auth/permissions/base.py | 8 +++++++- 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/kolibri/core/api.py b/kolibri/core/api.py index 37768221cf3..5c02843ecf8 100644 --- a/kolibri/core/api.py +++ b/kolibri/core/api.py @@ -237,12 +237,10 @@ def serialize(self, queryset): ) def serialize_object(self, **filter_kwargs): - queryset = self.get_queryset() try: filter_kwargs = filter_kwargs or self._get_lookup_filter() - return self.serialize( - self.filter_queryset(queryset.filter(**filter_kwargs)) - )[0] + queryset = self.get_queryset().filter(**filter_kwargs) + return self.serialize(self.filter_queryset(queryset))[0] except IndexError: raise Http404( "No %s matches the given query." % queryset.model._meta.object_name diff --git a/kolibri/core/auth/api.py b/kolibri/core/auth/api.py index 92d037681a6..c2f316a91bf 100644 --- a/kolibri/core/auth/api.py +++ b/kolibri/core/auth/api.py @@ -100,7 +100,7 @@ class KolibriAuthPermissionsFilter(filters.BaseFilterBackend): def filter_queryset(self, request, queryset, view): if request.method == "GET": - # only filter down the queryset in the case of the list view being requested + # If a 'GET' method only return readable items to filter down the queryset. return request.user.filter_readable(queryset) # otherwise, return the full queryset, as permission checks will happen object-by-object # (and filtering here then leads to 404's instead of the more correct 403's) diff --git a/kolibri/core/auth/permissions/auth.py b/kolibri/core/auth/permissions/auth.py index cd86c38362a..9068fa18500 100644 --- a/kolibri/core/auth/permissions/auth.py +++ b/kolibri/core/auth/permissions/auth.py @@ -27,7 +27,7 @@ def __init__(self): can_be_read_by=(ADMIN, COACH), can_be_updated_by=(ADMIN,), can_be_deleted_by=None, - collection_field="self", + collection_field=".", ) def user_can_create_object(self, user, obj): diff --git a/kolibri/core/auth/permissions/base.py b/kolibri/core/auth/permissions/base.py index 5bbcdbf968e..3d7c7908574 100644 --- a/kolibri/core/auth/permissions/base.py +++ b/kolibri/core/auth/permissions/base.py @@ -100,6 +100,7 @@ def __init__( :param tuple can_be_updated_by: a tuple of role kinds that should give a user permission to update the object :param tuple can_be_deleted_by: a tuple of role kinds that should give a user permission to delete the object :param str collection_field: the name of the field through which collections can be identified for the object. + (or "." if the object itself is the collection) :param is_syncable: Boolean indicating whether the model is a syncable model, if it is, we can use dataset_id to do quick filtering in some cases, if not, then we need to use the target_field as the source of the dataset_id. """ @@ -108,10 +109,15 @@ def __init__( self.can_be_updated_by = can_be_updated_by self.can_be_deleted_by = can_be_deleted_by self.target_field = target_field - if collection_field == "self": + if collection_field == ".": + # Process special keyword '.' to allow this class to be used with Collection objects, particularly the + # readably by user filter, where the collection is the object itself, so we point at the id, + # and for parent collections to its parent. self.collection_field = "id" self.parent_collection_field = "parent" else: + # Otherwise, we just set the collection field to the value passed in (which defaults to "collection") + # and set the parent field as "__parent" to point to the parent of the FKed collection. self.collection_field = collection_field self.parent_collection_field = "{}__parent".format(self.collection_field) self.is_syncable = is_syncable