Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions geonode/base/api/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,3 +131,33 @@ def filter_queryset(self, request, queryset, _):
_filter["id__in"] = [_facet.id for _facet in queryset if not _facet.resourcebase_set.exists()]

return queryset.filter(**_filter)


class AdvertisedFilter(BaseFilterBackend):
def filter_queryset(self, request, queryset, view):
# advertised
# if superuser, all resources will be visible, otherwise only the advertised one and
# the resource which the user is owner will be returned

if getattr(view, "action", None) == "retrieve":
return queryset

user = request.user
try:
_filter = request.query_params.get("advertised", "None")
advertised = strtobool(_filter) if _filter.lower() != "all" else "all"
except Exception:
advertised = None
Comment on lines +146 to +150
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The logic for parsing the advertised query parameter can be made more explicit and robust. Using a broad except Exception can hide unexpected errors by catching more than just ValueError from strtobool and AttributeError if the parameter is missing. A clearer approach would be to handle the absence of the parameter separately and then parse its value.

Suggested change
try:
_filter = request.query_params.get("advertised", "None")
advertised = strtobool(_filter) if _filter.lower() != "all" else "all"
except Exception:
advertised = None
_filter = request.query_params.get("advertised")
if _filter is None:
advertised = None
else:
if _filter.lower() == "all":
advertised = "all"
else:
try:
advertised = strtobool(_filter)
except ValueError:
advertised = None


if advertised == "all":
pass
elif advertised is not None:
queryset = queryset.filter(advertised=advertised)
else:
is_admin = user.is_superuser if user and user.is_authenticated else False
if not is_admin and user and not user.is_anonymous:
queryset = (queryset.filter(advertised=True) | queryset.filter(owner=user)).distinct()
elif not user or user.is_anonymous:
queryset = queryset.filter(advertised=True)
Comment on lines +157 to +161
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The conditional logic for filtering based on user authentication and admin status can be simplified for better readability and maintainability. The request.user object provides boolean properties like is_authenticated and is_superuser that can make these checks more direct and remove the need for the is_admin local variable.

Suggested change
is_admin = user.is_superuser if user and user.is_authenticated else False
if not is_admin and user and not user.is_anonymous:
queryset = (queryset.filter(advertised=True) | queryset.filter(owner=user)).distinct()
elif not user or user.is_anonymous:
queryset = queryset.filter(advertised=True)
if not user.is_superuser and user.is_authenticated:
queryset = (queryset.filter(advertised=True) | queryset.filter(owner=user)).distinct()
elif not user.is_authenticated:
queryset = queryset.filter(advertised=True)


return queryset
2 changes: 2 additions & 0 deletions geonode/base/api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
FacetVisibleResourceFilter,
FavoriteFilter,
TKeywordsFilter,
AdvertisedFilter,
)
from geonode.groups.models import GroupProfile, Group
from geonode.security.permissions import get_compact_perms_list, PermSpec
Expand Down Expand Up @@ -308,6 +309,7 @@ class ResourceBaseViewSet(ApiPresetsInitializer, DynamicModelViewSet, Advertised
permission_classes = [IsAuthenticatedOrReadOnly, UserHasPerms]
filter_backends = [
TKeywordsFilter,
AdvertisedFilter,
DynamicFilterBackend,
DynamicSortingFilter,
DynamicSearchFilter,
Expand Down
18 changes: 10 additions & 8 deletions geonode/facets/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
from django.conf import settings

from geonode.base.api.views import ResourceBaseViewSet
from geonode.base.models import ResourceBase
from geonode.facets.models import FacetProvider, DEFAULT_FACET_PAGE_SIZE, facet_registry
from geonode.security.utils import get_visible_resources

Expand Down Expand Up @@ -86,13 +85,16 @@ def _prefilter_topics(cls, request):
filters = {k: vlist for k, vlist in request.query_params.lists() if k.startswith("filter{")}
logger.warning(f"FILTERING BY {filters}")

if filters:
viewset = ResourceBaseViewSet(request=request, format_kwarg={}, kwargs=filters)
viewset.initial(request)
return get_visible_resources(queryset=viewset.filter_queryset(viewset.get_queryset()), user=request.user)
else:
# return ResourceBase.objects
return get_visible_resources(ResourceBase.objects, request.user)
# kwargs will be {} if no filter applied
viewset = ResourceBaseViewSet(
request=request,
format_kwarg={},
kwargs=filters,
)
viewset.initial(request)

queryset = viewset.filter_queryset(viewset.get_queryset())
return get_visible_resources(queryset=queryset, user=request.user)

@classmethod
def _resolve_language(cls, request) -> (str, bool):
Expand Down