fix: loan and items

* Corrects small loan and item files.
* Better error handling and logging for ref resolvers.

Co-Authored-by: Peter Weber <[email protected]>

rero_ils/modules/cli.py

@@ -302,9 +302,18 @@ def create(infile, pid_type, schema, verbose, dbcommit, reindex, append):
     if append:
         identifier = record_class.provider.identifier
         try:
-            append_fixtures_new_identifiers(identifier, sorted(pids), pid_type)
+            append_fixtures_new_identifiers(
+                identifier,
+                sorted(pids, key=lambda x: int(x)),
+                pid_type
+            )
         except Exception as err:
-            pass
+            click.secho(
+                "ERROR append fixtures new identifiers: {err}".format(
+                    err=err
+                ),
+                fg='red'
+            )
 
 
 fixtures.add_command(create)

rero_ils/modules/documents/jsonresolver.py

@@ -19,20 +19,11 @@
 
 
 import jsonresolver
-from flask import current_app
-from invenio_pidstore.models import PersistentIdentifier, PIDStatus
+
+from ..jsonresolver import ref_resolver
 
 
 @jsonresolver.route('/api/documents/<pid>', host='ils.rero.ch')
 def document_resolver(pid):
     """Document resolver."""
-    persistent_id = PersistentIdentifier.get('doc', pid)
-    if persistent_id.status == PIDStatus.REGISTERED:
-        return dict(pid=persistent_id.pid_value)
-    current_app.logger.error(
-        'Doc resolver error: /api/documents/{pid} {persistent_id}'.format(
-            pid=pid,
-            persistent_id=persistent_id
-        )
-    )
-    raise Exception('unable to resolve')
+    return ref_resolver('doc', pid)

rero_ils/modules/holdings/jsonresolver.py

@@ -18,20 +18,11 @@
 """Holding resolver."""
 
 import jsonresolver
-from flask import current_app
-from invenio_pidstore.models import PersistentIdentifier, PIDStatus
+
+from ..jsonresolver import ref_resolver
 
 
 @jsonresolver.route('/api/holdings/<pid>', host='ils.rero.ch')
 def holding_resolver(pid):
     """Resolver for holding record."""
-    persistent_id = PersistentIdentifier.get('hold', pid)
-    if persistent_id.status == PIDStatus.REGISTERED:
-        return dict(pid=persistent_id.pid_value)
-    current_app.logger.error(
-        'Doc resolver error: /api/holdings/{pid} {persistent_id}'.format(
-            pid=pid,
-            persistent_id=persistent_id
-        )
-    )
-    raise Exception('unable to resolve')
+    return ref_resolver('hold', pid)

rero_ils/modules/item_types/jsonresolver.py

@@ -19,13 +19,11 @@
 
 
 import jsonresolver
-from invenio_pidstore.models import PersistentIdentifier, PIDStatus
+
+from ..jsonresolver import ref_resolver
 
 
 @jsonresolver.route('/api/item_types/<pid>', host='ils.rero.ch')
 def item_type_resolver(pid):
     """Item type resolver."""
-    persistent_id = PersistentIdentifier.get('itty', pid)
-    if persistent_id.status == PIDStatus.REGISTERED:
-        return dict(pid=persistent_id.pid_value)
-    raise Exception('unable to resolve')
+    return ref_resolver('itty', pid)

rero_ils/modules/items/cli.py

@@ -67,31 +67,17 @@ def reindex_items():
 
 
 @click.command('create_items')
-@click.option(
-    '-c', '--count', 'count',
-    type=click.INT, default=-1, help='default=for all records'
-)
-@click.option(
-    '-i', '--itemscount', 'itemscount',
-    type=click.INT, default=1, help='default=1'
-)
-@click.option(
-    '-m', '--missing', 'missing', type=click.INT, default=5, help='default=5'
-)
+@click.option('-c', '--count', 'count',
+              type=click.INT, default=-1, help='default=for all records')
+@click.option('-i', '--itemscount', 'itemscount',
+              type=click.INT, default=1, help='default=1')
+@click.option('-m', '--missing', 'missing',
+              type=click.INT, default=5, help='default=5')
 # @click.argument('output', type=click.File('w'))
-@click.option(
-    '-t',
-    '--items_f',
-    'items_f',
-    help='Items output file.')
-@click.option(
-    '-h',
-    '--holdings_f',
-    'holdings_f',
-    help='Holdings output file.')
+@click.option('-t', '--items_f', 'items_f', help='Items output file.')
+@click.option('-h', '--holdings_f', 'holdings_f', help='Holdings output file.')
 @with_appcontext
-def create_items(
-        count, itemscount, missing, items_f, holdings_f):
+def create_items(count, itemscount, missing, items_f, holdings_f):
     """Create circulation items."""
     def generate(count, itemscount, missing):
 
