templates: suppress links from the data section

* Removes all fields from the template data dictionary that are linked to other records or that causes issues with the circulation module and with data integrity at the creation of the templates. * Closes #2385. * Fixes problem when librarian can be deleted even if they own templates. Use the following command to clean up the current templates saved in database: The current templates will be backed-up in this file `templates.json` ``` poetry run tools.py tools migration clean_templates -o templates.json ``` Co-Authored-by: Aly Badr <[email protected]>
rero · Oct 13, 2021 · dd83a91 · dd83a91
1 parent c4bdd9e
commit dd83a91
Show file tree

Hide file tree

Showing 6 changed files with 141 additions and 10 deletions.
diff --git a/rero_ils/modules/patrons/api.py b/rero_ils/modules/patrons/api.py
@@ -38,6  38,7 @@
 from ..organisations.api import Organisation
 from ..patron_transactions.api import PatronTransaction
 from ..providers import Provider
 from ..templates.api import TemplatesSearch
 from ..users.api import User
 from ..utils import extracted_data_from_ref, get_patron_from_arguments, \
     get_ref_for_pid, sorted_pids, trim_patron_barcode_for_record
@@ -486,21  487,27 @@ def get_links_to_me(self, get_pids=False):
             links = {}
             exclude_states = [LoanState.CANCELLED, LoanState.ITEM_RETURNED,
                               LoanState.CREATED]
-            query = current_circulation.loan_search_cls()\
             loan_query = current_circulation.loan_search_cls()\
                 .filter('term', patron_pid=self.pid)\
                 .exclude('terms', state=exclude_states)
             template_query = TemplatesSearch()\
                 .filter('term', creator__pid=self.pid)
             if get_pids:
-                loans = sorted_pids(query)
                 loans = sorted_pids(loan_query)
                 transactions = PatronTransaction \
                     .get_transactions_pids_for_patron(self.pid, status='open')
                 templates = sorted_pids(template_query)
             else:
-                loans = query.count()
                 loans = loan_query.count()
                 transactions = PatronTransaction \
                     .get_transactions_count_for_patron(self.pid, status='open')
                 templates = template_query.count()
             if loans:
                 links['loans'] = loans
             if transactions:
                 links['transactions'] = transactions
             if templates:
                 links['templates'] = templates
             return links
 
     def reasons_to_keep(self):

diff --git a/rero_ils/modules/templates/api.py b/rero_ils/modules/templates/api.py
@@ -19,7  19,7 @@
 """API for manipulating Templates."""
 from functools import partial
 
-from .extensions import RemoveDataPidExtension
 from .extensions import CleanDataDictExtension
 from .models import TemplateIdentifier, TemplateMetadata
 from ..api import IlsRecord, IlsRecordsIndexer, IlsRecordsSearch
 from ..fetchers import id_fetcher
@@ -56,7  56,7 @@ class Meta:
 class Template(IlsRecord):
     """Templates class."""
 
-    _extensions = [RemoveDataPidExtension()]
     _extensions = [CleanDataDictExtension()]
 
     minter = template_id_minter
     fetcher = template_id_fetcher

diff --git a/rero_ils/modules/templates/extensions.py b/rero_ils/modules/templates/extensions.py
@@ -20,14  20,31 @@
 from invenio_records.extensions import RecordExtension
 
 
-class RemoveDataPidExtension(RecordExtension):
 class CleanDataDictExtension(RecordExtension):
     """Defines the methods needed by an extension."""
 
     def post_init(self, record, data, model=None, **kwargs):
         """Called after a record is initialized.
 
         Removes fields that can have a link to other records in the database.
 
         :param data: The dict passed to the record's constructor
         :param model: The model class used for initialization.
         """
-        # force removing of record pid
-        record.get('data', {}).pop('pid', None)
         fields = ['pid']
         if record.get('template_type') == 'items':
             fields = fields   [
                 'barcode', 'status', 'document', 'holding', 'organisation',
                 'library']
         elif record.get('template_type') == 'holdings':
             fields = fields   ['organisation', 'library', 'document']
         elif record.get('template_type') == 'patrons':
             fields = fields   [
                 'user_id', 'patron.subscriptions', 'patron.barcode']
 
         for field in fields:
             if '.' in field:
                 level_1, level_2 = field.split('.')
                 record.get('data', {}).get(level_1, {}).pop(level_2, None)
             else:
                 record.get('data', {}).pop(field, None)
diff --git a/tests/api/templates/test_templates_rest.py b/tests/api/templates/test_templates_rest.py
@@ -25,6  25,8 @@
 from utils import VerifyRecordPermissionPatch, get_json, postdata, \
     to_relative_url
 
 from rero_ils.modules.utils import get_ref_for_pid
 
 
 @mock.patch('invenio_records_rest.views.verify_record_permission',
             mock.MagicMock(return_value=VerifyRecordPermissionPatch))
