Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Faceting fixes #11191

Merged
merged 3 commits into from
Jun 29, 2023
Merged
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
15 changes: 15 additions & 0 deletions geonode/facets/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
FACET_TYPE_USER = "user"
FACET_TYPE_THESAURUS = "thesaurus"
FACET_TYPE_CATEGORY = "category"
FACET_TYPE_RESOURCETYPE = "resourcetype"

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -86,6 +87,20 @@
"""
pass

def get_topics(self, keys: list, lang="en", **kwargs) -> list:
"""
Return the topics with the requested ids as a list
- list, topic records. A topic record is a dict having these keys:
- key: the key of the items that should be used for filtering
- label: a generic label for the item; the client should try and localize it whenever possible
- localized_label: a localized label for the item
- other facet specific keys
:param keys: the list of the keys of the topics, as returned by the get_facet_items() method
:param lang: the preferred language for the labels
:return: list of items
"""
pass

Check warning on line 102 in geonode/facets/models.py

View check run for this annotation

Codecov / codecov/patch

geonode/facets/models.py#L102

Added line #L102 was not covered by tests

@classmethod
def register(cls, registry, **kwargs) -> None:
"""
Expand Down
22 changes: 20 additions & 2 deletions geonode/facets/providers/category.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

from django.db.models import Count

from geonode.base.models import TopicCategory
from geonode.facets.models import FacetProvider, DEFAULT_FACET_PAGE_SIZE, FACET_TYPE_CATEGORY

