stats: add tests

* Splits the classes to compute statistics into several files. * Fixes some spelling. * Renames abstract class which is not abstract. * Removes useless parameters in the statics methods. * Get the location names using only one es query. * Creates a loan search query used for several statistics. * Fixes delete items statistics. * Adds tests for statistics. * Fixes some test fixtures. * Adds a type filter to the operation logs search to avoid unwanted documents. Co-Authored-by: Johnny Mariéthoz <[email protected]> Co-Authored-by: Bertrand Zuchuat <[email protected]>
rero · Sep 21, 2023 · 0dab6d8 · 0dab6d8
1 parent 770dc5e
commit 0dab6d8
Show file tree

Hide file tree

Showing 25 changed files with 1,191 additions and 752 deletions.
diff --git a/pyproject.toml b/pyproject.toml
@@ -329,7 +329,7 @@ patron_id = "rero_ils.modules.patrons.api:patron_id_fetcher"
 patron_transaction_event_id = "rero_ils.modules.patron_transaction_events.api:patron_transaction_event_id_fetcher"
 patron_transaction_id = "rero_ils.modules.patron_transactions.api:patron_transaction_id_fetcher"
 patron_type_id = "rero_ils.modules.patron_types.api:patron_type_id_fetcher"
-stat_id = "rero_ils.modules.stats.api:stat_id_fetcher"
+stat_id = "rero_ils.modules.stats.api.api:stat_id_fetcher"
 template_id = "rero_ils.modules.templates.api:template_id_fetcher"
 entity_id = "rero_ils.modules.entities.fetchers:id_fetcher"
 vendor_id = "rero_ils.modules.vendors.api:vendor_id_fetcher"
@@ -360,7 +360,7 @@ patron_id = "rero_ils.modules.patrons.api:patron_id_minter"
 patron_transaction_event_id = "rero_ils.modules.patron_transaction_events.api:patron_transaction_event_id_minter"
 patron_transaction_id = "rero_ils.modules.patron_transactions.api:patron_transaction_id_minter"
 patron_type_id = "rero_ils.modules.patron_types.api:patron_type_id_minter"
-stat_id = "rero_ils.modules.stats.api:stat_id_minter"
+stat_id = "rero_ils.modules.stats.api.api:stat_id_minter"
 template_id = "rero_ils.modules.templates.api:template_id_minter"
 entity_id = "rero_ils.modules.entities.minters:id_minter"
 vendor_id = "rero_ils.modules.vendors.api:vendor_id_minter"

diff --git a/rero_ils/config.py b/rero_ils/config.py
@@ -127,7 +127,7 @@
 from .modules.patrons.models import CommunicationChannel
 from .modules.patrons.permissions import PatronPermissionPolicy
 from .modules.selfcheck.permissions import seflcheck_permission_factory
-from .modules.stats.api import Stat
+from .modules.stats.api.api import Stat
 from .modules.stats.permissions import StatisticsPermissionPolicy
 from .modules.templates.permissions import TemplatePermissionPolicy
 from .modules.users.api import get_profile_countries, \
@@ -773,9 +773,9 @@ def _(x):
         pid_type='stat',
         pid_minter='stat_id',
         pid_fetcher='stat_id',
-        search_class='rero_ils.modules.stats.api:StatsSearch',
+        search_class='rero_ils.modules.stats.api.api:StatsSearch',
         search_index='stats',
