Skip to content

fix(facet-count): count library facets by document #4169

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.

Sign up for GitHub

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

Open
anouillz wants to merge 1 commit into
base: staging
Choose a base branch
from
Open

fix(facet-count): count library facets by document #4169

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
51 changes: 20 additions & 31 deletions rero_ils/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2114,38 +2114,27 @@ def _(x):
bool=dict()
),
aggs=dict(
nested_holdings=dict(
nested=dict(
path="nested_holdings"
),
aggs=dict(
organisation=dict(
terms=dict(
field="nested_holdings.organisation.organisation_pid",
size=DOCUMENTS_AGGREGATION_SIZE,
min_doc_count=0,
),
aggs=dict(
library=dict(
terms=dict(
field="nested_holdings.organisation.library_pid",
size=50,
min_doc_count=0,
),
aggs=dict(
location=dict(
terms=dict(
field="nested_holdings.location.pid",
size=100,
min_doc_count=0,
)
)
)
)
)
)
organisation=dict(
terms=dict(
field="organisation_library_location.organisation",
size=DOCUMENTS_AGGREGATION_SIZE,
min_doc_count=0,
)
)
),
library=dict(
terms=dict(
field="organisation_library_location.library",
size=50,
min_doc_count=0,
)
),
location=dict(
terms=dict(
field="organisation_library_location.location",
size=100,
min_doc_count=0,
)
),
)
),
status=dict(
Expand Down
17 changes: 16 additions & 1 deletion rero_ils/facets.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,15 @@ def default_facets_factory(search, index):
(facet_body.get(k)["field"] for k in ["terms", "date_histogram"] if k in facet_body),
None,
)
if not facet_field and index == "documents" and facet_name == "organisation":
facet_field = next(
(
body.get("terms", {}).get("field")
for body in facet_body.get("aggs", {}).values()
if body.get("terms", {}).get("field")
),
None,
)
facet_filter = None
if facet_field:
# get DSL expression of post_filters,
Expand All @@ -77,7 +86,13 @@ def default_facets_factory(search, index):

# Check if 'filter' is defined into the facet configuration. If yes,
# then add this filter to the facet filter previously created.
if "filter" in facet_body:
if index == "documents" and facet_name == "organisation" and "filter" in facet_body:
agg_filter = obj_or_import_string(facet_body["filter"])
if callable(agg_filter):
agg_filter = agg_filter(search, urlkwargs)
facet_body["filter"] = (facet_filter & Q(agg_filter) if facet_filter else Q(agg_filter)).to_dict()
facet_filter = None
elif "filter" in facet_body:
agg_filter = obj_or_import_string(facet_body.pop("filter"))
if callable(agg_filter):
agg_filter = agg_filter(search, urlkwargs)
Expand Down
26 changes: 20 additions & 6 deletions rero_ils/modules/documents/dumpers/indexer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,22 +25,31 @@ def _process_holdings(record, data):
from rero_ils.modules.items.models import ItemNoteTypes

