fix: correct circulation policy

* Corrects item types and patron_types of circulation policy with pid = 10 and 11. * Adds test to check data consistency when ci-po are created. * Adds new patron type. * Corrects import sorting. * Closes #626. * Closes #213. Co-Authored-by: Alicia Zangger <alicia.zangger@rero.ch>
rero · Nov 22, 2019 · 3b23791 · 3b23791
1 parent 2356ada
commit 3b23791
Show file tree

Hide file tree

Showing 8 changed files with 118 additions and 39 deletions.
diff --git a/data/circulation_policies.json b/data/circulation_policies.json
@@ -331,15 +331,15 @@
           "$ref": "https://ils.rero.ch/api/patron_types/1"
         },
         "item_type": {
-          "$ref": "https://ils.rero.ch/api/item_types/7"
+          "$ref": "https://ils.rero.ch/api/item_types/8"
         }
       },
       {
         "patron_type": {
           "$ref": "https://ils.rero.ch/api/patron_types/2"
         },
         "item_type": {
-          "$ref": "https://ils.rero.ch/api/item_types/7"
+          "$ref": "https://ils.rero.ch/api/item_types/8"
         }
       }
     ]
@@ -359,18 +359,18 @@
     "settings": [
       {
         "patron_type": {
-          "$ref": "https://ils.rero.ch/api/patron_types/3"
+          "$ref": "https://ils.rero.ch/api/patron_types/6"
         },
         "item_type": {
-          "$ref": "https://ils.rero.ch/api/item_types/8"
+          "$ref": "https://ils.rero.ch/api/item_types/9"
         }
       },
       {
         "patron_type": {
           "$ref": "https://ils.rero.ch/api/patron_types/4"
         },
         "item_type": {
-          "$ref": "https://ils.rero.ch/api/item_types/8"
+          "$ref": "https://ils.rero.ch/api/item_types/9"
         }
       }
     ]

diff --git a/data/patron_types.json b/data/patron_types.json
@@ -43,5 +43,14 @@
     "organisation": {
       "$ref": "https://ils.rero.ch/api/organisations/3"
     }
+  },
+  {
+    "$schema": "https://ils.rero.ch/schema/patron_types/patron_type-v0.0.1.json",
+    "pid": "6",
+    "name": "Children",
+    "description": "Children and teenagers (< 16 years old).",
+    "organisation": {
+      "$ref": "https://ils.rero.ch/api/organisations/2"
+    }
   }
 ]
diff --git a/rero_ils/modules/api.py b/rero_ils/modules/api.py
@@ -106,9 +106,10 @@ def validate(self, **kwargs):
             raise RecordValidationError(validation_message)
 
     def extended_validation(self, **kwargs):
-        """Returns reasons for validation failures, otherwise True.
+        """Validate record against schema.
 
-        Override this function for classes that require extended validations
+        and extended validation to check that patron types and item types are
+        part of the correct organisation.
         """
         return True
 

diff --git a/rero_ils/modules/circ_policies/api.py b/rero_ils/modules/circ_policies/api.py
@@ -59,6 +59,28 @@ class CircPolicy(IlsRecord):
     fetcher = circ_policy_id_fetcher
     provider = CircPolicyProvider
 
+    def extended_validation(self, **kwargs):
+        """Returns reasons for validation failures, otherwise True.
+
+        Override this function for classes that require extended validations
+        """
+        from ..patron_types.api import PatronType
+        from ..item_types.api import ItemType
+
+        org = self.get('organisation')
+        for setting in self.replace_refs().get('settings', []):
+            patron_type = PatronType.get_record_by_pid(setting.get(
+                'patron_type', {}).get('pid')
+            )
+            item_type = ItemType.get_record_by_pid(setting.get(
+                'item_type', {}).get('pid')
+            )
+            if patron_type.get('organisation') != org or item_type.get(
+                'organisation'
+            ) != org:
+                return False
+        return True
+
     @classmethod
     def exist_name_and_organisation_pid(cls, name, organisation_pid):
         """Check if the policy name is unique on organisation."""

diff --git a/rero_ils/modules/cli.py b/rero_ils/modules/cli.py
@@ -209,18 +209,22 @@ def init(force):
 @click.option('-s', '--schema', 'schema', default=None)
 @click.option('-p', '--pid_type', 'pid_type', default=None)
 @click.option('-l', '--lazy', 'lazy', is_flag=True, default=False)
+@click.option('-o', '--dont-stop', 'dont_stop_on_error',
+              is_flag=True, default=False)
 @click.argument('infile', type=click.File('r'), default=sys.stdin)
 @with_appcontext
-def create(infile, append, reindex, dbcommit, verbose, schema, pid_type, lazy):
+def create(infile, append, reindex, dbcommit, verbose, schema, pid_type, lazy,
+           dont_stop_on_error):
     """Load REROILS record.
 