-        indexer_class='rero_ils.modules.stats.api:StatsIndexer',
+        indexer_class='rero_ils.modules.stats.api.api:StatsIndexer',
         search_type=None,
         record_serializers={
             'application/json': 'rero_ils.modules.serializers:json_v1_response',
@@ -792,8 +792,8 @@ def _(x):
         record_loaders={
             'application/json': lambda: Stat(request.get_json()),
         },
-        record_class='rero_ils.modules.stats.api:Stat',
-        item_route='/stats/<pid(stat, record_class="rero_ils.modules.stats.api:Stat"):pid_value>',
+        record_class='rero_ils.modules.stats.api.api:Stat',
+        item_route='/stats/<pid(stat, record_class="rero_ils.modules.stats.api.api:Stat"):pid_value>',
         default_media_type='application/json',
         max_result_window=MAX_RESULT_WINDOW,
         list_permission_factory_imp=lambda record: StatisticsPermissionPolicy('search', record=record),
@@ -2829,7 +2829,7 @@ def _(x):
         pid_type='stat',
         route='/stats/<pid_value>',
         template='rero_ils/detailed_view_stats.html',
-        record_class='rero_ils.modules.stats.api:Stat',
+        record_class='rero_ils.modules.stats.api.api:Stat',
         view_imp='rero_ils.modules.stats.views.stats_view_method',
         permission_factory_imp='rero_ils.modules.stats.permissions:stats_ui_permission_factory',
     )
@@ -2897,7 +2897,7 @@ def _(x):
 # Statistics Configuration
 # ========================
 # Compute the stats with a timeframe given in monthes
-RERO_ILS_STATS_BILLING_TIMEFRAME_IN_MONTHES = 3
+RERO_ILS_STATS_BILLING_TIMEFRAME_IN_MONTHS = 3
 
 
 # =============================================================================

diff --git a/rero_ils/modules/documents/api.py b/rero_ils/modules/documents/api.py
@@ -97,6 +97,16 @@ def by_entity(self, entity, subjects=True, imported_subjects=True,
             filters |= Q('term', **{field: entity.pid})
         return self.filter(filters)
 
+    def by_library_pid(self, library_pid):
+        """Build a search to get hits related to a library pid.
+
+        :param library_pid: string - the library pid to filter with
+        :returns: An ElasticSearch query to get hits related the entity.
+        :rtype: `elasticsearch_dsl.Search`
+        """
+        return self.filter(
+            'term', holdings__organisation__library_pid=library_pid)
+
 
 class Document(IlsRecord):
     """Document class."""

diff --git a/rero_ils/modules/libraries/api.py b/rero_ils/modules/libraries/api.py
@@ -64,6 +64,15 @@ class Meta():
 
         default_filter = None
 
+    def by_organisation_id(self, organisation_pid):
+        """Build a search to get hits related to an organisation pid.
+
+        :param organisation_pid: string - the organisation pid to filter with
+        :returns: An ElasticSearch query to get hits related the entity.
+        :rtype: `elasticsearch_dsl.Search`
+        """
+        return self.filter('term', organisation__pid=organisation_pid)
+
 
 class Library(IlsRecord):
     """Library class."""

diff --git a/rero_ils/modules/loans/logs/api.py b/rero_ils/modules/loans/logs/api.py
@@ -17,16 +17,32 @@
 
 """Loans logs API."""
 
-from invenio_search import RecordsSearch
 
-from rero_ils.modules.operation_logs.logs.api import \
-    AbstractSpecificOperationLog
+from rero_ils.modules.operation_logs.api import OperationLog, \
+    OperationLogsSearch
+from rero_ils.modules.operation_logs.logs.api import SpecificOperationLog
 
 from ...items.api import Item
 from ...patrons.api import Patron, current_librarian
 
 
-class LoanOperationLog(AbstractSpecificOperationLog):
+class LoanOperationLogsSearch(OperationLogsSearch):
+    """RecordsSearch for LoanOperationLogs."""
+
+    def get_logs_by_trigger(self, triggers, date_range=None):
+        """Get the operation logs base es search.
+
+        :param triggers: list[str] - loan triggers value to filter
+        :return: an elasticsearch dsl search query
+        """
+        query = self.filter('term', record__type='loan')\
+            .filter('terms', loan__trigger=triggers)
+        if date_range:
+            query = query.filter('range', date=date_range)
+        return query
+
+
+class LoanOperationLog(OperationLog, SpecificOperationLog):
     """Operation log for loans."""
 
     @classmethod
@@ -97,28 +113,13 @@ def create(cls, data, id_=None, index_refresh='false', **kwargs):
             }
         return super().create(log, index_refresh=index_refresh)
 
