CircPoliy: add unique default policy validation

Closes rero#2079.

Co-Authored-by: Renaud Michotte <renaud.michotte@gmail.com>

rero_ils/modules/circ_policies/api.py

@@ -89,6 +89,13 @@ def extended_validation(self, **kwargs):
         from ..item_types.api import ItemType
         from ..patron_types.api import PatronType
 
+        # Only one default policy by organisation
+        if self.get('is_default', False):
+            default_cipo = CircPolicy.get_default_circ_policy(
+                self.organisation_pid)
+            if default_cipo and default_cipo.pid != self.pid:
+                return 'CircPolicy: already a default policy for this org'
+
         for library in self.get('libraries', []):
             library_pid = extracted_data_from_ref(library)
             if not Library.get_record_by_pid(library_pid):
@@ -147,6 +154,15 @@ def extended_validation(self, **kwargs):
 
         return True
 
+    @classmethod
+    def create(cls, data, id_=None, delete_pid=False,
+               dbcommit=True, reindex=True, pidcheck=True, **kwargs):
+        """Create a new circulation policy record."""
+        # default behavior is to reindex the record. Needed to check that there
+        # is only one default policy by organisation
+        return super().create(
+            data, id_, delete_pid, dbcommit, reindex, **kwargs)
+
     @classmethod
     def exist_name_and_organisation_pid(cls, name, organisation_pid):
         """Check if the policy name is unique on organisation.

tests/api/circ_policies/test_circ_policies_rest.py

@@ -18,16 +18,14 @@
 """Tests REST API for circulation policies."""
 
 import json
+from copy import deepcopy
 
 import mock
-import pytest
 from flask import url_for
 from invenio_accounts.testutils import login_user_via_session
 from utils import VerifyRecordPermissionPatch, get_json, postdata, \
     to_relative_url
 
-from rero_ils.modules.api import IlsRecordError
-
 
 def test_circ_policies_permissions(
         client, circ_policy_default_martigny, json_header):
@@ -118,32 +116,30 @@ def test_filtered_circ_policies_get(
 @mock.patch('invenio_records_rest.views.verify_record_permission',
             mock.MagicMock(return_value=VerifyRecordPermissionPatch))
 def test_circ_policies_post_put_delete(client, org_martigny,
-                                       circ_policy_default_martigny_data,
+                                       circ_policy_short_martigny_data,
                                        json_header):
     """Test policy retrieval."""
     # Create policy / POST
     item_url = url_for('invenio_records_rest.cipo_item', pid_value='1')
     list_url = url_for('invenio_records_rest.cipo_list', q='pid:1')
-    del circ_policy_default_martigny_data['pid']
+    del circ_policy_short_martigny_data['pid']
     res, data = postdata(
         client,
         'invenio_records_rest.cipo_list',
-        circ_policy_default_martigny_data
+        circ_policy_short_martigny_data
     )
     assert res.status_code == 201
 
     # Check that the returned policy matches the given data
-    circ_policy_default_martigny_data['pid'] = '1'
-
-    assert data['metadata'] == circ_policy_default_martigny_data
-
+    circ_policy_short_martigny_data['pid'] = '1'
+    assert data['metadata'] == circ_policy_short_martigny_data
     res = client.get(item_url)
     assert res.status_code == 200
     data = get_json(res)
-    assert circ_policy_default_martigny_data == data['metadata']
+    assert circ_policy_short_martigny_data == data['metadata']
 
     # Update policy/PUT
-    data = circ_policy_default_martigny_data
+    data = circ_policy_short_martigny_data
     data['name'] = 'Test Name'
     res = client.put(
         item_url,
@@ -170,12 +166,8 @@ def test_circ_policies_post_put_delete(client, org_martigny,
     assert data['metadata']['name'] == 'Test Name'
 
     # Delete policy/DELETE
-    with pytest.raises(IlsRecordError.NotDeleted):
-        res = client.delete(item_url)
-    assert res.status_code == 200
-
-    res = client.get(item_url)
-    assert res.status_code == 200
+    res = client.delete(item_url)
+    assert res.status_code == 204
 
 
 @mock.patch('rero_ils.modules.decorators.login_and_librarian',
@@ -230,37 +222,27 @@ def test_circ_policy_secure_api(client, json_header,
 
 
 def test_circ_policy_secure_api_create(client, json_header,
-                                       circ_policy_default_martigny,
                                        system_librarian_martigny,
                                        system_librarian_sion,
-                                       circ_policy_default_martigny_data):
+                                       circ_policy_short_martigny_data):
     """Test circulation policies secure api create."""
     # Martigny
     login_user_via_session(client, system_librarian_martigny.user)
     post_entrypoint = 'invenio_records_rest.cipo_list'
 
-    del circ_policy_default_martigny_data['pid']
-    res, _ = postdata(
-        client,
-        post_entrypoint,
-        circ_policy_default_martigny_data
-    )
+    cipo_data = deepcopy(circ_policy_short_martigny_data)
+    del cipo_data['pid']
+    res, _ = postdata(client, post_entrypoint, cipo_data)
     assert res.status_code == 201
 
     # Sion
     login_user_via_session(client, system_librarian_sion.user)
-
-    res, _ = postdata(
-        client,
-        post_entrypoint,
-        circ_policy_default_martigny_data
-    )
+    res, _ = postdata(client, post_entrypoint, cipo_data)
     assert res.status_code == 403
 
 
 def test_circ_policy_secure_api_update(client,
                                        circ_policy_short_martigny,
-                                       circ_policy_short_martigny_data,
                                        circ_policy_temp_martigny,
                                        circ_policy_temp_martigny_data,
                                        system_librarian_martigny,
@@ -274,7 +256,7 @@ def test_circ_policy_secure_api_update(client,
     record_url = url_for('invenio_records_rest.cipo_item',
                          pid_value=circ_policy_short_martigny.pid)
 
-    data = circ_policy_short_martigny_data
+    data = deepcopy(circ_policy_short_martigny)
     data['name'] = 'Test Name'
     res = client.put(
         record_url,

tests/ui/circ_policies/test_circ_policies_api.py

@@ -36,6 +36,7 @@ def test_no_default_policy(app):
 
 
 def test_circ_policy_create(circ_policy_martigny_data_tmp,
+                            circ_policy_short_martigny_data,
                             org_martigny,
                             lib_martigny, lib_saxon,
                             patron_type_children_martigny,
@@ -56,9 +57,9 @@ def test_circ_policy_create(circ_policy_martigny_data_tmp,
     assert fetched_pid.pid_value == '1'
     assert fetched_pid.pid_type == 'cipo'
 
-    circ_policy = deepcopy(circ_policy_martigny_data_tmp)
-    del circ_policy['$schema']
-    cipo = CircPolicy.create(circ_policy, delete_pid=True)
+    circ_policy_data = deepcopy(circ_policy_short_martigny_data)
+    del circ_policy_data['$schema']
+    cipo = CircPolicy.create(circ_policy_data, delete_pid=True)
     assert cipo.get('$schema')
     assert cipo.get('pid') == '2'
 
@@ -92,22 +93,32 @@ def test_circ_policy_create(circ_policy_martigny_data_tmp,
     with pytest.raises(ValidationError):
         cipo = CircPolicy.create(cipo_data, delete_pid=False)
 
+    # TEST #2 : create a second defaut policy
+    #   The first created policy (pid=1) is the default policy.
+    #   Creation of a second default policy should raise a ValidationError
+    default_cipo = CircPolicy.get_record_by_pid('1')
+    assert default_cipo.get('is_default')
+    with pytest.raises(ValidationError) as excinfo:
+        CircPolicy.create(circ_policy_martigny_data_tmp, delete_pid=True)
+    assert 'CircPolicy: already a default policy for this org' \
+           in str(excinfo.value)
+
 
 def test_circ_policy_exist_name_and_organisation_pid(
-        circ_policy_default_martigny):
-    """Test policy name existance."""
-    circ_policy = circ_policy_default_martigny
-    cipo = circ_policy.replace_refs()
+        circ_policy_short_martigny):
+    """Test policy name existence."""
+    cipo = circ_policy_short_martigny.replace_refs()
     assert CircPolicy.exist_name_and_organisation_pid(
         cipo.get('name'), cipo.get('organisation', {}).get('pid'))
     assert not CircPolicy.exist_name_and_organisation_pid(
         'not exists yet', cipo.get('organisation', {}).get('pid'))
 
 
-def test_circ_policy_can_not_delete(circ_policy_default_martigny,
-                                    circ_policy_short_martigny):
+def test_circ_policy_can_not_delete(circ_policy_short_martigny):
     """Test can not delete a policy."""
-    can, reasons = circ_policy_default_martigny.can_delete
+    org_pid = circ_policy_short_martigny.organisation_pid
+    defaut_cipo = CircPolicy.get_default_circ_policy(org_pid)
+    can, reasons = defaut_cipo.can_delete
     assert not can
     assert reasons['others']['is_default']