holdings: allow deletion of serials holdings

* Closes rero#1720.

Co-Authored-by: Peter Weber <peter.weber@rero.ch>

rero_ils/modules/api.py

@@ -37,6 +37,7 @@
 from invenio_pidstore.models import PersistentIdentifier, PIDStatus
 from invenio_records.api import Record
 from invenio_records_rest.utils import obj_or_import_string
+from invenio_search import current_search
 from invenio_search.api import RecordsSearch
 from jsonschema.exceptions import ValidationError
 from kombu.compat import Consumer
@@ -84,6 +85,11 @@ class Meta:
 
         default_filter = None
 
+    @classmethod
+    def flush_and_refresh(cls):
+        """Flush and refresh index."""
+        current_search.flush_and_refresh(cls.Meta.index)
+
 
 class IlsRecord(Record):
     """ILS Record class."""

rero_ils/modules/holdings/api.py

@@ -34,7 +34,7 @@
 from rero_ils.modules.items.models import ItemIssueStatus
 
 from .models import HoldingIdentifier, HoldingMetadata, HoldingTypes
-from ..api import IlsRecord, IlsRecordsIndexer
+from ..api import IlsRecord, IlsRecordError, IlsRecordsIndexer
 from ..documents.api import Document
 from ..errors import MissingRequiredParameterError, RegularReceiveNotAllowed
 from ..fetchers import id_fetcher
@@ -173,6 +173,22 @@ def extended_validation(self, **kwargs):
             return _('Can not have multiple notes of same type.')
         return True
 
+    def delete(self, force=False, dbcommit=False, delindex=False):
+        """Delete record and persistent identifier."""
+        can, _ = self.can_delete
+        if can:
+            if self.is_serial:
+                # Delete all attached items
+                for item in self.get_items:
+                    item.delete(
+                        force=force, dbcommit=dbcommit, delindex=delindex)
+                if delindex:
+                    ItemsSearch.flush_and_refresh()
+            return super().delete(
+                force=force, dbcommit=dbcommit, delindex=delindex)
+        else:
+            raise IlsRecordError.NotDeleted()
+
     @property
     def is_serial(self):
         """Shortcut to check if holding is a serial holding record."""
@@ -344,13 +360,15 @@ def get_items(self):
         """Return standard items and received issues for a holding record."""
         for item_pid in Item.get_items_pid_by_holding_pid(self.pid):
             item = Item.get_record_by_pid(item_pid)
-            if not item.issue_status or \
-                    item.issue_status == ItemIssueStatus.RECEIVED:
-                # inherit holdings first call# for issues with no 1st call#.
-                issue_call_number = item.issue_inherited_first_call_number
-                if issue_call_number:
-                    item['call_number'] = issue_call_number
-                yield item
+            if item:
+                if not item.issue_status or \
+                        item.issue_status == ItemIssueStatus.RECEIVED:
+                    # inherit holdings first call#
+                    # for issues with no 1st call#.
+                    issue_call_number = item.issue_inherited_first_call_number
+                    if issue_call_number:
+                        item['call_number'] = issue_call_number
+                    yield item
 
     def get_number_of_items(self):
         """Get holding number of items."""
@@ -375,9 +393,19 @@ def get_links_to_me(self):
     def reasons_not_to_delete(self):
         """Get reasons not to delete record."""
         cannot_delete = {}
-        links = self.get_links_to_me()
-        if links:
-            cannot_delete['links'] = links
+        if self.is_serial:
+            # Find out if we can delete all items
+            not_deleteable_items = [
+                item for item in self.get_items if item.reasons_not_to_delete()
+            ]
+            if not_deleteable_items:
+                count = len(not_deleteable_items)
+                cannot_delete['others'] = {
+                    _(f'has {count} items with loan attached'): count}
+        else:
+            links = self.get_links_to_me()
+            if links:
+                cannot_delete['links'] = links
         return cannot_delete
 
     def get_holding_loan_conditions(self):

rero_ils/modules/items/api/api.py

@@ -102,10 +102,6 @@ def reasons_not_to_delete(self):
         links = self.get_links_to_me()
         if links:
             cannot_delete['links'] = links
-        if self.item_record_type == 'issue' and self.issue_is_regular:
-            cannot_delete['others'] = dict(
-                regular_issue_cannot_be_deleted=True
-            )
         return cannot_delete
 
     def in_collection(self, **kwargs):

tests/api/items/test_items_issue.py

@@ -47,7 +47,6 @@ def test_issues_permissions(client, json_header,
     assert issue_item is not None
     assert issue_item.issue_is_regular
 
-    # a regular issue cannot be deleted
     res = client.get(
         url_for(
             'api_blueprint.permissions',
@@ -57,4 +56,4 @@ def test_issues_permissions(client, json_header,
     )
     assert res.status_code == 200
     data = get_json(res)
-    assert not data['delete']['can']
+    assert data['delete']['can']

tests/ui/holdings/test_holdings_patterns.py

@@ -29,7 +29,6 @@
 from invenio_accounts.testutils import login_user_via_session
 from jsonschema.exceptions import ValidationError
 
-from rero_ils.modules.api import IlsRecordError
 from rero_ils.modules.holdings.api import Holding
 from rero_ils.modules.holdings.models import HoldingNoteTypes
 from rero_ils.modules.items.api import Item
@@ -552,30 +551,6 @@ def update_pattern(holding, frequency):
         previous_expected_date = expected_date
 
 
-def test_regular_issue_creation_update_delete_api(
-        client, holding_lib_martigny_w_patterns, loc_public_martigny,
-        lib_martigny):
-    """Test create, update and delete of a regular issue API."""
-    holding = holding_lib_martigny_w_patterns
-    issue_display, expected_date = holding._get_next_issue_display_text(
-                        holding.get('patterns'))
-    issue = holding.receive_regular_issue(dbcommit=True, reindex=True)
-    item = deepcopy(issue)
-    item['issue']['status'] = ItemIssueStatus.DELETED
-    issue.update(data=item, dbcommit=True, reindex=True)
-    created_issue = Item.get_record_by_pid(issue.pid)
-    assert created_issue.get('issue').get('status') == ItemIssueStatus.DELETED
-    # Unable to delete a regular issue
-    with pytest.raises(IlsRecordError.NotDeleted):
-        created_issue.delete(dbcommit=True, delindex=True)
-
-    # no errors when deleting an irregular issue
-    pid = created_issue.pid
-    created_issue.get('issue')['regular'] = False
-    created_issue.delete(dbcommit=True, delindex=True)
-    assert not Item.get_record_by_pid(pid)
-
-
 def test_holding_notes(client, librarian_martigny,
                        holding_lib_martigny_w_patterns, json_header):
     """Test holdings notes."""
@@ -609,3 +584,16 @@ def test_holding_notes(client, librarian_martigny,
     assert holding.get_note(HoldingNoteTypes.STAFF)
     assert holding.get_note(HoldingNoteTypes.ROUTING) is None
     assert holding.get_note('dummy') is None
+
+
+def test_regular_issue_creation_update_delete_api(
+        client, holding_lib_martigny_w_patterns, loc_public_martigny,
+        lib_martigny):
+    """Test create, update and delete of a regular issue API."""
+    holding = holding_lib_martigny_w_patterns
+    issue_display, expected_date = holding._get_next_issue_display_text(
+                        holding.get('patterns'))
+    issue = holding.receive_regular_issue(dbcommit=True, reindex=True)
+    issue_pid = issue.pid
+    assert holding.delete(dbcommit=True, delindex=True)
+    assert not Item.get_record_by_pid(issue_pid)