global: test coverage and docs for items

* BETTER increase test coverage and add docs for items.

Signed-off-by: Aly Badr <[email protected]>

rero_ils/modules/errors.py

@@ -35,3 +35,7 @@ class OrganisationDoesNotExist(RecordsError):
 
 class PolicyNameAlreadyExists(RecordsError):
     """Error raised when the name of the new policy record exists."""
+
+
+class InvalidRecordID(RecordsError):
+    """Error raised when the ID of record in invalid."""

rero_ils/modules/items/api.py

@@ -34,11 +34,13 @@
     NoValidTransitionAvailable
 from invenio_circulation.proxies import current_circulation
 from invenio_circulation.search.api import search_by_pid
+from invenio_i18n.ext import current_i18n
 from invenio_search import current_search
 
 from .models import ItemIdentifier, ItemStatus
 from ..api import IlsRecord, IlsRecordIndexer, IlsRecordsSearch
 from ..documents.api import Document, DocumentsSearch
+from ..errors import InvalidRecordID
 from ..fetchers import id_fetcher
 from ..libraries.api import Library
 from ..loans.api import Loan, LoanAction, \
@@ -63,10 +65,10 @@
 
 
 class ItemsIndexer(IlsRecordIndexer):
-    """."""
+    """Indexing items in Elasticsearch."""
 
     def index(self, record):
-        """."""
+        """Index an item."""
         return_value = super(ItemsIndexer, self).index(record)
         document_pid = record.replace_refs()['document']['pid']
         document = Document.get_record_by_pid(document_pid)
@@ -97,7 +99,7 @@ class Meta:
 
     @classmethod
     def flush(cls):
-        """."""
+        """Flush indexes."""
         current_search.flush_and_refresh(DocumentsSearch.Meta.index)
         current_search.flush_and_refresh(cls.Meta.index)
 
@@ -106,7 +108,7 @@ def add_loans_parameters_and_flush_indexes(function):
     """Add missing action parameters."""
     @wraps(function)
     def wrapper(item, *args, **kwargs):
-        """."""
+        """Executed before loan action."""
         loan = None
         web_request = False
         patron_pid = kwargs.get('patron_pid')
@@ -191,7 +193,7 @@ class Item(IlsRecord):
     }
 
     def dumps_for_circulation(self):
-        """."""
+        """Enhance item information for api_views."""
         item = self.replace_refs()
         data = item.dumps()
 
@@ -213,13 +215,13 @@ def dumps_for_circulation(self):
 
     @classmethod
     def get_document_pid_by_item_pid(cls, item_pid):
-        """."""
+        """Returns document pid from item pid."""
         item = cls.get_record_by_pid(item_pid).replace_refs()
         return item.get('document', {}).get('pid')
 
     @classmethod
     def get_items_pid_by_document_pid(cls, document_pid):
-        """."""
+        """Returns item pisd from document pid."""
         results = ItemsSearch()\
             .filter('term', document__pid=document_pid)\
             .source(['pid']).scan()
@@ -236,7 +238,7 @@ def get_loans_by_item_pid(cls, item_pid):
 
     @classmethod
     def get_loan_pid_with_item_on_loan(cls, item_pid):
-        """."""
+        """Returns loan pid for checked out item."""
         search = search_by_pid(
             item_pid=item_pid, filter_states=['ITEM_ON_LOAN'])
         results = search.source(['loan_pid']).scan()
@@ -247,7 +249,7 @@ def get_loan_pid_with_item_on_loan(cls, item_pid):
 
     @classmethod
     def get_loan_pid_with_item_in_transit(cls, item_pid):