logger = logging.getLogger(__name__)
Expand All @@ -38,7 +39,7 @@ def name(self) -> str:
def get_info(self, lang="en") -> dict:
return {
"name": self.name,
"key": "filter{category__identifier}",
"key": "filter{category.identifier}",
"label": "Category",
"type": FACET_TYPE_CATEGORY,
"hierarchical": False,
Expand All @@ -55,7 +56,9 @@ def get_facet_items(
) -> (int, list):
logger.debug("Retrieving facets for %s", self.name)

q = queryset.values("category__identifier", "category__gn_description", "category__fa_class")
q = queryset.values("category__identifier", "category__gn_description", "category__fa_class").filter(
category__isnull=False
)
if topic_contains:
q = q.filter(category__gn_description=topic_contains)
q = q.annotate(count=Count("owner")).order_by("-count")
Expand All @@ -78,6 +81,21 @@ def get_facet_items(

return cnt, topics

def get_topics(self, keys: list, lang="en", **kwargs) -> list:
q = TopicCategory.objects.filter(identifier__in=keys)

logger.debug(" ---> %s\n\n", q.query)
logger.debug(" ---> %r\n\n", q.all())

return [
{
"key": r.identifier,
"label": r.gn_description,
"fa_class": r.fa_class,
}
for r in q.all()
]

@classmethod
def register(cls, registry, **kwargs) -> None:
registry.register_facet_provider(CategoryFacetProvider())
15 changes: 15 additions & 0 deletions geonode/facets/providers/region.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

from django.db.models import Count

from geonode.base.models import Region
from geonode.facets.models import FacetProvider, DEFAULT_FACET_PAGE_SIZE, FACET_TYPE_PLACE

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -77,6 +78,20 @@ def get_facet_items(

return cnt, topics

def get_topics(self, keys: list, lang="en", **kwargs) -> list:
q = Region.objects.filter(code__in=keys).values("code", "name")

logger.debug(" ---> %s\n\n", q.query)
logger.debug(" ---> %r\n\n", q.all())

return [
{
"key": r["code"],
"label": r["name"],
}
for r in q.all()
]

@classmethod
def register(cls, registry, **kwargs) -> None:
registry.register_facet_provider(RegionFacetProvider())
41 changes: 35 additions & 6 deletions geonode/facets/providers/thesaurus.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@

import logging

from django.db.models import Count
from django.db.models import Count, OuterRef, Subquery

from geonode.base.models import ThesaurusKeyword, ThesaurusKeywordLabel
from geonode.facets.models import FacetProvider, DEFAULT_FACET_PAGE_SIZE, FACET_TYPE_THESAURUS

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -65,37 +66,65 @@ def get_facet_items(

filter = {
"tkeywords__thesaurus__identifier": self._name,
"tkeywords__keyword__lang": lang,
}

if topic_contains:
filter["tkeywords__keyword__label__icontains"] = topic_contains

q = (
queryset.filter(**filter)
.values("tkeywords", "tkeywords__keyword__label", "tkeywords__alt_label")
.values("tkeywords", "tkeywords__alt_label")
.annotate(count=Count("tkeywords"))
.annotate(
localized_label=Subquery(
ThesaurusKeywordLabel.objects.filter(keyword=OuterRef("tkeywords"), lang=lang).values("label")
)
)
.order_by("-count")
)

logger.debug(" ---> %s\n\n", q.query)

cnt = q.count()

logger.info("Found %d facets for %s", cnt, self._name)
logger.debug(" ---> %s\n\n", q.query)
logger.debug(" ---> %r\n\n", q.all())

topics = [
{
"key": r["tkeywords"],
"label": r["tkeywords__keyword__label"] or r["tkeywords__alt_label"],
"is_localized": r["tkeywords__keyword__label"] is not None,
"label": r["localized_label"] or r["tkeywords__alt_label"],
"is_localized": r["localized_label"] is not None,
"count": r["count"],
}
for r in q[start:end].all()
]

return cnt, topics

def get_topics(self, keys: list, lang="en", **kwargs) -> list:
q = (
ThesaurusKeyword.objects.filter(id__in=keys)
.values("id", "alt_label")
.annotate(
localized_label=Subquery(
ThesaurusKeywordLabel.objects.filter(keyword=OuterRef("id"), lang=lang).values("label")
)
)
)

logger.debug(" ---> %s\n\n", q.query)
logger.debug(" ---> %r\n\n", q.all())

return [
{
"key": r["id"],
"label": r["localized_label"] or r["alt_label"],
"is_localized": r["localized_label"] is not None,
}
for r in q.all()
]

@classmethod
def register(cls, registry, **kwargs) -> None:
# registry.register_facet_provider(CategoryFacetProvider())
Expand Down
16 changes: 15 additions & 1 deletion geonode/facets/providers/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import logging

from django.contrib.auth import get_user_model
from django.db.models import Count

from geonode.facets.models import FacetProvider, DEFAULT_FACET_PAGE_SIZE, FACET_TYPE_USER
Expand Down Expand Up @@ -70,14 +71,27 @@ def get_facet_items(
{
"key": r["owner"],
"label": r["owner__username"],
"localized_label": r["owner__username"],
"count": r["count"],
}
for r in q[start:end]
]

return cnt, topics

def get_topics(self, keys: list, lang="en", **kwargs) -> list:
q = get_user_model().objects.filter(id__in=keys).values("id", "username")

logger.debug(" ---> %s\n\n", q.query)
logger.debug(" ---> %r\n\n", q.all())

return [
{
"key": r["id"],
"label": r["username"],
}
for r in q.all()
]

@classmethod
def register(cls, registry, **kwargs) -> None:
registry.register_facet_provider(OwnerFacetProvider())
38 changes: 34 additions & 4 deletions geonode/facets/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ def _create_thesauri(cls):
ThesaurusLabel.objects.create(thesaurus=t, lang=tl, label=f"TLabel {tn} {tl}")

for tkn in range(10):
tk = ThesaurusKeyword.objects.create(thesaurus=t, alt_label=f"alt_tkn{tkn}_t{tn}")
tk = ThesaurusKeyword.objects.create(thesaurus=t, alt_label=f"T{tn}_K{tkn}_ALT")
cls.thesauri_k[f"{tn}_{tkn}"] = tk
for tkl in (
"en",
Expand Down Expand Up @@ -193,7 +193,7 @@ def test_facets_rich(self):
{
"name": "category",
"topics": {
"total": 1,
"total": 0,
},
},
{
Expand Down Expand Up @@ -261,8 +261,38 @@ def test_facets_rich(self):
def test_bad_lang(self):
# for thesauri, make sure that by requesting a non-existent language the faceting is still working,
# using the default labels
# TODO impl+test
pass

# run the request with a valid language
req = self.rf.get(reverse("get_facet", args=["t_0"]), data={"lang": "en"})
res: JsonResponse = views.get_facet(req, "t_0")
obj = json.loads(res.content)

self.assertEqual(2, obj["topics"]["total"])
self.assertEqual(10, obj["topics"]["items"][0]["count"])
self.assertEqual("T0_K0_en", obj["topics"]["items"][0]["label"])
self.assertTrue(obj["topics"]["items"][0]["is_localized"])

# run the request with an INVALID language
req = self.rf.get(reverse("get_facet", args=["t_0"]), data={"lang": "ZZ"})
res: JsonResponse = views.get_facet(req, "t_0")
obj = json.loads(res.content)

self.assertEqual(2, obj["topics"]["total"])
self.assertEqual(10, obj["topics"]["items"][0]["count"]) # make sure the count is still there
self.assertEqual("T0_K0_ALT", obj["topics"]["items"][0]["label"]) # check for the alternate label
self.assertFalse(obj["topics"]["items"][0]["is_localized"]) # check for the localization flag

def test_topics(self):
for facet, keys, exp in (
("t_0", [self.thesauri_k["0_0"].id, self.thesauri_k["0_1"].id, -999], 2),
("category", ["C1", "C2", "nomatch"], 0),
("owner", [self.user.id, -100], 1),
("region", ["R0", "R1", "nomatch"], 2),
):
req = self.rf.get(reverse("get_facet_topics", args=[facet]), data={"lang": "en", "key": keys})
res: JsonResponse = views.get_facet_topics(req, facet)
obj = json.loads(res.content)
self.assertEqual(exp, len(obj["topics"]["items"]), f"Unexpected topic count {exp} for facet {facet}")

def test_user_auth(self):
# make sure the user authorization pre-filters the visible resources
Expand Down
1 change: 1 addition & 0 deletions geonode/facets/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,5 @@
urlpatterns = [
path("facets", views.list_facets, name="list_facets"),
path("facets/<facet>", views.get_facet, name="get_facet"),
path("facets/<facet>/topics", views.get_facet_topics, name="get_facet_topics"),
]
21 changes: 20 additions & 1 deletion geonode/facets/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
from rest_framework.authentication import SessionAuthentication, BasicAuthentication
from rest_framework.decorators import api_view, authentication_classes

from django.http import HttpResponseNotFound, JsonResponse
from django.http import HttpResponseNotFound, JsonResponse, HttpResponseBadRequest
from django.urls import reverse

from django.conf import settings
Expand Down Expand Up @@ -116,6 +116,25 @@
return JsonResponse(info)


@api_view(["GET"])
def get_facet_topics(request, facet):
logger.debug("get_facet_topics -> %r", facet)

# retrieve provider for the requested facet
provider: FacetProvider = facet_registry.get_provider(facet)
if not provider:
return HttpResponseNotFound("Facet not found")

Check warning on line 126 in geonode/facets/views.py

View check run for this annotation

Codecov / codecov/patch

geonode/facets/views.py#L126

Added line #L126 was not covered by tests

# parse some query params
lang, lang_requested = _resolve_language(request)
keys = request.query_params.getlist("key")
if not keys:
return HttpResponseBadRequest("Missing key parameter")

Check warning on line 132 in geonode/facets/views.py

View check run for this annotation

Codecov / codecov/patch

geonode/facets/views.py#L132

Added line #L132 was not covered by tests

ret = {"topics": {"items": provider.get_topics(keys, lang=lang)}}
return JsonResponse(ret)


def _get_topics(
provider,
queryset,
Expand Down
Loading