-    @classmethod
-    def get_logs_by_record_pid(cls, pid):
-        """Get all logs for a given record PID.
-
-        :param str pid: record PID.
-        :returns: List of logs.
-        """
-        return list(
-            RecordsSearch(index=cls.index_name).filter(
-                'bool', must={
-                    'exists': {
-                        'field': 'loan'
-                    }
-                }).filter('term', record__value=pid).scan())
-
     @classmethod
     def anonymize_logs(cls, loan_pid):
         """Anonymize all logs corresponding to the given loan.
 
         :param loan_pid: Loan PID.
         """
-        for log in cls.get_logs_by_record_pid(loan_pid):
+        for log in OperationLogsSearch().get_logs_by_record_pid(loan_pid):
             record = log.to_dict()
             record['loan']['patron']['name'] = 'anonymized'
             record['loan']['patron']['pid'] = 'anonymized'

diff --git a/rero_ils/modules/locations/api.py b/rero_ils/modules/locations/api.py
@@ -58,6 +58,16 @@ class Meta:
 
         default_filter = None
 
+    def location_pids(self, library_pid, source='pid'):
+        """Locations pid for given library.
+
+        :param library_pid: string - the library to filter with
+        :return: list of pid locations
+        :rtype: list
+        """
+        return [location.pid for location in self.filter(
+                'term', library__pid=library_pid).source(source).scan()]
+
 
 class Location(IlsRecord):
     """Location class."""

diff --git a/rero_ils/modules/notifications/logs/api.py b/rero_ils/modules/notifications/logs/api.py
@@ -18,13 +18,13 @@
 
 """Notification logs API."""
 
-from rero_ils.modules.operation_logs.logs.api import \
-    AbstractSpecificOperationLog
+from rero_ils.modules.operation_logs.api import OperationLog
+from rero_ils.modules.operation_logs.logs.api import SpecificOperationLog
 
 from ..models import RecipientType
 
 
-class NotificationOperationLog(AbstractSpecificOperationLog):
+class NotificationOperationLog(OperationLog, SpecificOperationLog):
     """Operation log for notification."""
 
     @classmethod

diff --git a/rero_ils/modules/operation_logs/api.py b/rero_ils/modules/operation_logs/api.py
@@ -31,7 +31,7 @@
 
 
 class OperationLogsSearch(IlsRecordsSearch):
-    """RecordsSearch for Notifications."""
+    """RecordsSearch for OperationLogs."""
 
     class Meta:
         """Search only on Notifications index."""
@@ -54,6 +54,20 @@ def get_logs_by_notification_pid(self, notif_pid):
         for hit in query.scan():
             yield hit.to_dict()
 
+    def get_logs_by_record_pid(cls, pid):
+        """Get all logs for a given record PID.
+
+        :param str pid: record PID.
+        :returns: List of logs.
+        """
+        return list(
+            self.filter(
+                'bool', must={
+                    'exists': {
+                        'field': 'loan'
+                    }
+                }).filter('term', record__value=pid).scan())
+
 
 def operation_log_id_fetcher(record_uuid, data):
     """Fetch an Organisation record's identifier.

diff --git a/rero_ils/modules/operation_logs/logs/api.py b/rero_ils/modules/operation_logs/logs/api.py
@@ -21,12 +21,11 @@
 import hashlib
 
 from rero_ils.modules.locations.api import Location
-from rero_ils.modules.operation_logs.api import OperationLog
 from rero_ils.modules.utils import extracted_data_from_ref
 
 
-class AbstractSpecificOperationLog(OperationLog):
-    """Abstract Specific Operation log."""
+class SpecificOperationLog():
+    """Specific Operation log."""
 
     @classmethod
     def _get_patron_data(cls, patron):