-    infile: Json file
-    append: appends pids to database
-    reindex: reindex record by record
-    dbcommit: commit record to database
-    pid_type: record type
-    schema: recoord schema
-    lazy: lazy reads file
+    :param infile: Json file
+    :param append: appends pids to database
+    :param reindex: reindex record by record
+    :param dbcommit: commit record to database
+    :param pid_type: record type
+    :param schema: recoord schema
+    :param lazy: lazy reads file
+    :param dont_stop_on_error: don't stop on error
     """
     click.secho(
         'Loading {pid_type} records from {file_name}.'.format(
@@ -263,15 +267,17 @@ def create(infile, append, reindex, dbcommit, verbose, schema, pid_type, lazy):
         except Exception as err:
             error_records.append(record)
             click.secho(
-                '{count: <8} {pid_type} creat error {pid}: {err}'.format(
+                '{count: <8} {pid_type} create error {pid}: {err}'.format(
                     count=count,
                     pid_type=pid_type,
                     pid=record.get('pid', '???'),
                     err=err
                 ),
                 err=True,
                 fg='red'
-                )
+            )
+            if not dont_stop_on_error:
+                sys.exit(1)
 
     if error_records:
         err_file_name = '{pid_type}_error.json'.format(pid_type=pid_type)

diff --git a/rero_ils/modules/patrons/views.py b/rero_ils/modules/patrons/views.py
@@ -32,7 +32,7 @@
 from ..documents.api import Document
 from ..items.api import Item
 from ..libraries.api import Library
-from ..loans.api import get_loans_by_patron_pid, Loan
+from ..loans.api import Loan, get_loans_by_patron_pid
 from ..locations.api import Location
 
 api_blueprint = Blueprint(

diff --git a/scripts/setup b/scripts/setup
@@ -25,6 +25,8 @@ DATA_PATH=$(pipenv --where)/data
 #       used for create only the items and holdings files for the 'big' documents file
 #  --deployment:
 #       used for deploy the 'big' files
+#  --dont_stop:
+#       used for continue script on error
 
 RED='\033[0;31m'
 GREEN='\033[0;0;32m'
@@ -43,9 +45,10 @@ CREATE_ITEMS_HOLDINGS_SMALL=false
 CREATE_ITEMS_HOLDINGS_BIG=false
 STOP_EXECUTION=true
 CREATE_LAZY=""
+DONT_STOP=""
 
 # options may be followed by one colon to indicate they have a required argument
-if ! options=$(getopt -o dsb -l deployment,create_items_holdings_small,create_items_holdings_big,lazy,data_path: -- "$@")
+if ! options=$(getopt -o dsb -l deployment,create_items_holdings_small,create_items_holdings_big,lazy,dont_stop,data_path: -- "$@")
 then
     # something went wrong, getopt will put out an error message for us
     exit 1
@@ -59,6 +62,7 @@ do
     -b|--create_items_holdings_big) CREATE_ITEMS_HOLDINGS_BIG=true ;;
     -c|--continue) STOP_EXECUTION=false ;;
     -l|--lazy) CREATE_LAZY="--lazy" ;;
+    -p|--pursue) DONT_STOP="--dont-stop" ;;
     -D|--data_path) DATA_PATH=$2 ;;
     (--) shift; break;;
     (-*) display_error_message "$0: error - unrecognized option $1"; exit 1;;
@@ -141,23 +145,23 @@ pipenv run invenio access allow superuser-access role superusers
 pipenv run invenio roles add admin@rero.ch admins
 pipenv run invenio roles add admin@rero.ch superusers
 
-display_success_message "Organisations: ${CREATE_LAZY}"
-pipenv run invenio fixtures create --pid_type org ${DATA_PATH}/organisations.json --append ${CREATE_LAZY}
+display_success_message "Organisations: ${CREATE_LAZY} ${DONT_STOP}"
+pipenv run invenio fixtures create --pid_type org ${DATA_PATH}/organisations.json --append ${CREATE_LAZY} ${DONT_STOP}
 pipenv run invenio index reindex -t org --yes-i-know
-display_success_message "Libraries: ${CREATE_LAZY}"
-pipenv run invenio fixtures create --pid_type lib ${DATA_PATH}/libraries.json --append ${CREATE_LAZY}
+display_success_message "Libraries: ${CREATE_LAZY} ${DONT_STOP}"
+pipenv run invenio fixtures create --pid_type lib ${DATA_PATH}/libraries.json --append ${CREATE_LAZY} ${DONT_STOP}
 pipenv run invenio index reindex -t lib --yes-i-know
-display_success_message "Locations: ${CREATE_LAZY}"
-pipenv run invenio fixtures create --pid_type loc ${DATA_PATH}/locations.json  --append ${CREATE_LAZY}
+display_success_message "Locations: ${CREATE_LAZY} ${DONT_STOP}"
+pipenv run invenio fixtures create --pid_type loc ${DATA_PATH}/locations.json  --append ${CREATE_LAZY} ${DONT_STOP}
 pipenv run invenio index reindex -t loc --yes-i-know
-display_success_message "Item types: ${CREATE_LAZY}"
-pipenv run invenio fixtures create --pid_type itty ${DATA_PATH}/item_types.json  --append ${CREATE_LAZY}
+display_success_message "Item types: ${CREATE_LAZY} ${DONT_STOP}"
+pipenv run invenio fixtures create --pid_type itty ${DATA_PATH}/item_types.json  --append ${CREATE_LAZY} ${DONT_STOP}
 pipenv run invenio index reindex -t itty --yes-i-know
-display_success_message "Patron types: ${CREATE_LAZY}"
-pipenv run invenio fixtures create --pid_type ptty ${DATA_PATH}/patron_types.json --append ${CREATE_LAZY}
+display_success_message "Patron types: ${CREATE_LAZY} ${DONT_STOP}"
+pipenv run invenio fixtures create --pid_type ptty ${DATA_PATH}/patron_types.json --append ${CREATE_LAZY} ${DONT_STOP}
 pipenv run invenio index reindex -t ptty --yes-i-know
-display_success_message "Circulation policies: ${CREATE_LAZY}"
-pipenv run invenio fixtures create --pid_type cipo ${DATA_PATH}/circulation_policies.json --append ${CREATE_LAZY}
+display_success_message "Circulation policies: ${CREATE_LAZY} ${DONT_STOP}"
+pipenv run invenio fixtures create --pid_type cipo ${DATA_PATH}/circulation_policies.json --append ${CREATE_LAZY} ${DONT_STOP}
 pipenv run invenio index reindex -t cipo --yes-i-know
 pipenv run invenio index run --raise-on-error
 
@@ -185,9 +189,9 @@ else
     HOLDINGS=${DATA_PATH}/holdings_small.json
 fi
 
-display_success_message "Documents:"
+display_success_message "Documents: ${DONT_STOP}"
 echo -e ${DOCUMENTS}
-pipenv run invenio fixtures create --pid_type doc --schema 'http://ils.rero.ch/schema/documents/document-v0.0.1.json' ${DOCUMENTS} --append
+pipenv run invenio fixtures create --pid_type doc --schema 'http://ils.rero.ch/schema/documents/document-v0.0.1.json' ${DOCUMENTS} --append ${DONT_STOP}
 
 if $CREATE_ITEMS_HOLDINGS_SMALL
 then
@@ -211,15 +215,15 @@ then
     fi
 fi
 
-display_success_message "Holdings: ${CREATE_LAZY}"
+display_success_message "Holdings: ${CREATE_LAZY} ${DONT_STOP}"
 echo -e ${HOLDINGS}
-pipenv run invenio fixtures create --pid_type hold --schema 'http://ils.rero.ch/schema/holdings/holding-v0.0.1.json' ${HOLDINGS} --append ${CREATE_LAZY}
+pipenv run invenio fixtures create --pid_type hold --schema 'http://ils.rero.ch/schema/holdings/holding-v0.0.1.json' ${HOLDINGS} --append ${CREATE_LAZY} ${DONT_STOP}
 pipenv run invenio index reindex -t hold --yes-i-know
 pipenv run invenio index run -c 4 --raise-on-error
 
-display_success_message "Items: ${CREATE_LAZY}"
+display_success_message "Items: ${CREATE_LAZY} ${DONT_STOP}"
 echo -e ${ITEMS}
-pipenv run invenio fixtures create --pid_type item --schema 'http://ils.rero.ch/schema/items/item-v0.0.1.json' ${ITEMS} --append ${CREATE_LAZY}
+pipenv run invenio fixtures create --pid_type item --schema 'http://ils.rero.ch/schema/items/item-v0.0.1.json' ${ITEMS} --append ${CREATE_LAZY} ${DONT_STOP}
 pipenv run invenio index reindex -t item --yes-i-know
 pipenv run invenio index run -c 4 --raise-on-error
 

diff --git a/tests/ui/circ_policies/test_circ_policies_api.py b/tests/ui/circ_policies/test_circ_policies_api.py
@@ -20,12 +20,13 @@
 from __future__ import absolute_import, print_function
 
 from copy import deepcopy
-
+import pytest
 import mock
 from utils import get_mapping
 
 from rero_ils.modules.circ_policies.api import CircPoliciesSearch, \
     CircPolicy, circ_policy_id_fetcher
+from rero_ils.modules.errors import RecordValidationError
 
 
 def test_no_default_policy(app):
@@ -34,7 +35,15 @@ def test_no_default_policy(app):
     assert not cipo
 
 
-def test_circ_policy_create(db, circ_policy_martigny_data_tmp):
+def test_circ_policy_create(circ_policy_martigny_data_tmp,
+                            org_martigny,
+                            lib_martigny, lib_saxon,
+                            patron_type_children_martigny,
+                            item_type_standard_martigny,
+                            patron_type_adults_martigny,
+                            item_type_specific_martigny,
+                            item_type_regular_sion,
+                            patron_type_youngsters_sion):
     """Test circulation policy creation."""
     cipo = CircPolicy.create(circ_policy_martigny_data_tmp, delete_pid=True)
     assert cipo == circ_policy_martigny_data_tmp
@@ -53,6 +62,34 @@ def test_circ_policy_create(db, circ_policy_martigny_data_tmp):
     assert cipo.get('$schema')
     assert cipo.get('pid') == '2'
 
+    cipo_data = {
+        '$schema': 'https://ils.rero.ch/schema/'
+        'circ_policies/circ_policy-v0.0.1.json',
+        'pid': 'cipo_test',
+        'name': 'test',
+        'organisation': {
+            '$ref': 'https://ils.rero.ch/api/organisations/org1'
+        },
+        'is_default': False,
+        'settings': [{
+            'patron_type': {
+                '$ref': 'https://ils.rero.ch/api/patron_types/ptty3'
+            },
+            'item_type': {
+                '$ref': 'https://ils.rero.ch/api/item_types/itty1'
+            }
+        }, {
+            'patron_type': {
+                '$ref': 'https://ils.rero.ch/api/patron_types/ptty2'
+            },
+            'item_type': {
+                '$ref': 'https://ils.rero.ch/api/item_types/itty4'
+            }
+        }]
+    }
+    with pytest.raises(RecordValidationError):
+        cipo = CircPolicy.create(cipo_data, delete_pid=False)
+
 
 def test_circ_policy_exist_name_and_organisation_pid(
         circ_policy_default_martigny):