@@ -101,8 +87,10 @@ def generate(count, itemscount, missing):
             count = len(documents_pids)
 
         click.secho(
-            'Starting generating {0} items, random {1} ...'.format(
-                count, itemscount),
+            'Starting generating {count} items, random {itemsc} ...'.format(
+                count=count,
+                itemsc=itemscount
+            ),
             fg='green',
         )
 
@@ -152,6 +140,7 @@ def generate(count, itemscount, missing):
                     )
                     item_pid += 1
                     yield item, new_holding
+
     items = []
     holdings = []
     with open(holdings_f, 'w', encoding='utf-8') as holdings_file:

rero_ils/modules/items/jsonresolver.py

@@ -19,13 +19,11 @@
 
 
 import jsonresolver
-from invenio_pidstore.models import PersistentIdentifier, PIDStatus
+
+from ..jsonresolver import ref_resolver
 
 
 @jsonresolver.route('/api/items/<pid>', host='ils.rero.ch')
 def item_resolver(pid):
     """Item resolver."""
-    persistent_id = PersistentIdentifier.get('item', pid)
-    if persistent_id.status == PIDStatus.REGISTERED:
-        return dict(pid=persistent_id.pid_value)
-    raise Exception('unable to resolve')
+    return ref_resolver('item', pid)

rero_ils/modules/jsonresolver.py

@@ -0,0 +1,49 @@
+# -*- coding: utf-8 -*-
+#
+# RERO ILS
+# Copyright (C) 2019 RERO
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, version 3 of the License.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+"""Holding resolver."""
+
+import click
+from flask import current_app
+from invenio_pidstore.models import PersistentIdentifier, PIDStatus
+
+
+def ref_resolver(pid_type, pid):
+    """Resolver for holding record."""
+    try:
+        persistent_id = PersistentIdentifier.get(pid_type, pid)
+    except Exception as err:
+        click.secho(
+            'Unable to resolve {pid_type} pid: {pid}'.format(
+                pid_type=pid_type,
+                pid=pid
+            ),
+            fg='red'
+        )
+    else:
+        if persistent_id.status == PIDStatus.REGISTERED:
+            return dict(pid=persistent_id.pid_value)
+        current_app.logger.error(
+            'Doc resolver error: /api/documents/{pid} {persistent_id}'.format(
+                pid=pid,
+                persistent_id=persistent_id
+            )
+        )
+    raise Exception('Unable to resolve {pid_type} pid: {pid}'.format(
+        pid=pid,
+        pid_type=pid_type
+    ))

rero_ils/modules/libraries/jsonresolver.py

@@ -19,13 +19,12 @@
 
 
 import jsonresolver
-from invenio_pidstore.models import PersistentIdentifier, PIDStatus
+
+from ..jsonresolver import ref_resolver
 
 
 @jsonresolver.route('/api/libraries/<pid>', host='ils.rero.ch')
 def library_resolver(pid):
     """Library resolver."""
+    return ref_resolver('lib', pid)
     persistent_id = PersistentIdentifier.get('lib', pid)
-    if persistent_id.status == PIDStatus.REGISTERED:
-        return dict(pid=persistent_id.pid_value)
-    raise Exception('unable to resolve')

rero_ils/modules/loans/jsonresolver.py

@@ -19,13 +19,11 @@
 
 
 import jsonresolver
-from invenio_pidstore.models import PersistentIdentifier, PIDStatus
+
+from ..jsonresolver import ref_resolver
 
 
 @jsonresolver.route('/api/loans/<pid>', host='ils.rero.ch')
 def loan_resolver(pid):
     """Loan resolver."""
-    persistent_id = PersistentIdentifier.get('loanid', pid)
-    if persistent_id.status == PIDStatus.REGISTERED:
-        return dict(pid=persistent_id.pid_value)
-    raise Exception('unable to resolve')
+    return ref_resolver('loanid', pid)

rero_ils/modules/locations/jsonresolver.py

@@ -19,13 +19,11 @@
 
 
 import jsonresolver
-from invenio_pidstore.models import PersistentIdentifier, PIDStatus
+
+from ..jsonresolver import ref_resolver
 
 
 @jsonresolver.route('/api/locations/<pid>', host='ils.rero.ch')
 def location_resolver(pid):
     """Location resolver."""
-    persistent_id = PersistentIdentifier.get('loc', pid)
-    if persistent_id.status == PIDStatus.REGISTERED:
-        return dict(pid=persistent_id.pid_value)
-    raise Exception('unable to resolve')
+    return ref_resolver('loc', pid)

rero_ils/modules/notifications/jsonresolver.py

@@ -18,20 +18,11 @@
 """Notifications resolver."""
 
 import jsonresolver
-from flask import current_app
-from invenio_pidstore.models import PersistentIdentifier, PIDStatus
+
+from ..jsonresolver import ref_resolver
 
 
 @jsonresolver.route('/api/notifications/<pid>', host='ils.rero.ch')
 def notification_resolver(pid):
     """Resolver for notifications record."""