holdings = []
organisations = set()
libraries = set()
locations = set()
search_holdings = HoldingsSearch().filter("term", document__pid=record["pid"]).source().scan()
for holding in search_holdings:
holding = holding.to_dict()
organisation_pid = holding["organisation"]["pid"]
library_pid = holding["library"]["pid"]
location_pid = holding["location"]["pid"]
organisations.add(organisation_pid)
libraries.add(f"{organisation_pid}|{library_pid}")
locations.add(f"{organisation_pid}|{library_pid}|{location_pid}")
hold_data = {
"pid": holding["pid"],
"location": {
"pid": holding["location"]["pid"],
"pid": location_pid,
},
"circulation_category": [
{
"pid": holding["circulation_category"]["pid"],
}
],
"organisation": {
"organisation_pid": holding["organisation"]["pid"],
"library_pid": holding["library"]["pid"],
"organisation_pid": organisation_pid,
"library_pid": library_pid,
},
"holdings_type": holding["holdings_type"],
}
Expand Down Expand Up @@ -86,9 +95,9 @@ def _process_holdings(record, data):
# 'nested' structure.
if acq_date := item.get("acquisition_date"):
item_data["acquisition"] = {
"organisation_pid": holding["organisation"]["pid"],
"library_pid": holding["library"]["pid"],
"location_pid": holding["location"]["pid"],
"organisation_pid": organisation_pid,
"library_pid": library_pid,
"location_pid": location_pid,
"date": acq_date,
}
if public_notes_content := [
Expand All @@ -101,6 +110,11 @@ def _process_holdings(record, data):
if holdings:
data["holdings"] = holdings
data["nested_holdings"] = holdings
data["organisation_library_location"] = {
"organisation": sorted(organisations),
"library": sorted(libraries),
"location": sorted(locations),
}

@staticmethod
def _process_identifiers(record, data):
Expand Down
14 changes: 14 additions & 0 deletions rero_ils/modules/documents/mappings/v7/documents/document-v0.0.1.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,20 @@
"location_pid": {
"type": "keyword"
},
"organisation_library_location": {
"type": "object",
"properties": {
"organisation": {
"type": "keyword"
},
"library": {
"type": "keyword"
},
"location": {
"type": "keyword"
}
}
},
"files": {
"type": "object",
"properties": {
Expand Down
58 changes: 51 additions & 7 deletions rero_ils/modules/documents/serializers/json.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,50 @@
class DocumentJSONSerializer(JSONSerializer, CachedDataSerializerMixin):
"""Serializer for RERO-ILS `Document` records as JSON."""

@staticmethod
def _new_terms_aggregation(buckets=None):
"""Build a terms aggregation result."""
return {
"buckets": buckets or [],
"doc_count_error_upper_bound": 0,
"sum_other_doc_count": 0,
}

@classmethod
def _build_organisation_aggregation(cls, aggregation):
"""Build the organisation aggregation from indexed location paths."""
organisations = {
bucket["key"]: {
**bucket,
"library": cls._new_terms_aggregation(),
}
for bucket in aggregation.get("organisation", {}).get("buckets", [])
}
libraries = {}
for bucket in aggregation.get("library", {}).get("buckets", []):
org_pid, library_pid = bucket["key"].split("|", 1)
if org_pid not in organisations:
continue
library_bucket = {
**bucket,
"key": library_pid,
"location": cls._new_terms_aggregation(),
}
organisations[org_pid]["library"]["buckets"].append(library_bucket)
libraries[(org_pid, library_pid)] = library_bucket

for bucket in aggregation.get("location", {}).get("buckets", []):
org_pid, library_pid, location_pid = bucket["key"].split("|", 2)
if library_bucket := libraries.get((org_pid, library_pid)):
library_bucket["location"]["buckets"].append(
{
**bucket,
"key": location_pid,
}
)

return cls._new_terms_aggregation(list(organisations.values()))

@staticmethod
def _get_view_information():
"""Get the `view_id` and `view_code` to use to build response."""
Expand All @@ -41,7 +85,7 @@ def _get_view_information():
return view_id, view_code

def preprocess_record(self, pid, record, links_factory=None, **kwargs):
"""Prepare a record and persistent identifier for serialization."""
"""Prepare a record and persistent identifier for serialization"""
rec = record

# TODO: uses dumpers
Expand Down Expand Up @@ -144,17 +188,19 @@ def extract_acquisition_date(key, default):
"max": extract_acquisition_date("date_max", datetime.now().strftime("%Y-%m-%d")),
},
}
# organisation aggregation is a nested aggregation, we need to
# remove this useless level.
# Build the public organisation/library/location facet shape from the
# internal aggregation representation.
if aggr_by_org := aggregations.pop("organisation", None):
if aggr_by_org.get("nested_holdings"):
aggregations["organisation"] = aggr_by_org["nested_holdings"]["organisation"]
elif "organisation" in aggr_by_org:
aggregations["organisation"] = self._build_organisation_aggregation(aggr_by_org)
else:
aggregations["organisation"] = aggr_by_org

if aggr_org := aggregations.get("organisation", {}).get("buckets", []):
# nested aggregation, we need to filter empty buckets
aggr_org = filter(lambda agg: agg["doc_count"] > 0, aggr_org)
aggr_org = list(filter(lambda agg: agg["doc_count"] > 0, aggr_org))
aggregations["organisation"]["buckets"] = aggr_org
# load all organisations, libraries and locations in cache
# to avoid multiple queries
self.load_all(
Expand All @@ -166,7 +212,6 @@ def extract_acquisition_date(key, default):
# organisation. We can filter the organisation aggregation to keep
# only this value
if view_code != GLOBAL_VIEW_CODE:
# nested aggregation, we need to filter empty buckets
aggr_org = list(filter(lambda term: term["key"] == view_id, aggr_org))
aggregations["organisation"]["buckets"] = aggr_org
for org in aggr_org:
Expand All @@ -176,7 +221,6 @@ def extract_acquisition_date(key, default):
for lib_term in org["library"].get("buckets", []):
# add aggregation name
lib_term["name"] = self.get_resource(LibrariesSearch(), lib_term["key"])["name"]
# nested aggregation, we need to filter empty buckets
lib_term["location"]["buckets"] = list(
filter(lambda agg: agg["doc_count"] > 0, lib_term["location"]["buckets"])
)
Expand Down
50 changes: 3 additions & 47 deletions tests/api/documents/test_documents_rest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ def test_documents_organisation_facets(client, document, item_lib_martigny, item

assert aggs["organisation"]["buckets"] == [
{
"doc_count": 2,
"doc_count": 1,
"key": "org1",
"library": {
"buckets": [
Expand Down Expand Up @@ -384,7 +384,7 @@ def test_documents_organisation_facets(client, document, item_lib_martigny, item

assert aggs["organisation"]["buckets"] == [
{
"doc_count": 2,
"doc_count": 1,
"key": "org1",
"library": {
"buckets": [
Expand Down Expand Up @@ -423,51 +423,7 @@ def test_documents_organisation_facets(client, document, item_lib_martigny, item
data = get_json(res)
aggs = data["aggregations"]

assert aggs["organisation"]["buckets"] == [
{
"doc_count": 0,
"key": "org1",
"library": {
"buckets": [
{
"doc_count": 0,
"key": "lib1",
"location": {
"buckets": [
{"doc_count": 0, "key": "loc1"},
{
"doc_count": 0,
"key": "loc3",
},
],
"doc_count_error_upper_bound": 0,
"sum_other_doc_count": 0,
},
},
{
"doc_count": 0,
"key": "lib2",
"location": {
"buckets": [
{
"doc_count": 0,
"key": "loc1",
},
{
"doc_count": 0,
"key": "loc3",
},
],
"doc_count_error_upper_bound": 0,
"sum_other_doc_count": 0,
},
},
],
"doc_count_error_upper_bound": 0,
"sum_other_doc_count": 0,
},
}
]
assert aggs["organisation"]["buckets"] == []


@mock.patch(
Expand Down
Loading