@@ -200,12  202,16 @@ def test_template_secure_api(client, json_header,
 def test_template_secure_api_create(client, json_header,
                                     system_librarian_martigny,
                                     system_librarian_sion,
-                                    templ_doc_public_martigny_data):
                                     templ_doc_public_martigny_data,
                                     templ_item_public_martigny,
                                     templ_hold_public_martigny,
                                     templ_patron_public_martigny):
     """Test templates secure api create."""
     # Martigny
     login_user_via_session(client, system_librarian_martigny.user)
     post_entrypoint = 'invenio_records_rest.tmpl_list'
 
     # test template creation for documents
     del templ_doc_public_martigny_data['pid']
     # add a pid to the record data
     templ_doc_public_martigny_data['data']['pid'] = 'toto'
@@ -218,6  224,83 @@ def test_template_secure_api_create(client, json_header,
     # ensure that pid is removed from recordds
     assert 'pid' not in res.json['metadata']['data']
 
     # test template creation for items
     del templ_item_public_martigny['pid']
     # add fields that will be removed at the creation of the template.
     templ_item_public_martigny['data']['pid'] = 'toto'
     templ_item_public_martigny['data']['barcode'] = 'toto'
     templ_item_public_martigny['data']['status'] = 'on_loan'
     templ_item_public_martigny['data']['library'] = \
         {'$ref': get_ref_for_pid('lib', 'toto')}
     templ_item_public_martigny['data']['document'] = \
         {'$ref': get_ref_for_pid('doc', 'toto')}
     templ_item_public_martigny['data']['holding'] = \
         {'$ref': get_ref_for_pid('hold', 'toto')}
     templ_item_public_martigny['data']['organisation'] = \
         {'$ref': get_ref_for_pid('org', 'toto')}
 
     res, _ = postdata(
         client,
         post_entrypoint,
         templ_item_public_martigny
     )
     assert res.status_code == 201
     # ensure that added fields are removed from record.
     fields = [
         'barcode', 'pid', 'status', 'document', 'holding', 'organisation',
         'library']
     for field in fields:
         assert field not in res.json['metadata']['data']
 
     # templates now prevent the deletion of its owner
     assert system_librarian_martigny.get_links_to_me().get('templates')
 
     # test template creation for holdings
     del templ_hold_public_martigny['pid']
     # add fields that will be removed at the creation of the template.
     templ_hold_public_martigny['data']['pid'] = 'toto'
     templ_hold_public_martigny['data']['organisation'] = \
         {'$ref': get_ref_for_pid('org', 'toto')}
     templ_hold_public_martigny['data']['library'] = \
         {'$ref': get_ref_for_pid('lib', 'toto')}
     templ_hold_public_martigny['data']['document'] = \
         {'$ref': get_ref_for_pid('doc', 'toto')}
 
     res, _ = postdata(
         client,
         post_entrypoint,
         templ_hold_public_martigny
     )
     assert res.status_code == 201
     # ensure that added fields are removed from record.
     fields = ['organisation', 'library', 'document', 'pid']
     for field in fields:
         assert field not in res.json['metadata']['data']
 
     # test template creation for patrons
     del templ_patron_public_martigny['pid']
     # add fields that will be removed at the creation of the template.
     templ_patron_public_martigny['data']['pid'] = 'toto'
     templ_patron_public_martigny['data']['user_id'] = 'toto'
     templ_patron_public_martigny['data']['patron']['subscriptions'] = 'toto'
     templ_patron_public_martigny['data']['patron']['barcode'] = ['toto']
 
     res, _ = postdata(
         client,
         post_entrypoint,
         templ_patron_public_martigny
     )
     assert res.status_code == 201
     # ensure that added fields are removed from record.
     fields = ['user_id', 'patron.subscriptions', 'patron.barcode', 'pid']
     json_data = res.json['metadata']['data']
     for field in fields:
         if '.' in field:
             level_1, level_2 = field.split('.')
             assert level_2 not in json_data.get(level_1)
         else:
             assert field not in json_data
 
     # Sion
     login_user_via_session(client, system_librarian_sion.user)
 

diff --git a/tests/data/data.json b/tests/data/data.json
@@ -3825,7  3825,11 @@
     },
     "visibility": "public",
     "template_type": "patrons",
-    "data": {}
     "data": {
       "patron": {
         "source": "imported"
       }
     }
   },
   "illr1": {
     "$schema": "https://bib.rero.ch/schemas/ill_requests/ill_request-v0.0.1.json",

diff --git a/tests/fixtures/metadata.py b/tests/fixtures/metadata.py
@@ -1000,6  1000,26 @@ def templ_item_public_martigny(
     return template
 
 
 @pytest.fixture(scope="module")
 def templ_hold_public_martigny_data(data):
     """Load template for a public holding martigny data."""
     return deepcopy(data.get('tmpl4'))
 
 
 @pytest.fixture(scope="module")
 def templ_hold_public_martigny(
         app, org_martigny, templ_hold_public_martigny_data,
         system_librarian_martigny):
     """Load template for a public holding martigny."""
     template = Template.create(
         data=templ_hold_public_martigny_data,
         delete_pid=False,
         dbcommit=True,
         reindex=True)
     flush_index(TemplatesSearch.Meta.index)
     return template
 
 
 @pytest.fixture(scope="module")
 def templ_patron_public_martigny_data(data):
     """Load template for a public patron martigny data."""