-    persistent_id = PersistentIdentifier.get('notif', pid)
-    if persistent_id.status == PIDStatus.REGISTERED:
-        return dict(pid=persistent_id.pid_value)
-    current_app.logger.error(
-        'Doc resolver error: /api/notifications/{pid} {persistent_id}'.format(
-            pid=pid,
-            persistent_id=persistent_id
-        )
-    )
-    raise Exception('unable to resolve')
+    return ref_resolver('notif', pid)

rero_ils/modules/organisations/jsonresolver.py

@@ -19,13 +19,11 @@
 
 
 import jsonresolver
-from invenio_pidstore.models import PersistentIdentifier, PIDStatus
+
+from ..jsonresolver import ref_resolver
 
 
 @jsonresolver.route('/api/organisations/<pid>', host='ils.rero.ch')
 def organisation_resolver(pid):
     """Organisation resolver."""
-    persistent_id = PersistentIdentifier.get('org', pid)
-    if persistent_id.status == PIDStatus.REGISTERED:
-        return dict(pid=persistent_id.pid_value)
-    raise Exception('unable to resolve')
+    return ref_resolver('org', pid)

rero_ils/modules/patron_types/jsonresolver.py

@@ -19,13 +19,11 @@
 
 
 import jsonresolver
-from invenio_pidstore.models import PersistentIdentifier, PIDStatus
+
+from ..jsonresolver import ref_resolver
 
 
 @jsonresolver.route('/api/patron_types/<pid>', host='ils.rero.ch')
 def patron_type_resolver(pid):
     """Patron type resolver."""
-    persistent_id = PersistentIdentifier.get('ptty', pid)
-    if persistent_id.status == PIDStatus.REGISTERED:
-        return dict(pid=persistent_id.pid_value)
-    raise Exception('unable to resolve')
+    return ref_resolver('ptty', pid)

rero_ils/modules/patrons/jsonresolver.py

@@ -19,13 +19,11 @@
 
 
 import jsonresolver
-from invenio_pidstore.models import PersistentIdentifier, PIDStatus
+
+from ..jsonresolver import ref_resolver
 
 
 @jsonresolver.route('/api/patrons/<pid>', host='ils.rero.ch')
 def patron_resolver(pid):
     """Patron resolver."""
-    persistent_id = PersistentIdentifier.get('ptrn', pid)
-    if persistent_id.status == PIDStatus.REGISTERED:
-        return dict(pid=persistent_id.pid_value)
-    raise Exception('unable to resolve')
+    return ref_resolver('ptrn', pid)

scripts/setup

@@ -190,8 +190,8 @@ pipenv run invenio fixtures create --pid_type doc --schema 'http://ils.rero.ch/s
 if $CREATE_ITEMS_HOLDINGS_SMALL
 then
     # to generate small items file small documents must exist in DB
-    pipenv run invenio fixtures create_items -i 2 -t ${DATA_PATH}/items_small.json -h ${DATA_PATH}/holdings_small.json
-    display_success_message "Creation of items and holdings done for 'small' documents done."
+    pipenv run invenio fixtures create_items -i 3 -t ${DATA_PATH}/items_small.json -h ${DATA_PATH}/holdings_small.json
+    display_success_message "Creation of items and holdings done for 'small' documents."
     if $STOP_EXECUTION
     then
         exit 0
@@ -201,8 +201,8 @@ fi
 if $CREATE_ITEMS_HOLDINGS_BIG
 then
     # to generate big items file big documents must exist in DB
-    pipenv run invenio fixtures create_items -i 2 -t ${DATA_PATH}/items_big.json -h ${DATA_PATH}/holdings_big.json
-    display_success_message "Creation of items and holdings done for 'big' documents done."
+    pipenv run invenio fixtures create_items -i 3 -t ${DATA_PATH}/items_big.json -h ${DATA_PATH}/holdings_big.json
+    display_success_message "Creation of items and holdings done for 'big' documents."
     if $STOP_EXECUTION
     then
         exit 0
@@ -221,7 +221,7 @@ pipenv run invenio fixtures create --pid_type item --schema 'http://ils.rero.ch/
 pipenv run invenio index reindex -t item --yes-i-know
 pipenv run invenio index run -c 4 --raise-on-error
 
-# index documents
+display_success_message "Index Documents:"
 pipenv run invenio index reindex -t doc --yes-i-know
 pipenv run invenio index run -c 4 --raise-on-error
 

tests/api/test_pid_rest.py

@@ -18,9 +18,10 @@
 """API tests for PID and IlsRecords"""
 
 from invenio_accounts.testutils import login_user_via_session
-from rero_ils.modules.locations.api import Location
 from utils import postdata
 
+from rero_ils.modules.locations.api import Location
+
 
 def test_ilsrecord_pid_after_validationerror(
         client, loc_online_martigny_data, librarian_martigny_no_email):