-        """."""
+        """Returns loan pi for in_transit item."""
         search = search_by_pid(
             item_pid=item_pid, filter_states=[
                 "ITEM_IN_TRANSIT_FOR_PICKUP",
@@ -270,7 +272,7 @@ def get_item_by_barcode(cls, barcode=None):
 
     @classmethod
     def get_pendings_loans(cls, library_pid):
-        """."""
+        """Returns list of pending loand for a given library."""
         # check library exists
         lib = Library.get_record_by_pid(library_pid)
         if not lib:
@@ -288,12 +290,11 @@ def get_pendings_loans(cls, library_pid):
 
     @classmethod
     def get_checked_out_loans(cls, patron_pid):
-        """."""
+        """Returns checked out loans for a given patron."""
         # check library exists
         patron = Patron.get_record_by_pid(patron_pid)
         if not patron:
-            raise Exception('Invalid Patron PID')
-
+            raise InvalidRecordID('Invalid Patron PID')
         results = current_circulation.loan_search\
             .source(['loan_pid'])\
             .params(preserve_order=True)\
@@ -306,7 +307,7 @@ def get_checked_out_loans(cls, patron_pid):
 
     @classmethod
     def get_checked_out_items(cls, patron_pid):
-        """."""
+        """Return checked out items for a given patron."""
         loans = cls.get_checked_out_loans(patron_pid)
         returned_item_pids = []
         for loan in loans:
@@ -332,7 +333,7 @@ def get_requests(self):
 
     @classmethod
     def get_requests_to_validate(cls, library_pid):
-        """."""
+        """Returns list of requests to validate for a given library."""
         loans = cls.get_pendings_loans(library_pid)
         returned_item_pids = []
         for loan in loans:
@@ -344,11 +345,11 @@ def get_requests_to_validate(cls, library_pid):
                 yield item, loan
 
     def get_library(self):
-        """Shortcut library."""
+        """Shortcut to the library of the item location."""
         return self.get_location().get_library()
 
     def get_location(self):
-        """Shortcut for item status."""
+        """Shortcut to the location of the item."""
         location_pid = self.replace_refs()['location']['pid']
         return Location.get_record_by_pid(location_pid)
 
@@ -508,13 +509,12 @@ def available(self):
             self.number_of_requests() == 0
 
     def get_item_end_date(self):
-        """Get item due date a given item."""
+        """Get item due date for a given item."""
         loan = get_loan_for_item(self.pid)
         if loan:
             end_date = loan['end_date']
             # due_date = datetime.strptime(end_date, '%Y-%m-%d')
             from ...filter import format_date_filter
-            from invenio_i18n.ext import current_i18n
 
             due_date = format_date_filter(
                 end_date,
@@ -663,7 +663,7 @@ def cancel_loan(self, current_loan, **kwargs):
         }
 
     def get_owning_pickup_location_pid(self):
-        """."""
+        """Returns the pickup location for the item owning location."""
         library = self.get_library()
         return library.get_pickup_location_pid()
 

rero_ils/modules/items/api_views.py

@@ -63,7 +63,7 @@ def decorated_view(*args, **kwargs):
 
 
 def jsonify_error(func):
-    """."""
+    """Jsonify errors."""
     @wraps(func)
     def decorated_view(*args, **kwargs):
         try:
@@ -77,7 +77,7 @@ def decorated_view(*args, **kwargs):
 
 
 def jsonify_action(func):
-    """."""
+    """Jsonify loan actions."""
     @wraps(func)
     def decorated_view(*args, **kwargs):
         try:

rero_ils/modules/items/cli.py

@@ -64,7 +64,7 @@ def __len__(self):
 @click.command('reindex_items')
 @with_appcontext
 def reindex_items():
-    """."""
+    """Reindexing of item."""
     ids = Item.get_all_ids()
     with click.progressbar(ids, length=len(ids)) as bar:
         for uuid in bar:

rero_ils/modules/items/jsonschemas/__init__.py

@@ -22,4 +22,4 @@
 # waive the privileges and immunities granted to it by virtue of its status
 # as an Intergovernmental Organization or submit itself to any jurisdiction.
 
-"""JSON schemas."""
+"""Item JSON schemas."""

rero_ils/modules/items/mappings/__init__.py

@@ -22,4 +22,4 @@
 # waive the privileges and immunities granted to it by virtue of its status
 # as an Intergovernmental Organization or submit itself to any jurisdiction.
 
-"""Elasticsearch mappings."""
+"""Item Elasticsearch mappings."""

rero_ils/modules/items/mappings/v6/__init__.py

@@ -22,4 +22,4 @@
 # waive the privileges and immunities granted to it by virtue of its status
 # as an Intergovernmental Organization or submit itself to any jurisdiction.
 
-"""Elasticsearch mappings."""
+"""Item Elasticsearch mappings."""

rero_ils/modules/items/views.py

@@ -90,7 +90,7 @@ def decorated_view(*args, **kwargs):
 
 
 def jsonify_error(func):
-    """."""
+    """Jsonify the errors."""
     @wraps(func)
     def decorated_view(*args, **kwargs):
         try:

tests/api/test_items_rest.py

@@ -23,9 +23,6 @@
 
 """Tests REST API items."""
 
-# import json
-# from utils import get_json, to_relative_url
-
 import json
 from datetime import datetime, timedelta
 
@@ -172,7 +169,7 @@ def test_items_failed_actions(client, patron_martigny_no_email,
                               loc_public_martigny,
                               item_type_standard_martigny,
                               item_lib_martigny, json_header):
-    """."""
+    """Test item failed actions."""
     login_user_via_session(client, librarian_martigny_no_email.user)
     item = item_lib_martigny
     item_pid = item.pid
@@ -221,7 +218,7 @@ def test_items_simple_checkout(client, librarian_martigny_no_email,
                                item_type_standard_martigny,
                                item_lib_martigny, json_header,
                                circulation_policies):
-    """."""
+    """Test item checkout."""
     login_user_via_session(client, librarian_martigny_no_email.user)
     item = item_lib_martigny
     item_pid = item.pid
@@ -446,7 +443,7 @@ def test_items_requests(client, librarian_martigny_no_email,
                         item_type_standard_martigny,
                         item_lib_martigny, json_header,
                         circulation_policies):
-    """."""
+    """Test requesting an item and validation."""
     login_user_via_session(client, librarian_martigny_no_email.user)
     item = item_lib_martigny
     item_pid = item.pid
@@ -609,7 +606,7 @@ def test_items_cancel_request(client, librarian_martigny_no_email,
                               item_type_standard_martigny,
                               item_lib_martigny, json_header,
                               circulation_policies):
-    """."""
+    """Test cancel item request."""
     login_user_via_session(client, librarian_martigny_no_email.user)
     item = item_lib_martigny
     item_pid = item.pid
@@ -659,7 +656,7 @@ def test_items_extend(client, librarian_martigny_no_email,
                       item_type_standard_martigny,
                       item_lib_martigny, json_header,
                       circulation_policies):
-    """."""
+    """Test item renewal."""
     login_user_via_session(client, librarian_martigny_no_email.user)
     item = item_lib_martigny
     item_pid = item.pid
@@ -733,7 +730,7 @@ def test_items_lose(client, librarian_martigny_no_email,
                     item_type_standard_martigny,
                     item_lib_martigny, json_header,
                     circulation_policies):
-    """."""
+    """Test item action lose."""
     login_user_via_session(client, librarian_martigny_no_email.user)
     item = item_lib_martigny
     item_pid = item.pid
@@ -821,7 +818,7 @@ def test_items_receive(client, librarian_martigny_no_email,
                        item_type_standard_martigny,
                        item_lib_martigny, json_header,
                        circulation_policies):
-    """."""
+    """Test item receive."""
     login_user_via_session(client, librarian_martigny_no_email.user)
     item = item_lib_martigny
     item_pid = item.pid
@@ -892,7 +889,7 @@ def test_items_automatic_checkin(client, librarian_martigny_no_email,
                                  item_type_standard_martigny,
                                  item_lib_martigny, json_header,
                                  circulation_policies):
-    """."""
+    """Test automatic checkin for items."""
     login_user_via_session(client, librarian_martigny_no_email.user)
     item = item_lib_martigny
     item_pid = item.pid
@@ -1012,7 +1009,7 @@ def test_items_no_extend(client, librarian_martigny_no_email,
                          item_type_standard_martigny,
                          item_lib_martigny, json_header,
                          circ_policy_short_martigny):
-    """."""
+    """Test items when no renewals is possible."""
     login_user_via_session(client, librarian_martigny_no_email.user)
     item = item_lib_martigny
     item_pid = item.pid
@@ -1085,7 +1082,7 @@ def test_items_deny_requests(client, librarian_martigny_no_email,
                              item_type_standard_martigny,
                              item_lib_martigny, json_header,
                              circ_policy_short_martigny):
-    """."""
+    """Test items when requests are denied."""
     location = loc_public_martigny
     circ_policy_short_martigny['allow_requests'] = False
     circ_policy_short_martigny.update(
@@ -1126,7 +1123,7 @@ def test_extend_possible_actions(client, item_lib_martigny,
                                  librarian_martigny_no_email,
                                  patron_martigny_no_email,
                                  circ_policy_short_martigny):
-    """Extend action changes according to params of cipo"""
+    """Extend action changes according to params of cipo."""
     login_user_via_session(client, librarian_martigny_no_email.user)
     circ_policy = circ_policy_short_martigny
     item = item_lib_martigny
@@ -1200,7 +1197,7 @@ def test_item_possible_actions(client, item_lib_martigny,
                                librarian_martigny_no_email,
                                patron_martigny_no_email,
                                circulation_policies):
-    """Possible action changes according to params of cipo"""
+    """Possible action changes according to params of cipo."""
     login_user_via_session(client, librarian_martigny_no_email.user)
     item = item_lib_martigny
     patron_pid = patron_martigny_no_email.pid
@@ -1258,7 +1255,7 @@ def test_items_extend_rejected(client, librarian_martigny_no_email,
                                item_type_standard_martigny,
                                item_lib_martigny, json_header,
                                circ_policy_short_martigny):
-    """."""
+    """Test items when extend will be rejected."""
     login_user_via_session(client, librarian_martigny_no_email.user)
     item = item_lib_martigny
     item_pid = item.pid
@@ -1341,7 +1338,7 @@ def test_items_extend_end_date(client, librarian_martigny_no_email,
                                item_type_standard_martigny,
                                item_lib_martigny, json_header,
                                circ_policy_short_martigny):
-    """."""
+    """Test correct renewal due date for items."""
     login_user_via_session(client, librarian_martigny_no_email.user)
     item = item_lib_martigny
     item_pid = item.pid